linux-kernel - Re: [PATCH] vfs: fix EBUSY on FSCONFIG_CMD

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <k7kcsc6wljl32mik2qqwij23hjsqtxqbuq6a5gbu7r6z33vq5c@7jeeepio6jkd>
Date: Mon, 15 Dec 2025 09:46:19 +0100
From: Jan Kara <jack@...e.cz>
To: me@...ck-desk.cn
Cc: Alexander Viro <viro@...iv.linux.org.uk>, 
	Christian Brauner <brauner@...nel.org>, Jan Kara <jack@...e.cz>, linux-fsdevel@...r.kernel.org, 
	linux-kernel@...r.kernel.org, stable@...r.kernel.org
Subject: 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@...ck-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@...r.kernel.org
> Signed-off-by: Chen Linxuan <me@...ck-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@...e.cz>

								Honza

-- 
Jan Kara <jack@...e.com>
SUSE Labs, CR