Re: [PATCH] vfs: fix EBUSY on FSCONFIG_CMD_CREATE retry

On Sat 13-12-25 02:03:56, Chen Linxuan via B4 Relay wrote:

From: Chen Linxuan me@black-desk.cn

When using fsconfig(..., FSCONFIG_CMD_CREATE, ...), the filesystem context is retrieved from the file descriptor. Since the file structure persists across syscall restarts, the context state is preserved:

// fs/fsopen.c SYSCALL_DEFINE5(fsconfig, ...) { ... fc = fd_file(f)->private_data; ... ret = vfs_fsconfig_locked(fc, cmd, &param); ... }

In vfs_cmd_create(), the context phase is transitioned to FS_CONTEXT_CREATING before calling vfs_get_tree():

// fs/fsopen.c static int vfs_cmd_create(struct fs_context *fc, bool exclusive) { ... fc->phase = FS_CONTEXT_CREATING; ... ret = vfs_get_tree(fc); ... }

However, vfs_get_tree() may return -ERESTARTNOINTR if the filesystem implementation needs to restart the syscall. For example, cgroup v1 does this when it encounters a race condition where the root is dying:

// kernel/cgroup/cgroup-v1.c int cgroup1_get_tree(struct fs_context *fc) { ... if (unlikely(ret > 0)) { msleep(10); return restart_syscall(); } return ret; }

If the syscall is restarted, fsconfig() is called again and retrieves the *same* fs_context. However, vfs_cmd_create() rejects the call because the phase was left as FS_CONTEXT_CREATING during the first attempt:

Well, not quite. The phase is actually set to FS_CONTEXT_FAILED if vfs_get_tree() returns any error. Still the effect is the same.

if (fc->phase != FS_CONTEXT_CREATE_PARAMS) return -EBUSY;

Fix this by resetting fc->phase back to FS_CONTEXT_CREATE_PARAMS if vfs_get_tree() returns -ERESTARTNOINTR.

Cc: stable@vger.kernel.org Signed-off-by: Chen Linxuan me@black-desk.cn

---

fs/fsopen.c | 4 ++++ 1 file changed, 4 insertions(+)

diff --git a/fs/fsopen.c b/fs/fsopen.c index f645c99204eb..8a7cb031af50 100644 --- a/fs/fsopen.c +++ b/fs/fsopen.c @@ -229,6 +229,10 @@ static int vfs_cmd_create(struct fs_context *fc, bool exclusive) fc->exclusive = exclusive; ret = vfs_get_tree(fc);

• if (ret == -ERESTARTNOINTR) {

• fc->phase = FS_CONTEXT_CREATE_PARAMS;
  

• return ret;
  

• } if (ret) { fc->phase = FS_CONTEXT_FAILED; return ret;

Thanks for the patch. It looks good to me. I'd slightly prefer style like:

if (ret) { if (ret == -ERESTARTNOINTR) fc->phase = FS_CONTEXT_CREATE_PARAMS; else fc->phase = FS_CONTEXT_FAILED; return ret; }

but I can live with what you propose as well. So feel free to add:

Reviewed-by: Jan Kara jack@suse.cz

Honza