Re: [PATCH v2 2/8] fuse: add helpers for EIO return value with kernel message

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] fs/fuse: uninitialized err returned in fuse_file_io_open()
--

commit bfd7cbb3d1064d14b9422fa08119f1cc83817677
Author: Miklos Szeredi <mszeredi@redhat.com>

fuse: add helpers for EIO return value with kernel message

This commit introduces helpers to print a short message to the kernel log
and return an error value when a buggy fuse server triggers an EIO
condition. It also updates various I/O and passthrough open handlers to
utilize these helpers, removing older debug messages and changing some
functions to return boolean values on success.

> diff --git a/fs/fuse/iomode.c b/fs/fuse/iomode.c
> index 79637c09e8839..1a10bc65fb310 100644
> --- a/fs/fuse/iomode.c
> +++ b/fs/fuse/iomode.c
[ ... ]
> @@ -208,11 +201,12 @@ int fuse_file_io_open(struct file *file, struct inode *inode)
>  
>  	/*
>  	 * Server is expected to use FOPEN_PASSTHROUGH for all opens of an inode
> -	 * which is already open for passthrough.
> +	 * which is already open for passthrough.  Using incorrect open mode is
> +	 * a server mistake, which results in user visible failure of open()
> +	 * with EIO error.
>  	 */
> -	err = -EINVAL;
>  	if (fuse_inode_backing(fi) && !(ff->open_flags & FOPEN_PASSTHROUGH))
> -		goto fail;
> +		return fuse_EIO("FOPEN_PASSTHROUGH expected");
[ ... ]
>  	if (ff->open_flags & FOPEN_PASSTHROUGH)
>  		err = fuse_file_passthrough_open(inode, file);
> -	else
> -		err = fuse_file_cached_io_open(inode, ff);
> -	if (err)
> -		goto fail;
> +	else if (!fuse_file_cached_io_open(inode, ff))
> +		err = fuse_EIO("conflicting passthrough open");
>  
> -	return 0;
> -
> -fail:
> -	pr_debug("failed to open file in requested io mode (open_flags=0x%x, err=%i).\n",
> -		 ff->open_flags, err);
> -	/*
> -	 * The file open mode determines the inode io mode.
> -	 * Using incorrect open mode is a server mistake, which results in
> -	 * user visible failure of open() with EIO error.
> -	 */
> -	return -EIO;
> +	return err;
>  }

[Severity: High]
Does this code leak an uninitialized stack value on success?

On the standard cached I/O open path, if the FOPEN_PASSTHROUGH flag is
not set and fuse_file_cached_io_open() returns true for success,
execution will bypass both branches of the if/else-if statement.

Since the initial err = -EINVAL assignment was removed earlier in the
function, the err variable remains unassigned in this scenario. This means
a successful file open will return an uninitialized stack variable,
potentially leading to undefined VFS behavior or random system call
failures depending on the stack contents.

Could we initialize err to 0 at declaration, or explicitly return 0 on
the success path?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261001150935.655979-1-mszeredi@redhat.com?part=2