Re: [PATCH 08/32] aio: replace kiocb_set_cancel_fn with a cancel_kiocb file operation

2018-05-19 Thread Al Viro 
On Tue, May 15, 2018 at 09:48:09PM +0200, Christoph Hellwig wrote:

>   case -EIOCBQUEUED:
> + if (req->ki_filp->f_op->cancel_kiocb) {
> + struct aio_kiocb *iocb =
> + container_of(req, struct aio_kiocb, rw);
> + struct kioctx *ctx = iocb->ki_ctx;
> + unsigned long flags;
> +
> + spin_lock_irqsave(>ctx_lock, flags);
> + list_add_tail(>ki_list, >active_reqs);

Use after free - that list insertion used to be done by drivers and doing
so before any ->ki_complete() calls might've happened used to be their
responsibility.  Now you've taken that to the point after ->read_iter()
(or ->write_iter()) return, so there's no way in hell to guarantee it's
not been completed (and freed) by that point.

Incidentally, none of the callers gives a damn about the difference between
0 and -EIOCBQUEUED now, so aio_rw_ret() might as well had been made void...

Re: [PATCH 08/32] aio: replace kiocb_set_cancel_fn with a cancel_kiocb file operation

2018-05-19 Thread Al Viro 
On Tue, May 15, 2018 at 09:48:09PM +0200, Christoph Hellwig wrote:

>   case -EIOCBQUEUED:
> + if (req->ki_filp->f_op->cancel_kiocb) {
> + struct aio_kiocb *iocb =
> + container_of(req, struct aio_kiocb, rw);
> + struct kioctx *ctx = iocb->ki_ctx;
> + unsigned long flags;
> +
> + spin_lock_irqsave(>ctx_lock, flags);
> + list_add_tail(>ki_list, >active_reqs);

Use after free - that list insertion used to be done by drivers and doing
so before any ->ki_complete() calls might've happened used to be their
responsibility.  Now you've taken that to the point after ->read_iter()
(or ->write_iter()) return, so there's no way in hell to guarantee it's
not been completed (and freed) by that point.

Incidentally, none of the callers gives a damn about the difference between
0 and -EIOCBQUEUED now, so aio_rw_ret() might as well had been made void...

[PATCH 08/32] aio: replace kiocb_set_cancel_fn with a cancel_kiocb file operation

2018-05-15 Thread Christoph Hellwig 
The current kiocb_set_cancel_fn implementation assumes the kiocb is
embedded into an aio_kiocb, which is fundamentally unsafe as it might
have been submitted by non-aio callers.  Instead add a cancel_kiocb
file operation that replaced the ki_cancel function pointer set by
kiocb_set_cancel_fn, and only adds iocbs to the active list when
the read/write_iter methods return -EIOCBQUEUED and the file has
a cancel_kiocb method.

Signed-off-by: Christoph Hellwig 
---
 drivers/usb/gadget/function/f_fs.c | 10 ++
 drivers/usb/gadget/legacy/inode.c  |  5 ++---
 fs/aio.c   | 31 --
 fs/orangefs/orangefs-kernel.h  |  1 -
 include/linux/aio.h|  7 ---
 include/linux/fs.h |  1 +
 6 files changed, 17 insertions(+), 38 deletions(-)

diff --git a/drivers/usb/gadget/function/f_fs.c 
b/drivers/usb/gadget/function/f_fs.c
index 0294e4f18873..531a7206407b 100644
--- a/drivers/usb/gadget/function/f_fs.c
+++ b/drivers/usb/gadget/function/f_fs.c
@@ -26,7 +26,6 @@
 #include 
 #include 
 
-#include 
 #include 
 #include 
 #include 
@@ -1072,7 +1071,7 @@ ffs_epfile_open(struct inode *inode, struct file *file)
return 0;
 }
 
-static int ffs_aio_cancel(struct kiocb *kiocb)
+static int ffs_epfile_cancel_kiocb(struct kiocb *kiocb)
 {
struct ffs_io_data *io_data = kiocb->private;
struct ffs_epfile *epfile = kiocb->ki_filp->private_data;
@@ -1115,9 +1114,6 @@ static ssize_t ffs_epfile_write_iter(struct kiocb *kiocb, 
struct iov_iter *from)
 
kiocb->private = p;
 
-   if (p->aio)
-   kiocb_set_cancel_fn(kiocb, ffs_aio_cancel);
-
res = ffs_epfile_io(kiocb->ki_filp, p);
if (res == -EIOCBQUEUED)
return res;
@@ -1160,9 +1156,6 @@ static ssize_t ffs_epfile_read_iter(struct kiocb *kiocb, 
struct iov_iter *to)
 
kiocb->private = p;
 
-   if (p->aio)
-   kiocb_set_cancel_fn(kiocb, ffs_aio_cancel);
-
res = ffs_epfile_io(kiocb->ki_filp, p);
if (res == -EIOCBQUEUED)
return res;
@@ -1274,6 +1267,7 @@ static const struct file_operations ffs_epfile_operations 
= {
.read_iter =ffs_epfile_read_iter,
.release =  ffs_epfile_release,
.unlocked_ioctl =   ffs_epfile_ioctl,
+   .cancel_kiocb = ffs_epfile_cancel_kiocb,
 };
 
 
diff --git a/drivers/usb/gadget/legacy/inode.c 
b/drivers/usb/gadget/legacy/inode.c
index 37ca0e669bd8..02ea83c8861a 100644
--- a/drivers/usb/gadget/legacy/inode.c
+++ b/drivers/usb/gadget/legacy/inode.c
@@ -21,7 +21,6 @@
 #include 
 #include 
 #include 
-#include 
 #include 
 #include 
 #include 
@@ -435,7 +434,7 @@ struct kiocb_priv {
unsignedactual;
 };
 
-static int ep_aio_cancel(struct kiocb *iocb)
+static int ep_cancel_kiocb(struct kiocb *iocb)
 {
struct kiocb_priv   *priv = iocb->private;
struct ep_data  *epdata;
@@ -528,7 +527,6 @@ static ssize_t ep_aio(struct kiocb *iocb,
iocb->private = priv;
priv->iocb = iocb;
 
-   kiocb_set_cancel_fn(iocb, ep_aio_cancel);
get_ep(epdata);
priv->epdata = epdata;
priv->actual = 0;
@@ -700,6 +698,7 @@ static const struct file_operations ep_io_operations = {
.unlocked_ioctl = ep_ioctl,
.read_iter =ep_read_iter,
.write_iter =   ep_write_iter,
+   .cancel_kiocb = ep_cancel_kiocb,
 };
 
 /* ENDPOINT INITIALIZATION
diff --git a/fs/aio.c b/fs/aio.c
index 8991baa38d5d..be10dde20c8e 100644
--- a/fs/aio.c
+++ b/fs/aio.c
@@ -171,7 +171,6 @@ struct aio_kiocb {
};
 
struct kioctx   *ki_ctx;
-   kiocb_cancel_fn *ki_cancel;
 
struct iocb __user  *ki_user_iocb;  /* user's aiocb */
__u64   ki_user_data;   /* user's data for completion */
@@ -545,22 +544,6 @@ static int aio_setup_ring(struct kioctx *ctx, unsigned int 
nr_events)
 #define AIO_EVENTS_FIRST_PAGE  ((PAGE_SIZE - sizeof(struct aio_ring)) / 
sizeof(struct io_event))
 #define AIO_EVENTS_OFFSET  (AIO_EVENTS_PER_PAGE - AIO_EVENTS_FIRST_PAGE)
 
-void kiocb_set_cancel_fn(struct kiocb *iocb, kiocb_cancel_fn *cancel)
-{
-   struct aio_kiocb *req = container_of(iocb, struct aio_kiocb, rw);
-   struct kioctx *ctx = req->ki_ctx;
-   unsigned long flags;
-
-   if (WARN_ON_ONCE(!list_empty(>ki_list)))
-   return;
-
-   spin_lock_irqsave(>ctx_lock, flags);
-   list_add_tail(>ki_list, >active_reqs);
-   req->ki_cancel = cancel;
-   spin_unlock_irqrestore(>ctx_lock, flags);
-}
-EXPORT_SYMBOL(kiocb_set_cancel_fn);
-
 /*
  * free_ioctx() should be RCU delayed to synchronize against the RCU
  * protected lookup_ioctx() and also needs process context to call
@@ -608,7 +591,7 @@ static void free_ioctx_users(struct percpu_ref *ref)
req = list_first_entry(>active_reqs,
   struct aio_kiocb, ki_list);

[PATCH 08/32] aio: replace kiocb_set_cancel_fn with a cancel_kiocb file operation

2018-05-15 Thread Christoph Hellwig 
The current kiocb_set_cancel_fn implementation assumes the kiocb is
embedded into an aio_kiocb, which is fundamentally unsafe as it might
have been submitted by non-aio callers.  Instead add a cancel_kiocb
file operation that replaced the ki_cancel function pointer set by
kiocb_set_cancel_fn, and only adds iocbs to the active list when
the read/write_iter methods return -EIOCBQUEUED and the file has
a cancel_kiocb method.

Signed-off-by: Christoph Hellwig 
---
 drivers/usb/gadget/function/f_fs.c | 10 ++
 drivers/usb/gadget/legacy/inode.c  |  5 ++---
 fs/aio.c   | 31 --
 fs/orangefs/orangefs-kernel.h  |  1 -
 include/linux/aio.h|  7 ---
 include/linux/fs.h |  1 +
 6 files changed, 17 insertions(+), 38 deletions(-)

diff --git a/drivers/usb/gadget/function/f_fs.c 
b/drivers/usb/gadget/function/f_fs.c
index 0294e4f18873..531a7206407b 100644
--- a/drivers/usb/gadget/function/f_fs.c
+++ b/drivers/usb/gadget/function/f_fs.c
@@ -26,7 +26,6 @@
 #include 
 #include 
 
-#include 
 #include 
 #include 
 #include 
@@ -1072,7 +1071,7 @@ ffs_epfile_open(struct inode *inode, struct file *file)
return 0;
 }
 
-static int ffs_aio_cancel(struct kiocb *kiocb)
+static int ffs_epfile_cancel_kiocb(struct kiocb *kiocb)
 {
struct ffs_io_data *io_data = kiocb->private;
struct ffs_epfile *epfile = kiocb->ki_filp->private_data;
@@ -1115,9 +1114,6 @@ static ssize_t ffs_epfile_write_iter(struct kiocb *kiocb, 
struct iov_iter *from)
 
kiocb->private = p;
 
-   if (p->aio)
-   kiocb_set_cancel_fn(kiocb, ffs_aio_cancel);
-
res = ffs_epfile_io(kiocb->ki_filp, p);
if (res == -EIOCBQUEUED)
return res;
@@ -1160,9 +1156,6 @@ static ssize_t ffs_epfile_read_iter(struct kiocb *kiocb, 
struct iov_iter *to)
 
kiocb->private = p;
 
-   if (p->aio)
-   kiocb_set_cancel_fn(kiocb, ffs_aio_cancel);
-
res = ffs_epfile_io(kiocb->ki_filp, p);
if (res == -EIOCBQUEUED)
return res;
@@ -1274,6 +1267,7 @@ static const struct file_operations ffs_epfile_operations 
= {
.read_iter =ffs_epfile_read_iter,
.release =  ffs_epfile_release,
.unlocked_ioctl =   ffs_epfile_ioctl,
+   .cancel_kiocb = ffs_epfile_cancel_kiocb,
 };
 
 
diff --git a/drivers/usb/gadget/legacy/inode.c 
b/drivers/usb/gadget/legacy/inode.c
index 37ca0e669bd8..02ea83c8861a 100644
--- a/drivers/usb/gadget/legacy/inode.c
+++ b/drivers/usb/gadget/legacy/inode.c
@@ -21,7 +21,6 @@
 #include 
 #include 
 #include 
-#include 
 #include 
 #include 
 #include 
@@ -435,7 +434,7 @@ struct kiocb_priv {
unsignedactual;
 };
 
-static int ep_aio_cancel(struct kiocb *iocb)
+static int ep_cancel_kiocb(struct kiocb *iocb)
 {
struct kiocb_priv   *priv = iocb->private;
struct ep_data  *epdata;
@@ -528,7 +527,6 @@ static ssize_t ep_aio(struct kiocb *iocb,
iocb->private = priv;
priv->iocb = iocb;
 
-   kiocb_set_cancel_fn(iocb, ep_aio_cancel);
get_ep(epdata);
priv->epdata = epdata;
priv->actual = 0;
@@ -700,6 +698,7 @@ static const struct file_operations ep_io_operations = {
.unlocked_ioctl = ep_ioctl,
.read_iter =ep_read_iter,
.write_iter =   ep_write_iter,
+   .cancel_kiocb = ep_cancel_kiocb,
 };
 
 /* ENDPOINT INITIALIZATION
diff --git a/fs/aio.c b/fs/aio.c
index 8991baa38d5d..be10dde20c8e 100644
--- a/fs/aio.c
+++ b/fs/aio.c
@@ -171,7 +171,6 @@ struct aio_kiocb {
};
 
struct kioctx   *ki_ctx;
-   kiocb_cancel_fn *ki_cancel;
 
struct iocb __user  *ki_user_iocb;  /* user's aiocb */
__u64   ki_user_data;   /* user's data for completion */
@@ -545,22 +544,6 @@ static int aio_setup_ring(struct kioctx *ctx, unsigned int 
nr_events)
 #define AIO_EVENTS_FIRST_PAGE  ((PAGE_SIZE - sizeof(struct aio_ring)) / 
sizeof(struct io_event))
 #define AIO_EVENTS_OFFSET  (AIO_EVENTS_PER_PAGE - AIO_EVENTS_FIRST_PAGE)
 
-void kiocb_set_cancel_fn(struct kiocb *iocb, kiocb_cancel_fn *cancel)
-{
-   struct aio_kiocb *req = container_of(iocb, struct aio_kiocb, rw);
-   struct kioctx *ctx = req->ki_ctx;
-   unsigned long flags;
-
-   if (WARN_ON_ONCE(!list_empty(>ki_list)))
-   return;
-
-   spin_lock_irqsave(>ctx_lock, flags);
-   list_add_tail(>ki_list, >active_reqs);
-   req->ki_cancel = cancel;
-   spin_unlock_irqrestore(>ctx_lock, flags);
-}
-EXPORT_SYMBOL(kiocb_set_cancel_fn);
-
 /*
  * free_ioctx() should be RCU delayed to synchronize against the RCU
  * protected lookup_ioctx() and also needs process context to call
@@ -608,7 +591,7 @@ static void free_ioctx_users(struct percpu_ref *ref)
req = list_first_entry(>active_reqs,
   struct aio_kiocb, ki_list);

[PATCH 08/32] aio: replace kiocb_set_cancel_fn with a cancel_kiocb file operation

2018-05-11 Thread Christoph Hellwig 
The current kiocb_set_cancel_fn implementation assumes the kiocb is
embedded into an aio_kiocb, which is fundamentally unsafe as it might
have been submitted by non-aio callers.  Instead add a cancel_kiocb
file operation that replaced the ki_cancel function pointer set by
kiocb_set_cancel_fn, and only adds iocbs to the active list when
the read/write_iter methods return -EIOCBQUEUED and the file has
a cancel_kiocb method.

Signed-off-by: Christoph Hellwig 
---
 drivers/usb/gadget/function/f_fs.c | 10 ++
 drivers/usb/gadget/legacy/inode.c  |  5 ++---
 fs/aio.c   | 31 --
 fs/orangefs/orangefs-kernel.h  |  1 -
 include/linux/aio.h|  7 ---
 include/linux/fs.h |  1 +
 6 files changed, 17 insertions(+), 38 deletions(-)

diff --git a/drivers/usb/gadget/function/f_fs.c 
b/drivers/usb/gadget/function/f_fs.c
index 0294e4f18873..531a7206407b 100644
--- a/drivers/usb/gadget/function/f_fs.c
+++ b/drivers/usb/gadget/function/f_fs.c
@@ -26,7 +26,6 @@
 #include 
 #include 
 
-#include 
 #include 
 #include 
 #include 
@@ -1072,7 +1071,7 @@ ffs_epfile_open(struct inode *inode, struct file *file)
return 0;
 }
 
-static int ffs_aio_cancel(struct kiocb *kiocb)
+static int ffs_epfile_cancel_kiocb(struct kiocb *kiocb)
 {
struct ffs_io_data *io_data = kiocb->private;
struct ffs_epfile *epfile = kiocb->ki_filp->private_data;
@@ -1115,9 +1114,6 @@ static ssize_t ffs_epfile_write_iter(struct kiocb *kiocb, 
struct iov_iter *from)
 
kiocb->private = p;
 
-   if (p->aio)
-   kiocb_set_cancel_fn(kiocb, ffs_aio_cancel);
-
res = ffs_epfile_io(kiocb->ki_filp, p);
if (res == -EIOCBQUEUED)
return res;
@@ -1160,9 +1156,6 @@ static ssize_t ffs_epfile_read_iter(struct kiocb *kiocb, 
struct iov_iter *to)
 
kiocb->private = p;
 
-   if (p->aio)
-   kiocb_set_cancel_fn(kiocb, ffs_aio_cancel);
-
res = ffs_epfile_io(kiocb->ki_filp, p);
if (res == -EIOCBQUEUED)
return res;
@@ -1274,6 +1267,7 @@ static const struct file_operations ffs_epfile_operations 
= {
.read_iter =ffs_epfile_read_iter,
.release =  ffs_epfile_release,
.unlocked_ioctl =   ffs_epfile_ioctl,
+   .cancel_kiocb = ffs_epfile_cancel_kiocb,
 };
 
 
diff --git a/drivers/usb/gadget/legacy/inode.c 
b/drivers/usb/gadget/legacy/inode.c
index 37ca0e669bd8..02ea83c8861a 100644
--- a/drivers/usb/gadget/legacy/inode.c
+++ b/drivers/usb/gadget/legacy/inode.c
@@ -21,7 +21,6 @@
 #include 
 #include 
 #include 
-#include 
 #include 
 #include 
 #include 
@@ -435,7 +434,7 @@ struct kiocb_priv {
unsignedactual;
 };
 
-static int ep_aio_cancel(struct kiocb *iocb)
+static int ep_cancel_kiocb(struct kiocb *iocb)
 {
struct kiocb_priv   *priv = iocb->private;
struct ep_data  *epdata;
@@ -528,7 +527,6 @@ static ssize_t ep_aio(struct kiocb *iocb,
iocb->private = priv;
priv->iocb = iocb;
 
-   kiocb_set_cancel_fn(iocb, ep_aio_cancel);
get_ep(epdata);
priv->epdata = epdata;
priv->actual = 0;
@@ -700,6 +698,7 @@ static const struct file_operations ep_io_operations = {
.unlocked_ioctl = ep_ioctl,
.read_iter =ep_read_iter,
.write_iter =   ep_write_iter,
+   .cancel_kiocb = ep_cancel_kiocb,
 };
 
 /* ENDPOINT INITIALIZATION
diff --git a/fs/aio.c b/fs/aio.c
index 8991baa38d5d..be10dde20c8e 100644
--- a/fs/aio.c
+++ b/fs/aio.c
@@ -171,7 +171,6 @@ struct aio_kiocb {
};
 
struct kioctx   *ki_ctx;
-   kiocb_cancel_fn *ki_cancel;
 
struct iocb __user  *ki_user_iocb;  /* user's aiocb */
__u64   ki_user_data;   /* user's data for completion */
@@ -545,22 +544,6 @@ static int aio_setup_ring(struct kioctx *ctx, unsigned int 
nr_events)
 #define AIO_EVENTS_FIRST_PAGE  ((PAGE_SIZE - sizeof(struct aio_ring)) / 
sizeof(struct io_event))
 #define AIO_EVENTS_OFFSET  (AIO_EVENTS_PER_PAGE - AIO_EVENTS_FIRST_PAGE)
 
-void kiocb_set_cancel_fn(struct kiocb *iocb, kiocb_cancel_fn *cancel)
-{
-   struct aio_kiocb *req = container_of(iocb, struct aio_kiocb, rw);
-   struct kioctx *ctx = req->ki_ctx;
-   unsigned long flags;
-
-   if (WARN_ON_ONCE(!list_empty(>ki_list)))
-   return;
-
-   spin_lock_irqsave(>ctx_lock, flags);
-   list_add_tail(>ki_list, >active_reqs);
-   req->ki_cancel = cancel;
-   spin_unlock_irqrestore(>ctx_lock, flags);
-}
-EXPORT_SYMBOL(kiocb_set_cancel_fn);
-
 /*
  * free_ioctx() should be RCU delayed to synchronize against the RCU
  * protected lookup_ioctx() and also needs process context to call
@@ -608,7 +591,7 @@ static void free_ioctx_users(struct percpu_ref *ref)
req = list_first_entry(>active_reqs,
   struct aio_kiocb, ki_list);

[PATCH 08/32] aio: replace kiocb_set_cancel_fn with a cancel_kiocb file operation

2018-05-11 Thread Christoph Hellwig 
The current kiocb_set_cancel_fn implementation assumes the kiocb is
embedded into an aio_kiocb, which is fundamentally unsafe as it might
have been submitted by non-aio callers.  Instead add a cancel_kiocb
file operation that replaced the ki_cancel function pointer set by
kiocb_set_cancel_fn, and only adds iocbs to the active list when
the read/write_iter methods return -EIOCBQUEUED and the file has
a cancel_kiocb method.

Signed-off-by: Christoph Hellwig 
---
 drivers/usb/gadget/function/f_fs.c | 10 ++
 drivers/usb/gadget/legacy/inode.c  |  5 ++---
 fs/aio.c   | 31 --
 fs/orangefs/orangefs-kernel.h  |  1 -
 include/linux/aio.h|  7 ---
 include/linux/fs.h |  1 +
 6 files changed, 17 insertions(+), 38 deletions(-)

diff --git a/drivers/usb/gadget/function/f_fs.c 
b/drivers/usb/gadget/function/f_fs.c
index 0294e4f18873..531a7206407b 100644
--- a/drivers/usb/gadget/function/f_fs.c
+++ b/drivers/usb/gadget/function/f_fs.c
@@ -26,7 +26,6 @@
 #include 
 #include 
 
-#include 
 #include 
 #include 
 #include 
@@ -1072,7 +1071,7 @@ ffs_epfile_open(struct inode *inode, struct file *file)
return 0;
 }
 
-static int ffs_aio_cancel(struct kiocb *kiocb)
+static int ffs_epfile_cancel_kiocb(struct kiocb *kiocb)
 {
struct ffs_io_data *io_data = kiocb->private;
struct ffs_epfile *epfile = kiocb->ki_filp->private_data;
@@ -1115,9 +1114,6 @@ static ssize_t ffs_epfile_write_iter(struct kiocb *kiocb, 
struct iov_iter *from)
 
kiocb->private = p;
 
-   if (p->aio)
-   kiocb_set_cancel_fn(kiocb, ffs_aio_cancel);
-
res = ffs_epfile_io(kiocb->ki_filp, p);
if (res == -EIOCBQUEUED)
return res;
@@ -1160,9 +1156,6 @@ static ssize_t ffs_epfile_read_iter(struct kiocb *kiocb, 
struct iov_iter *to)
 
kiocb->private = p;
 
-   if (p->aio)
-   kiocb_set_cancel_fn(kiocb, ffs_aio_cancel);
-
res = ffs_epfile_io(kiocb->ki_filp, p);
if (res == -EIOCBQUEUED)
return res;
@@ -1274,6 +1267,7 @@ static const struct file_operations ffs_epfile_operations 
= {
.read_iter =ffs_epfile_read_iter,
.release =  ffs_epfile_release,
.unlocked_ioctl =   ffs_epfile_ioctl,
+   .cancel_kiocb = ffs_epfile_cancel_kiocb,
 };
 
 
diff --git a/drivers/usb/gadget/legacy/inode.c 
b/drivers/usb/gadget/legacy/inode.c
index 37ca0e669bd8..02ea83c8861a 100644
--- a/drivers/usb/gadget/legacy/inode.c
+++ b/drivers/usb/gadget/legacy/inode.c
@@ -21,7 +21,6 @@
 #include 
 #include 
 #include 
-#include 
 #include 
 #include 
 #include 
@@ -435,7 +434,7 @@ struct kiocb_priv {
unsignedactual;
 };
 
-static int ep_aio_cancel(struct kiocb *iocb)
+static int ep_cancel_kiocb(struct kiocb *iocb)
 {
struct kiocb_priv   *priv = iocb->private;
struct ep_data  *epdata;
@@ -528,7 +527,6 @@ static ssize_t ep_aio(struct kiocb *iocb,
iocb->private = priv;
priv->iocb = iocb;
 
-   kiocb_set_cancel_fn(iocb, ep_aio_cancel);
get_ep(epdata);
priv->epdata = epdata;
priv->actual = 0;
@@ -700,6 +698,7 @@ static const struct file_operations ep_io_operations = {
.unlocked_ioctl = ep_ioctl,
.read_iter =ep_read_iter,
.write_iter =   ep_write_iter,
+   .cancel_kiocb = ep_cancel_kiocb,
 };
 
 /* ENDPOINT INITIALIZATION
diff --git a/fs/aio.c b/fs/aio.c
index 8991baa38d5d..be10dde20c8e 100644
--- a/fs/aio.c
+++ b/fs/aio.c
@@ -171,7 +171,6 @@ struct aio_kiocb {
};
 
struct kioctx   *ki_ctx;
-   kiocb_cancel_fn *ki_cancel;
 
struct iocb __user  *ki_user_iocb;  /* user's aiocb */
__u64   ki_user_data;   /* user's data for completion */
@@ -545,22 +544,6 @@ static int aio_setup_ring(struct kioctx *ctx, unsigned int 
nr_events)
 #define AIO_EVENTS_FIRST_PAGE  ((PAGE_SIZE - sizeof(struct aio_ring)) / 
sizeof(struct io_event))
 #define AIO_EVENTS_OFFSET  (AIO_EVENTS_PER_PAGE - AIO_EVENTS_FIRST_PAGE)
 
