diff options
| author | NeilBrown <neil@brown.name> | 2025-11-13 11:18:37 +1100 |
|---|---|---|
| committer | Christian Brauner <brauner@kernel.org> | 2025-11-14 13:15:58 +0100 |
| commit | fe497f0759e0efb949f9480911d00b6045c21f50 (patch) | |
| tree | 1b0fa2942903931da22de557d9641b6c6a1c23e0 /fs/cachefiles | |
| parent | f046fbb4d81d1b0c4a169707411e3cd540c03354 (diff) | |
| download | linux-fe497f0759e0efb949f9480911d00b6045c21f50.tar.gz linux-fe497f0759e0efb949f9480911d00b6045c21f50.tar.bz2 linux-fe497f0759e0efb949f9480911d00b6045c21f50.zip | |
VFS: change vfs_mkdir() to unlock on failure.
vfs_mkdir() already drops the reference to the dentry on failure but it
leaves the parent locked.
This complicates end_creating() which needs to unlock the parent even
though the dentry is no longer available.
If we change vfs_mkdir() to unlock on failure as well as releasing the
dentry, we can remove the "parent" arg from end_creating() and simplify
the rules for calling it.
Note that cachefiles_get_directory() can choose to substitute an error
instead of actually calling vfs_mkdir(), for fault injection. In that
case it needs to call end_creating(), just as vfs_mkdir() now does on
error.
ovl_create_real() will now unlock on error. So the conditional
end_creating() after the call is removed, and end_creating() is called
internally on error.
Reviewed-by: Amir Goldstein <amir73il@gmail.com>
Reviewed-by: Jeff Layton <jlayton@kernel.org>
Tested-by: syzbot@syzkaller.appspotmail.com
Signed-off-by: NeilBrown <neil@brown.name>
Link: https://patch.msgid.link/20251113002050.676694-15-neilb@ownmail.net
Signed-off-by: Christian Brauner <brauner@kernel.org>
Diffstat (limited to 'fs/cachefiles')
| -rw-r--r-- | fs/cachefiles/namei.c | 16 |
1 files changed, 9 insertions, 7 deletions
diff --git a/fs/cachefiles/namei.c b/fs/cachefiles/namei.c index 0104ac00485d..59327618ac42 100644 --- a/fs/cachefiles/namei.c +++ b/fs/cachefiles/namei.c @@ -128,10 +128,12 @@ retry: if (ret < 0) goto mkdir_error; ret = cachefiles_inject_write_error(); - if (ret == 0) + if (ret == 0) { subdir = vfs_mkdir(&nop_mnt_idmap, d_inode(dir), subdir, 0700); - else + } else { + end_creating(subdir); subdir = ERR_PTR(ret); + } if (IS_ERR(subdir)) { trace_cachefiles_vfs_error(NULL, d_inode(dir), ret, cachefiles_trace_mkdir_error); @@ -140,7 +142,7 @@ retry: trace_cachefiles_mkdir(dir, subdir); if (unlikely(d_unhashed(subdir) || d_is_negative(subdir))) { - end_creating(subdir, dir); + end_creating(subdir); goto retry; } ASSERT(d_backing_inode(subdir)); @@ -154,7 +156,7 @@ retry: /* Tell rmdir() it's not allowed to delete the subdir */ inode_lock(d_inode(subdir)); dget(subdir); - end_creating(subdir, dir); + end_creating(subdir); if (!__cachefiles_mark_inode_in_use(NULL, d_inode(subdir))) { pr_notice("cachefiles: Inode already in use: %pd (B=%lx)\n", @@ -196,7 +198,7 @@ mark_error: return ERR_PTR(-EBUSY); mkdir_error: - end_creating(subdir, dir); + end_creating(subdir); pr_err("mkdir %s failed with error %d\n", dirname, ret); return ERR_PTR(ret); @@ -699,7 +701,7 @@ bool cachefiles_commit_tmpfile(struct cachefiles_cache *cache, if (ret < 0) goto out_end; - end_creating(dentry, fan); + end_creating(dentry); ret = cachefiles_inject_read_error(); if (ret == 0) @@ -733,7 +735,7 @@ bool cachefiles_commit_tmpfile(struct cachefiles_cache *cache, } out_end: - end_creating(dentry, fan); + end_creating(dentry); out: _leave(" = %u", success); return success; |