| 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
| ||
|
Message-Id: <1457714135-50289-3-git-send-email-seth.forshee@canonical.com>
Date: Fri, 11 Mar 2016 10:35:34 -0600
From: Seth Forshee <seth.forshee@...onical.com>
To: Miklos Szeredi <miklos@...redi.hu>
Cc: m.loschwitz@...eleven.de, robert@...byte.com,
seth.forshee@...onical.com, fuse-devel@...ts.sourceforge.net,
linux-kernel@...r.kernel.org
Subject: [PATCH 2/2] fuse: Add reference counting for fuse_io_priv
The req member of fuse_io_priv serves two purposes. First is to
track the number of oustanding async requests to the server and
to signal that the io request is completed. The second is to be a
reference count on the structure to know when it can be freed.
For sync io requests these purposes can be at odds.
fuse_direct_IO() wants to block until the request is done, and
since the signal is sent when req reaches 0 it cannot keep a
reference to the object. Yet it needs to use the object after the
userspace server has completed processing requests. This leads to
some handshaking and special casing that it needlessly
complicated and responsible for at least one race condition.
It's much cleaner and safer to maintain a separate reference
count for the object lifecycle and to let req just be a count of
outstanding requests to the userspace server. Then we can know
for sure when it is safe to free the object without any
handshaking or special cases.
The catch here is that most of the time these objects are stack
allocated and should not be freed. Initializing these objects
with a single reference that is never released prevents
accidental attempts to free the objects.
Fixes: 9d5722b7777e ("fuse: handle synchronous iocbs internally")
Cc: stable@...r.kernel.org # v4.1+
Signed-off-by: Seth Forshee <seth.forshee@...onical.com>
---
fs/fuse/cuse.c | 12 ++++++++++--
fs/fuse/file.c | 42 ++++++++++++++++++++++++++++++++++--------
fs/fuse/fuse_i.h | 15 +++++++++++++++
3 files changed, 59 insertions(+), 10 deletions(-)
diff --git a/fs/fuse/cuse.c b/fs/fuse/cuse.c
index 8e3ee1936c7e..990436a21533 100644
--- a/fs/fuse/cuse.c
+++ b/fs/fuse/cuse.c
@@ -90,7 +90,11 @@ static struct list_head *cuse_conntbl_head(dev_t devt)
static ssize_t cuse_read_iter(struct kiocb *kiocb, struct iov_iter *to)
{
- struct fuse_io_priv io = { .async = 0, .file = kiocb->ki_filp };
+ struct fuse_io_priv io = {
+ .refcnt = ATOMIC_INIT(1),
+ .async = 0,
+ .file = kiocb->ki_filp,
+ };
loff_t pos = 0;
return fuse_direct_io(&io, to, &pos, FUSE_DIO_CUSE);
@@ -98,7 +102,11 @@ static ssize_t cuse_read_iter(struct kiocb *kiocb, struct iov_iter *to)
static ssize_t cuse_write_iter(struct kiocb *kiocb, struct iov_iter *from)
{
- struct fuse_io_priv io = { .async = 0, .file = kiocb->ki_filp };
+ struct fuse_io_priv io = {
+ .refcnt = ATOMIC_INIT(1),
+ .async = 0,
+ .file = kiocb->ki_filp,
+ };
loff_t pos = 0;
/*
* No locking or generic_write_checks(), the server is
diff --git a/fs/fuse/file.c b/fs/fuse/file.c
index 34a23df361ca..6403c35fa39a 100644
--- a/fs/fuse/file.c
+++ b/fs/fuse/file.c
@@ -585,8 +585,9 @@ static void fuse_aio_complete(struct fuse_io_priv *io, int err, ssize_t pos)
}
io->iocb->ki_complete(io->iocb, res, 0);
- kfree(io);
}
+
+ fuse_io_unref(io);
}
static void fuse_aio_complete_req(struct fuse_conn *fc, struct fuse_req *req)
@@ -613,6 +614,7 @@ static size_t fuse_async_req_send(struct fuse_conn *fc, struct fuse_req *req,
size_t num_bytes, struct fuse_io_priv *io)
{
spin_lock(&io->lock);
+ fuse_io_ref(io);
io->size += num_bytes;
io->reqs++;
spin_unlock(&io->lock);
@@ -691,7 +693,11 @@ static void fuse_short_read(struct fuse_req *req, struct inode *inode,
static int fuse_do_readpage(struct file *file, struct page *page)
{
- struct fuse_io_priv io = { .async = 0, .file = file };
+ struct fuse_io_priv io = {
+ .refcnt = ATOMIC_INIT(1),
+ .async = 0,
+ .file = file,
+ };
struct inode *inode = page->mapping->host;
struct fuse_conn *fc = get_fuse_conn(inode);
struct fuse_req *req;
@@ -984,7 +990,11 @@ static size_t fuse_send_write_pages(struct fuse_req *req, struct file *file,
size_t res;
unsigned offset;
unsigned i;
- struct fuse_io_priv io = { .async = 0, .file = file };
+ struct fuse_io_priv io = {
+ .refcnt = ATOMIC_INIT(1),
+ .async = 0,
+ .file = file,
+ };
for (i = 0; i < req->num_pages; i++)
fuse_wait_on_page_writeback(inode, req->pages[i]->index);
@@ -1398,7 +1408,11 @@ static ssize_t __fuse_direct_read(struct fuse_io_priv *io,
static ssize_t fuse_direct_read_iter(struct kiocb *iocb, struct iov_iter *to)
{
- struct fuse_io_priv io = { .async = 0, .file = iocb->ki_filp };
+ struct fuse_io_priv io = {
+ .refcnt = ATOMIC_INIT(1),
+ .async = 0,
+ .file = iocb->ki_filp,
+ };
return __fuse_direct_read(&io, to, &iocb->ki_pos);
}
@@ -1406,7 +1420,11 @@ static ssize_t fuse_direct_write_iter(struct kiocb *iocb, struct iov_iter *from)
{
struct file *file = iocb->ki_filp;
struct inode *inode = file_inode(file);
- struct fuse_io_priv io = { .async = 0, .file = file };
+ struct fuse_io_priv io = {
+ .refcnt = ATOMIC_INIT(1),
+ .async = 0,
+ .file = file,
+ };
ssize_t res;
if (is_bad_inode(inode))
@@ -2864,6 +2882,7 @@ fuse_direct_IO(struct kiocb *iocb, struct iov_iter *iter, loff_t offset)
if (!io)
return -ENOMEM;
spin_lock_init(&io->lock);
+ atomic_set(&io->refcnt, 1);
io->reqs = 1;
io->bytes = -1;
io->size = 0;
@@ -2887,8 +2906,15 @@ fuse_direct_IO(struct kiocb *iocb, struct iov_iter *iter, loff_t offset)
iov_iter_rw(iter) == WRITE)
io->async = false;
- if (io->async && is_sync)
- io->done = &wait;
+ if (is_sync) {
+ /*
+ * Additional reference to keep io around after
+ * calling fuse_aio_complete()
+ */
+ fuse_io_ref(io);
+ if (io->async)
+ io->done = &wait;
+ }
if (iov_iter_rw(iter) == WRITE) {
ret = fuse_direct_io(io, iter, &pos, FUSE_DIO_WRITE);
@@ -2908,7 +2934,7 @@ fuse_direct_IO(struct kiocb *iocb, struct iov_iter *iter, loff_t offset)
ret = fuse_get_res_by_io(io);
}
- kfree(io);
+ fuse_io_unref(io);
if (iov_iter_rw(iter) == WRITE) {
if (ret > 0)
diff --git a/fs/fuse/fuse_i.h b/fs/fuse/fuse_i.h
index ce394b5fe6b4..77fb63e6dbae 100644
--- a/fs/fuse/fuse_i.h
+++ b/fs/fuse/fuse_i.h
@@ -22,6 +22,7 @@
#include <linux/rbtree.h>
#include <linux/poll.h>
#include <linux/workqueue.h>
+#include <linux/atomic.h>
/** Max number of pages that can be used in a single read request */
#define FUSE_MAX_PAGES_PER_REQ 32
@@ -243,6 +244,7 @@ struct fuse_args {
/** The request IO state (for asynchronous processing) */
struct fuse_io_priv {
+ atomic_t refcnt;
int async;
spinlock_t lock;
unsigned reqs;
@@ -256,6 +258,19 @@ struct fuse_io_priv {
struct completion *done;
};
+static inline void fuse_io_ref(struct fuse_io_priv *io)
+{
+ atomic_inc(&io->refcnt);
+}
+
+static inline void fuse_io_unref(struct fuse_io_priv *io)
+{
+ int cnt = atomic_dec_return(&io->refcnt);
+ BUG_ON(cnt < 0);
+ if (cnt == 0)
+ kfree(io);
+}
+
/**
* Request flags
*
--
1.9.1
Powered by blists - more mailing lists