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] [day] [month] [year] [list]
Message-ID: <20250929-musizieren-toxikologie-7b7674306977@brauner>
Date: Mon, 29 Sep 2025 10:58:14 +0200
From: Christian Brauner <brauner@...nel.org>
To: Deepanshu Kartikey <kartikey406@...il.com>
Cc: viro@...iv.linux.org.uk, jack@...e.cz, linux-fsdevel@...r.kernel.org, 
	linux-kernel@...r.kernel.org, syzbot+9eefe09bedd093f156c2@...kaller.appspotmail.com
Subject: Re: [PATCH v2] nsfs: handle inode number mismatches gracefully in
 file handles

On Wed, Sep 24, 2025 at 05:20:58PM +0530, Deepanshu Kartikey wrote:
> Replace VFS_WARN_ON_ONCE() with graceful error handling when file
> handles contain inode numbers that don't match the actual namespace
> inode. This prevents userspace from triggering kernel warnings by
> providing malformed file handles to open_by_handle_at().
> 
> The issue occurs when userspace provides a file handle with valid
> namespace type and ID that successfully locates a namespace, but
> specifies an incorrect inode number. Previously, this would trigger
> VFS_WARN_ON_ONCE() when comparing the real inode number against the
> provided value.
> 
> Since file handle data is user-controllable, inode number mismatches
> should be treated as invalid input rather than kernel consistency
> errors. Handle this case by returning NULL to indicate the file
> handle is invalid, rather than warning about what is essentially
> user input validation.
> 
> Reported-by: syzbot+9eefe09bedd093f156c2@...kaller.appspotmail.com
> Suggested-by: Jan Kara <jack@...e.cz>
> Reviewed-by: Jan Kara <jack@...e.cz>
> Signed-off-by: Deepanshu Kartikey <kartikey406@...il.com>
> ---
>  fs/nsfs.c | 5 +++--
>  1 file changed, 3 insertions(+), 2 deletions(-)
> 
> Changes in v2:
> - Handle all inode number mismatches, not just zero, as suggested by Jan Kara
> - Replace warning with graceful error handling for better architecture
> 
> diff --git a/fs/nsfs.c b/fs/nsfs.c
> index 32cb8c835a2b..002d424d9fa6 100644
> --- a/fs/nsfs.c
> +++ b/fs/nsfs.c
> @@ -490,8 +490,9 @@ static struct dentry *nsfs_fh_to_dentry(struct super_block *sb, struct fid *fh,
>  
>  		VFS_WARN_ON_ONCE(ns->ns_id != fid->ns_id);
>  		VFS_WARN_ON_ONCE(ns->ops->type != fid->ns_type);
> -		VFS_WARN_ON_ONCE(ns->inum != fid->ns_inum);
> -
> +		/* Someone is playing games and passing invalid file handles? */
> +		if (ns->inum != fid->ns_inum)
> +			return NULL;
>  		if (!refcount_inc_not_zero(&ns->count))
>  			return NULL;
>  	}
> -- 
> 2.43.0
> 

That seems sane although I have considered to relax the decoding part in
the future. IOW, the kernel must always return the file handle with all
fields filled in. But userspace may be allowed to specify just the
->ns_id and leave both ->inum and ->ns_type zero. This is based on a
patch for next cycled "unified nstree". Anyway, thanks for the fix.

Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