-void kiocb_set_cancel_fn(struct kiocb *iocb, kiocb_cancel_fn *cancel)
-{
-   struct aio_kiocb *req = container_of(iocb, struct aio_kiocb, rw);
-   struct kioctx *ctx = req->ki_ctx;
-   unsigned long flags;
-
-   if (WARN_ON_ONCE(!list_empty(>ki_list)))
-   return;
-
-   spin_lock_irqsave(>ctx_lock, flags);
-   list_add_tail(>ki_list, >active_reqs);
-   req->ki_cancel = cancel;
-   spin_unlock_irqrestore(>ctx_lock, flags);
-}
-EXPORT_SYMBOL(kiocb_set_cancel_fn);
-
 /*
  * free_ioctx() should be RCU delayed to synchronize against the RCU
  * protected lookup_ioctx() and also needs process context to call
@@ -608,7 +591,7 @@ static void free_ioctx_users(struct percpu_ref *ref)
req = list_first_entry(>active_reqs,
   struct aio_kiocb, ki_list);

Re: [PATCH 08/32] aio: replace kiocb_set_cancel_fn with a cancel_kiocb file operation

2018-04-05 Thread Al Viro 
On Fri, Mar 30, 2018 at 05:07:45PM +0200, Christoph Hellwig wrote:
> The current kiocb_set_cancel_fn implementation assumes the kiocb is
> embedded into an aio_kiocb, which is fundamentally unsafe as it might
> have been submitted by non-aio callers.  Instead add a cancel_kiocb
> file operation that replaced the ki_cancel function pointer set by
> kiocb_set_cancel_fn, and only adds iocbs to the active list when
> the read/write_iter methods return -EIOCBQUEUED and the file has
> a cancel_kiocb method.

> @@ -1440,6 +1423,16 @@ static inline ssize_t aio_rw_ret(struct kiocb *req, 
> ssize_t ret)
>  {
>   switch (ret) {
>   case -EIOCBQUEUED:
> + if (req->ki_filp->f_op->cancel_kiocb) {

... and by that point req might've been already freed by IO completion on
another CPU.
> + struct aio_kiocb *iocb =
> + container_of(req, struct aio_kiocb, rw);
> + struct kioctx *ctx = iocb->ki_ctx;
> + unsigned long flags;
> +
> + spin_lock_irqsave(>ctx_lock, flags);
> + list_add_tail(>ki_list, >active_reqs);

Re: [PATCH 08/32] aio: replace kiocb_set_cancel_fn with a cancel_kiocb file operation

2018-04-05 Thread Al Viro 
On Fri, Mar 30, 2018 at 05:07:45PM +0200, Christoph Hellwig wrote:
> The current kiocb_set_cancel_fn implementation assumes the kiocb is
> embedded into an aio_kiocb, which is fundamentally unsafe as it might
> have been submitted by non-aio callers.  Instead add a cancel_kiocb
> file operation that replaced the ki_cancel function pointer set by
> kiocb_set_cancel_fn, and only adds iocbs to the active list when
> the read/write_iter methods return -EIOCBQUEUED and the file has
> a cancel_kiocb method.

> @@ -1440,6 +1423,16 @@ static inline ssize_t aio_rw_ret(struct kiocb *req, 
> ssize_t ret)
>  {
>   switch (ret) {
>   case -EIOCBQUEUED:
> + if (req->ki_filp->f_op->cancel_kiocb) {

... and by that point req might've been already freed by IO completion on
another CPU.
> + struct aio_kiocb *iocb =
> + container_of(req, struct aio_kiocb, rw);
> + struct kioctx *ctx = iocb->ki_ctx;
> + unsigned long flags;
> +
> + spin_lock_irqsave(>ctx_lock, flags);
> + list_add_tail(>ki_list, >active_reqs);

[PATCH 08/32] aio: replace kiocb_set_cancel_fn with a cancel_kiocb file operation

2018-03-30 Thread Christoph Hellwig 
The current kiocb_set_cancel_fn implementation assumes the kiocb is
embedded into an aio_kiocb, which is fundamentally unsafe as it might
have been submitted by non-aio callers.  Instead add a cancel_kiocb
file operation that replaced the ki_cancel function pointer set by
kiocb_set_cancel_fn, and only adds iocbs to the active list when
the read/write_iter methods return -EIOCBQUEUED and the file has
a cancel_kiocb method.

Signed-off-by: Christoph Hellwig 
---
 drivers/usb/gadget/function/f_fs.c | 10 ++
 drivers/usb/gadget/legacy/inode.c  |  5 ++---
 fs/aio.c   | 31 ---
 fs/orangefs/orangefs-kernel.h  |  1 -
 include/linux/aio.h|  7 ---
 include/linux/fs.h |  1 +
 6 files changed, 17 insertions(+), 38 deletions(-)

diff --git a/drivers/usb/gadget/function/f_fs.c 
b/drivers/usb/gadget/function/f_fs.c
index c2592d883f67..4dcf21ae39f8 100644
--- a/drivers/usb/gadget/function/f_fs.c
+++ b/drivers/usb/gadget/function/f_fs.c
@@ -26,7 +26,6 @@
 #include 
 #include 
 
-#include 
 #include 
 #include 
 #include 
@@ -1068,7 +1067,7 @@ ffs_epfile_open(struct inode *inode, struct file *file)
return 0;
 }
 
-static int ffs_aio_cancel(struct kiocb *kiocb)
+static int ffs_epfile_cancel_kiocb(struct kiocb *kiocb)
 {
struct ffs_io_data *io_data = kiocb->private;
struct ffs_epfile *epfile = kiocb->ki_filp->private_data;
@@ -,9 +1110,6 @@ static ssize_t ffs_epfile_write_iter(struct kiocb *kiocb, 
struct iov_iter *from)
 
kiocb->private = p;
 
-   if (p->aio)
-   kiocb_set_cancel_fn(kiocb, ffs_aio_cancel);
-
res = ffs_epfile_io(kiocb->ki_filp, p);
if (res == -EIOCBQUEUED)
return res;
@@ -1156,9 +1152,6 @@ static ssize_t ffs_epfile_read_iter(struct kiocb *kiocb, 
struct iov_iter *to)
 
kiocb->private = p;
 
-   if (p->aio)
-   kiocb_set_cancel_fn(kiocb, ffs_aio_cancel);
-
res = ffs_epfile_io(kiocb->ki_filp, p);
if (res == -EIOCBQUEUED)
return res;
@@ -1270,6 +1263,7 @@ static const struct file_operations ffs_epfile_operations 
= {
.read_iter =ffs_epfile_read_iter,
.release =  ffs_epfile_release,
.unlocked_ioctl =   ffs_epfile_ioctl,
+   .cancel_kiocb = ffs_epfile_cancel_kiocb,
 };
 
 
diff --git a/drivers/usb/gadget/legacy/inode.c 
b/drivers/usb/gadget/legacy/inode.c
index 37ca0e669bd8..02ea83c8861a 100644
--- a/drivers/usb/gadget/legacy/inode.c
+++ b/drivers/usb/gadget/legacy/inode.c
@@ -21,7 +21,6 @@
 #include 
 #include 
 #include 
-#include 
 #include 
 #include 
 #include 
@@ -435,7 +434,7 @@ struct kiocb_priv {
unsignedactual;
 };
 
-static int ep_aio_cancel(struct kiocb *iocb)
+static int ep_cancel_kiocb(struct kiocb *iocb)
 {
struct kiocb_priv   *priv = iocb->private;
struct ep_data  *epdata;
@@ -528,7 +527,6 @@ static ssize_t ep_aio(struct kiocb *iocb,
iocb->private = priv;
priv->iocb = iocb;
 
-   kiocb_set_cancel_fn(iocb, ep_aio_cancel);
get_ep(epdata);
priv->epdata = epdata;
priv->actual = 0;
@@ -700,6 +698,7 @@ static const struct file_operations ep_io_operations = {
.unlocked_ioctl = ep_ioctl,
.read_iter =ep_read_iter,
.write_iter =   ep_write_iter,
+   .cancel_kiocb = ep_cancel_kiocb,
 };
 
 /* ENDPOINT INITIALIZATION
diff --git a/fs/aio.c b/fs/aio.c
index 5a5d537cec2e..7e4b517c60c3 100644
--- a/fs/aio.c
+++ b/fs/aio.c
@@ -171,7 +171,6 @@ struct aio_kiocb {
};
 
struct kioctx   *ki_ctx;
-   kiocb_cancel_fn *ki_cancel;
 
struct iocb __user  *ki_user_iocb;  /* user's aiocb */
__u64   ki_user_data;   /* user's data for completion */
@@ -545,22 +544,6 @@ static int aio_setup_ring(struct kioctx *ctx, unsigned int 
nr_events)
 #define AIO_EVENTS_FIRST_PAGE  ((PAGE_SIZE - sizeof(struct aio_ring)) / 
