On Mon 01 Feb 2016 11:15:56 AM PST, Jann Horn wrote:
On Mon, Feb 01, 2016 at 10:56:27AM -0800, Nikhilesh Reddy wrote:
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);
+
+ 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);
Why the get_file() and fput() in this method? This doesn't look right. There
is no lock you're releasing between get_file() and fput(). What are they
intended for?
Hi
Thanks for reviewing the code.
The passthrough file could be released under our feet say if the
userspace fuse daemon crashed or was killed ( while we are processing
the read or the write) causing bad things to happen.
The calls here are to increase the count temporarily and then decrease
it so that we dont release in the middle of a write and everything is
gracefully handled...
I have a comment right before the get_file call above saying the same
thing.
Please let me know if you have any more questions.
--
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.