From 28a625cbc2a14f17b83e47ef907b2658576a32aa Mon Sep 17 00:00:00 2001 From: Miklos Szeredi Date: Wed, 22 Jan 2014 19:36:57 +0100 Subject: fuse: fix pipe_buf_operations Having this struct in module memory could Oops when if the module is unloaded while the buffer still persists in a pipe. Since sock_pipe_buf_ops is essentially the same as fuse_dev_pipe_buf_steal merge them into nosteal_pipe_buf_ops (this is the same as default_pipe_buf_ops except stealing the page from the buffer is not allowed). Reported-by: Al Viro Signed-off-by: Miklos Szeredi Cc: stable@vger.kernel.org --- fs/fuse/dev.c | 22 +++++----------------- fs/splice.c | 18 ++++++++++++++++++ include/linux/pipe_fs_i.h | 2 ++ net/core/skbuff.c | 32 +------------------------------- 4 files changed, 26 insertions(+), 48 deletions(-) diff --git a/fs/fuse/dev.c b/fs/fuse/dev.c index ef74ad5fd362..fa8cb4b7b8fe 100644 --- a/fs/fuse/dev.c +++ b/fs/fuse/dev.c @@ -1296,22 +1296,6 @@ static ssize_t fuse_dev_read(struct kiocb *iocb, const struct iovec *iov, return fuse_dev_do_read(fc, file, &cs, iov_length(iov, nr_segs)); } -static int fuse_dev_pipe_buf_steal(struct pipe_inode_info *pipe, - struct pipe_buffer *buf) -{ - return 1; -} - -static const struct pipe_buf_operations fuse_dev_pipe_buf_ops = { - .can_merge = 0, - .map = generic_pipe_buf_map, - .unmap = generic_pipe_buf_unmap, - .confirm = generic_pipe_buf_confirm, - .release = generic_pipe_buf_release, - .steal = fuse_dev_pipe_buf_steal, - .get = generic_pipe_buf_get, -}; - static ssize_t fuse_dev_splice_read(struct file *in, loff_t *ppos, struct pipe_inode_info *pipe, size_t len, unsigned int flags) @@ -1358,7 +1342,11 @@ static ssize_t fuse_dev_splice_read(struct file *in, loff_t *ppos, buf->page = bufs[page_nr].page; buf->offset = bufs[page_nr].offset; buf->len = bufs[page_nr].len; - buf->ops = &fuse_dev_pipe_buf_ops; + /* + * Need to be careful about this. Having buf->ops in module + * code can Oops if the buffer persists after module unload. + */ + buf->ops = &nosteal_pipe_buf_ops; pipe->nrbufs++; page_nr++; diff --git a/fs/splice.c b/fs/splice.c index 46a08f772d7d..12028fa41def 100644 --- a/fs/splice.c +++ b/fs/splice.c @@ -555,6 +555,24 @@ static const struct pipe_buf_operations default_pipe_buf_ops = { .get = generic_pipe_buf_get, }; +static int generic_pipe_buf_nosteal(struct pipe_inode_info *pipe, + struct pipe_buffer *buf) +{ + return 1; +} + +/* Pipe buffer operations for a socket and similar. */ +const struct pipe_buf_operations nosteal_pipe_buf_ops = { + .can_merge = 0, + .map = generic_pipe_buf_map, + .unmap = generic_pipe_buf_unmap, + .confirm = generic_pipe_buf_confirm, + .release = generic_pipe_buf_release, + .steal = generic_pipe_buf_nosteal, + .get = generic_pipe_buf_get, +}; +EXPORT_SYMBOL(nosteal_pipe_buf_ops); + static ssize_t kernel_readv(struct file *file, const struct iovec *vec, unsigned long vlen, loff_t offset) { diff --git a/include/linux/pipe_fs_i.h b/include/linux/pipe_fs_i.h index b8809fef61f5..ab5752692113 100644 --- a/include/linux/pipe_fs_i.h +++ b/include/linux/pipe_fs_i.h @@ -157,6 +157,8 @@ int generic_pipe_buf_confirm(struct pipe_inode_info *, struct pipe_buffer *); int generic_pipe_buf_steal(struct pipe_inode_info *, struct pipe_buffer *); void generic_pipe_buf_release(struct pipe_inode_info *, struct pipe_buffer *); +extern const struct pipe_buf_operations nosteal_pipe_buf_ops; + /* for F_SETPIPE_SZ and F_GETPIPE_SZ */ long pipe_fcntl(struct file *, unsigned int, unsigned long arg); struct pipe_inode_info *get_pipe_info(struct file *file); diff --git a/net/core/skbuff.c b/net/core/skbuff.c index 06e72d3cdf60..0b5149c5bc4a 100644 --- a/net/core/skbuff.c +++ b/net/core/skbuff.c @@ -74,36 +74,6 @@ struct kmem_cache *skbuff_head_cache __read_mostly; static struct kmem_cache *skbuff_fclone_cache __read_mostly; -static void sock_pipe_buf_release(struct pipe_inode_info *pipe, - struct pipe_buffer *buf) -{ - put_page(buf->page); -} - -static void sock_pipe_buf_get(struct pipe_inode_info *pipe, - struct pipe_buffer *buf) -{ - get_page(buf->page); -} - -static int sock_pipe_buf_steal(struct pipe_inode_info *pipe, - struct pipe_buffer *buf) -{ - return 1; -} - - -/* Pipe buffer operations for a socket. */ -static const struct pipe_buf_operations sock_pipe_buf_ops = { - .can_merge = 0, - .map = generic_pipe_buf_map, - .unmap = generic_pipe_buf_unmap, - .confirm = generic_pipe_buf_confirm, - .release = sock_pipe_buf_release, - .steal = sock_pipe_buf_steal, - .get = sock_pipe_buf_get, -}; - /** * skb_panic - private function for out-of-line support * @skb: buffer @@ -1830,7 +1800,7 @@ int skb_splice_bits(struct sk_buff *skb, unsigned int offset, .partial = partial, .nr_pages_max = MAX_SKB_FRAGS, .flags = flags, - .ops = &sock_pipe_buf_ops, + .ops = &nosteal_pipe_buf_ops, .spd_release = sock_spd_release, }; struct sk_buff *frag_iter; -- cgit v1.2.3 From 063ec1e595f8a82b5a8fd0acb3e88c8b49a1e6c1 Mon Sep 17 00:00:00 2001 From: Miklos Szeredi Date: Wed, 22 Jan 2014 19:36:58 +0100 Subject: fuse: fix SetPageUptodate() condition in STORE As noticed by Coverity the "num != 0" condition never triggers. Instead it should check for a complete page. Signed-off-by: Miklos Szeredi --- fs/fuse/dev.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/fs/fuse/dev.c b/fs/fuse/dev.c index fa8cb4b7b8fe..0a648bb455ae 100644 --- a/fs/fuse/dev.c +++ b/fs/fuse/dev.c @@ -1587,7 +1587,8 @@ static int fuse_notify_store(struct fuse_conn *fc, unsigned int size, this_num = min_t(unsigned, num, PAGE_CACHE_SIZE - offset); err = fuse_copy_page(cs, &page, offset, this_num, 0); - if (!err && offset == 0 && (num != 0 || file_size == end)) + if (!err && offset == 0 && + (this_num == PAGE_CACHE_SIZE || file_size == end)) SetPageUptodate(page); unlock_page(page); page_cache_release(page); -- cgit v1.2.3 From 451418fc928b5ec1ee96a9afac807b6312811a2a Mon Sep 17 00:00:00 2001 From: Andrew Gallagher Date: Tue, 5 Nov 2013 03:55:43 -0800 Subject: fuse: don't invalidate attrs when not using atime Various read operations (e.g. readlink, readdir) invalidate the cached attrs for atime changes. This patch adds a new function 'fuse_invalidate_atime', which checks for a read-only super block and avoids the attr invalidation in that case. Signed-off-by: Andrew Gallagher Signed-off-by: Miklos Szeredi --- fs/fuse/dir.c | 14 ++++++++++++-- fs/fuse/file.c | 4 ++-- fs/fuse/fuse_i.h | 2 ++ 3 files changed, 16 insertions(+), 4 deletions(-) diff --git a/fs/fuse/dir.c b/fs/fuse/dir.c index c3eb2c46c8f1..1d1292c581c3 100644 --- a/fs/fuse/dir.c +++ b/fs/fuse/dir.c @@ -112,6 +112,16 @@ void fuse_invalidate_attr(struct inode *inode) get_fuse_inode(inode)->i_time = 0; } +/** + * Mark the attributes as stale due to an atime change. Avoid the invalidate if + * atime is not used. + */ +void fuse_invalidate_atime(struct inode *inode) +{ + if (!IS_RDONLY(inode)) + fuse_invalidate_attr(inode); +} + /* * Just mark the entry as stale, so that a next attempt to look it up * will result in a new lookup call to userspace @@ -1371,7 +1381,7 @@ static int fuse_readdir(struct file *file, struct dir_context *ctx) } __free_page(page); - fuse_invalidate_attr(inode); /* atime changed */ + fuse_invalidate_atime(inode); return err; } @@ -1404,7 +1414,7 @@ static char *read_link(struct dentry *dentry) link[req->out.args[0].size] = '\0'; out: fuse_put_request(fc, req); - fuse_invalidate_attr(inode); /* atime changed */ + fuse_invalidate_atime(inode); return link; } diff --git a/fs/fuse/file.c b/fs/fuse/file.c index 7e70506297bc..d53af8f15236 100644 --- a/fs/fuse/file.c +++ b/fs/fuse/file.c @@ -687,7 +687,7 @@ static int fuse_readpage(struct file *file, struct page *page) SetPageUptodate(page); } - fuse_invalidate_attr(inode); /* atime changed */ + fuse_invalidate_atime(inode); out: unlock_page(page); return err; @@ -716,7 +716,7 @@ static void fuse_readpages_end(struct fuse_conn *fc, struct fuse_req *req) fuse_read_update_size(inode, pos, req->misc.read.attr_ver); } - fuse_invalidate_attr(inode); /* atime changed */ + fuse_invalidate_atime(inode); } for (i = 0; i < req->num_pages; i++) { diff --git a/fs/fuse/fuse_i.h b/fs/fuse/fuse_i.h index 7d2730912667..dc44b9e3a0c9 100644 --- a/fs/fuse/fuse_i.h +++ b/fs/fuse/fuse_i.h @@ -788,6 +788,8 @@ void fuse_invalidate_attr(struct inode *inode); void fuse_invalidate_entry_cache(struct dentry *entry); +void fuse_invalidate_atime(struct inode *inode); + /** * Acquire reference to fuse_conn */ -- cgit v1.2.3 From 7678ac50615d9c7a491d9861e020e4f5f71b594c Mon Sep 17 00:00:00 2001 From: Andrew Gallagher Date: Tue, 5 Nov 2013 16:05:52 +0100 Subject: fuse: support clients that don't implement 'open' open/release operations require userspace transitions to keep track of the open count and to perform any FS-specific setup. However, for some purely read-only FSs which don't need to perform any setup at open/release time, we can avoid the performance overhead of calling into userspace for open/release calls. This patch adds the necessary support to the fuse kernel modules to prevent open/release operations from hitting in userspace. When the client returns ENOSYS, we avoid sending the subsequent release to userspace, and also remember this so that future opens also don't trigger a userspace operation. Signed-off-by: Miklos Szeredi --- fs/fuse/file.c | 37 +++++++++++++++++++++++++++---------- fs/fuse/fuse_i.h | 3 +++ 2 files changed, 30 insertions(+), 10 deletions(-) diff --git a/fs/fuse/file.c b/fs/fuse/file.c index d53af8f15236..74f6ca500504 100644 --- a/fs/fuse/file.c +++ b/fs/fuse/file.c @@ -127,7 +127,15 @@ static void fuse_file_put(struct fuse_file *ff, bool sync) if (atomic_dec_and_test(&ff->count)) { struct fuse_req *req = ff->reserved_req; - if (sync) { + if (ff->fc->no_open) { + /* + * Drop the release request when client does not + * implement 'open' + */ + req->background = 0; + path_put(&req->misc.release.path); + fuse_put_request(ff->fc, req); + } else if (sync) { req->background = 0; fuse_request_send(ff->fc, req); path_put(&req->misc.release.path); @@ -144,27 +152,36 @@ static void fuse_file_put(struct fuse_file *ff, bool sync) int fuse_do_open(struct fuse_conn *fc, u64 nodeid, struct file *file, bool isdir) { - struct fuse_open_out outarg; struct fuse_file *ff; - int err; int opcode = isdir ? FUSE_OPENDIR : FUSE_OPEN; ff = fuse_file_alloc(fc); if (!ff) return -ENOMEM; - err = fuse_send_open(fc, nodeid, file, opcode, &outarg); - if (err) { - fuse_file_free(ff); - return err; + ff->fh = 0; + ff->open_flags = FOPEN_KEEP_CACHE; /* Default for no-open */ + if (!fc->no_open || isdir) { + struct fuse_open_out outarg; + int err; + + err = fuse_send_open(fc, nodeid, file, opcode, &outarg); + if (!err) { + ff->fh = outarg.fh; + ff->open_flags = outarg.open_flags; + + } else if (err != -ENOSYS || isdir) { + fuse_file_free(ff); + return err; + } else { + fc->no_open = 1; + } } if (isdir) - outarg.open_flags &= ~FOPEN_DIRECT_IO; + ff->open_flags &= ~FOPEN_DIRECT_IO; - ff->fh = outarg.fh; ff->nodeid = nodeid; - ff->open_flags = outarg.open_flags; file->private_data = fuse_file_get(ff); return 0; diff --git a/fs/fuse/fuse_i.h b/fs/fuse/fuse_i.h index dc44b9e3a0c9..2da5db2c8bdb 100644 --- a/fs/fuse/fuse_i.h +++ b/fs/fuse/fuse_i.h @@ -485,6 +485,9 @@ struct fuse_conn { * and hence races in setting them will not cause malfunction */ + /** Is open/release not implemented by fs? */ + unsigned no_open:1; + /** Is fsync not implemented by fs? */ unsigned no_fsync:1; -- cgit v1.2.3