sizeof(struct io_event))
 #define AIO_EVENTS_OFFSET  (AIO_EVENTS_PER_PAGE - AIO_EVENTS_FIRST_PAGE)
 
-void kiocb_set_cancel_fn(struct kiocb *iocb, kiocb_cancel_fn *cancel)
-{
-   struct aio_kiocb *req = container_of(iocb, struct aio_kiocb, rw);
-   struct kioctx *ctx = req->ki_ctx;
-   unsigned long flags;
-
-   if (WARN_ON_ONCE(!list_empty(>ki_list)))
-   return;
-
-   spin_lock_irqsave(>ctx_lock, flags);
-   list_add_tail(>ki_list, >active_reqs);
-   req->ki_cancel = cancel;
-   spin_unlock_irqrestore(>ctx_lock, flags);
-}
-EXPORT_SYMBOL(kiocb_set_cancel_fn);
-
 static void free_ioctx(struct work_struct *work)
 {
struct kioctx *ctx = container_of(work, struct kioctx, free_work);
@@ -602,7 +585,7 @@ static void free_ioctx_users(struct percpu_ref *ref)
req = list_first_entry(>active_reqs,
   struct aio_kiocb, ki_list);

[PATCH 08/32] aio: replace kiocb_set_cancel_fn with a cancel_kiocb file operation

2018-03-30 Thread Christoph Hellwig 
The current kiocb_set_cancel_fn implementation assumes the kiocb is
embedded into an aio_kiocb, which is fundamentally unsafe as it might
have been submitted by non-aio callers.  Instead add a cancel_kiocb
file operation that replaced the ki_cancel function pointer set by
kiocb_set_cancel_fn, and only adds iocbs to the active list when
the read/write_iter methods return -EIOCBQUEUED and the file has
a cancel_kiocb method.

Signed-off-by: Christoph Hellwig 
---
 drivers/usb/gadget/function/f_fs.c | 10 ++
 drivers/usb/gadget/legacy/inode.c  |  5 ++---
 fs/aio.c   | 31 ---
 fs/orangefs/orangefs-kernel.h  |  1 -
 include/linux/aio.h|  7 ---
 include/linux/fs.h |  1 +
 6 files changed, 17 insertions(+), 38 deletions(-)

diff --git a/drivers/usb/gadget/function/f_fs.c 
b/drivers/usb/gadget/function/f_fs.c
index c2592d883f67..4dcf21ae39f8 100644
--- a/drivers/usb/gadget/function/f_fs.c
+++ b/drivers/usb/gadget/function/f_fs.c
@@ -26,7 +26,6 @@
 #include 
 #include 
 
-#include 
 #include 
 #include 
 #include 
@@ -1068,7 +1067,7 @@ ffs_epfile_open(struct inode *inode, struct file *file)
return 0;
 }
 
