Re: [PATCH v5] fuse: Add support for passthrough read/write

[Jann Horn]

On Mon, Feb 01, 2016 at 10:56:27AM -0800, Nikhilesh Reddy wrote:
> Add support for filesystem passthrough read/write of files
> when enabled in userspace through the option FUSE_PASSTHROUGH.
[...]
> diff --git a/fs/fuse/passthrough.c b/fs/fuse/passthrough.c
[...]
> +static ssize_t fuse_passthrough_read_write_iter(struct kiocb *iocb,
> +					    struct iov_iter *iter, int do_write)
> +{
> +	ssize_t ret_val;
> +	struct fuse_file *ff;
> +	struct file *fuse_file, *passthrough_filp;
> +	struct inode *fuse_inode, *passthrough_inode;
> +
> +	ff = iocb->ki_filp->private_data;
> +	fuse_file = iocb->ki_filp;
> +	passthrough_filp = ff->passthrough_filp;
> +
> +	/* lock passthrough file to prevent it from being released */
> +	get_file(passthrough_filp);
> +	iocb->ki_filp = passthrough_filp;
> +	fuse_inode = fuse_file->f_path.dentry->d_inode;
> +	passthrough_inode = file_inode(passthrough_filp);
> +
> +	if (do_write) {
> +		if (!passthrough_filp->f_op->write_iter)
> +			return -EIO;
> +		ret_val = passthrough_filp->f_op->write_iter(iocb, iter);

Uh... how do you know at this point that the file is actually writable?
Normally, e.g. vfs_write() will ensure that the file is writable, and
e.g. generic_file_write_iter() won't check for writability as far as I
can tell. This might allow someone to use the passthrough mechanism to
overwrite a file he is only allowed to read, but not write, like
/etc/passwd.

Also, I think this might bypass mandatory locks, the
security_file_permission hook (which seems like a bad idea anyway
though), inotify/fsnotify and sb_start_write.



> +
> +		if (ret_val >= 0 || ret_val == -EIOCBQUEUED) {
> +			fsstack_copy_inode_size(fuse_inode, passthrough_inode);
> +			fsstack_copy_attr_times(fuse_inode, passthrough_inode);
> +		}
> +	} else {
> +		if (!passthrough_filp->f_op->read_iter)
> +			return -EIO;
> +		ret_val = passthrough_filp->f_op->read_iter(iocb, iter);
> +		if (ret_val >= 0 || ret_val == -EIOCBQUEUED)
> +			fsstack_copy_attr_atime(fuse_inode, passthrough_inode);
> +	}
> +
> +	iocb->ki_filp = fuse_file;
> +
> +	/* unlock passthrough file */
> +	fput(passthrough_filp);
> +
> +	return ret_val;
> +}
> +
> +ssize_t fuse_passthrough_read_iter(struct kiocb *iocb, struct iov_iter *to)
> +{
> +	return fuse_passthrough_read_write_iter(iocb, to, 0);
> +}
> +
> +ssize_t fuse_passthrough_write_iter(struct kiocb *iocb, struct iov_iter *from)
> +{
> +	return fuse_passthrough_read_write_iter(iocb, from, 1);
> +}
> +
> +void fuse_passthrough_release(struct fuse_file *ff)
> +{
> +	if (!(ff->passthrough_filp))
> +		return;
> +
> +	/* Release the passthrough file. */
> +	fput(ff->passthrough_filp);
> +	ff->passthrough_filp = NULL;
> +}
> diff --git a/include/uapi/linux/fuse.h b/include/uapi/linux/fuse.h
> index c9aca04..a08933a 100644
> --- a/include/uapi/linux/fuse.h
> +++ b/include/uapi/linux/fuse.h
> @@ -250,6 +250,7 @@ struct fuse_file_lock {
>  #define FUSE_ASYNC_DIO		(1 << 15)
>  #define FUSE_WRITEBACK_CACHE	(1 << 16)
>  #define FUSE_NO_OPEN_SUPPORT	(1 << 17)
> +#define FUSE_PASSTHROUGH	(1 << 18)
>  
>  /**
>   * CUSE INIT request/reply flags
> @@ -480,7 +481,7 @@ struct fuse_create_in {
>  struct fuse_open_out {
>  	uint64_t	fh;
>  	uint32_t	open_flags;
> -	uint32_t	padding;
> +	int32_t         passthrough_fd;
>  };
>  
>  struct fuse_release_in {
> -- 
> 1.8.2.1
> 
> 
> -- 
> Thanks
> Nikhilesh Reddy
> 
> Qualcomm Innovation Center, Inc.
> The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
> a Linux Foundation Collaborative Project.

Attachment: signature.asc
Description: Digital signature