-static int ffs_aio_cancel(struct kiocb *kiocb)
+static int ffs_epfile_cancel_kiocb(struct kiocb *kiocb)
 {
struct ffs_io_data *io_data = kiocb->private;
struct ffs_epfile *epfile = kiocb->ki_filp->private_data;
@@ -,9 +1110,6 @@ static ssize_t ffs_epfile_write_iter(struct kiocb *kiocb, 
struct iov_iter *from)
 
kiocb->private = p;
 
-   if (p->aio)
-   kiocb_set_cancel_fn(kiocb, ffs_aio_cancel);
-
res = ffs_epfile_io(kiocb->ki_filp, p);
if (res == -EIOCBQUEUED)
return res;
@@ -1156,9 +1152,6 @@ static ssize_t ffs_epfile_read_iter(struct kiocb *kiocb, 
struct iov_iter *to)
 
kiocb->private = p;
 
-   if (p->aio)
-   kiocb_set_cancel_fn(kiocb, ffs_aio_cancel);
-
res = ffs_epfile_io(kiocb->ki_filp, p);
if (res == -EIOCBQUEUED)
return res;
@@ -1270,6 +1263,7 @@ static const struct file_operations ffs_epfile_operations 
= {
.read_iter =ffs_epfile_read_iter,
.release =  ffs_epfile_release,
.unlocked_ioctl =   ffs_epfile_ioctl,
+   .cancel_kiocb = ffs_epfile_cancel_kiocb,
 };
 
 
diff --git a/drivers/usb/gadget/legacy/inode.c 
b/drivers/usb/gadget/legacy/inode.c
index 37ca0e669bd8..02ea83c8861a 100644
--- a/drivers/usb/gadget/legacy/inode.c
+++ b/drivers/usb/gadget/legacy/inode.c
@@ -21,7 +21,6 @@
 #include 
 #include 
 #include 
-#include 
 #include 
 #include 
 #include 
@@ -435,7 +434,7 @@ struct kiocb_priv {
unsignedactual;
 };
 
-static int ep_aio_cancel(struct kiocb *iocb)
+static int ep_cancel_kiocb(struct kiocb *iocb)
 {
struct kiocb_priv   *priv = iocb->private;
struct ep_data  *epdata;
@@ -528,7 +527,6 @@ static ssize_t ep_aio(struct kiocb *iocb,
iocb->private = priv;
priv->iocb = iocb;
 
-   kiocb_set_cancel_fn(iocb, ep_aio_cancel);
get_ep(epdata);
priv->epdata = epdata;
priv->actual = 0;
@@ -700,6 +698,7 @@ static const struct file_operations ep_io_operations = {
.unlocked_ioctl = ep_ioctl,
.read_iter =ep_read_iter,
.write_iter =   ep_write_iter,
+   .cancel_kiocb = ep_cancel_kiocb,
 };
 
 /* ENDPOINT INITIALIZATION
diff --git a/fs/aio.c b/fs/aio.c
index 5a5d537cec2e..7e4b517c60c3 100644
--- a/fs/aio.c
+++ b/fs/aio.c
@@ -171,7 +171,6 @@ struct aio_kiocb {
};
 
struct kioctx   *ki_ctx;
-   kiocb_cancel_fn *ki_cancel;
 
struct iocb __user  *ki_user_iocb;  /* user's aiocb */
__u64   ki_user_data;   /* user's data for completion */
@@ -545,22 +544,6 @@ static int aio_setup_ring(struct kioctx *ctx, unsigned int 
nr_events)
 #define AIO_EVENTS_FIRST_PAGE  ((PAGE_SIZE - sizeof(struct aio_ring)) / 
sizeof(struct io_event))
 #define AIO_EVENTS_OFFSET  (AIO_EVENTS_PER_PAGE - AIO_EVENTS_FIRST_PAGE)
 
-void kiocb_set_cancel_fn(struct kiocb *iocb, kiocb_cancel_fn *cancel)
-{
-   struct aio_kiocb *req = container_of(iocb, struct aio_kiocb, rw);
-   struct kioctx *ctx = req->ki_ctx;
-   unsigned long flags;
-
-   if (WARN_ON_ONCE(!list_empty(>ki_list)))
-   return;
-
-   spin_lock_irqsave(>ctx_lock, flags);
-   list_add_tail(>ki_list, >active_reqs);
-   req->ki_cancel = cancel;
-   spin_unlock_irqrestore(>ctx_lock, flags);
-}
-EXPORT_SYMBOL(kiocb_set_cancel_fn);
-
 static void free_ioctx(struct work_struct *work)
 {
struct kioctx *ctx = container_of(work, struct kioctx, free_work);
@@ -602,7 +585,7 @@ static void free_ioctx_users(struct percpu_ref *ref)
req = list_first_entry(>active_reqs,
   struct aio_kiocb, ki_list);