usb: gadget: f_fs: change ep->status safe in ffs_epfile_io()
authorLinyu Yuan <quic_linyyuan@quicinc.com>
Fri, 10 Jun 2022 12:17:57 +0000 (20:17 +0800)
committerGreg Kroah-Hartman <gregkh@linuxfoundation.org>
Fri, 10 Jun 2022 12:45:38 +0000 (14:45 +0200)
If a task read/write data in blocking mode, it will wait the completion
in ffs_epfile_io(), if function unbind occurs, ffs_func_unbind() will
kfree ffs ep, once the task wake up, it still dereference the ffs ep to
obtain the request status.

Fix it by moving the request status to io_data which is stack-safe.

Cc: <stable@vger.kernel.org> # 5.15
Reported-by: Michael Wu <michael@allwinnertech.com>
Tested-by: Michael Wu <michael@allwinnertech.com>
Reviewed-by: John Keeping <john@metanate.com>
Signed-off-by: Linyu Yuan <quic_linyyuan@quicinc.com>
Link: https://lore.kernel.org/r/1654863478-26228-2-git-send-email-quic_linyyuan@quicinc.com
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
drivers/usb/gadget/function/f_fs.c

index 4585ee3..e1fcd8b 100644 (file)
@@ -122,8 +122,6 @@ struct ffs_ep {
        struct usb_endpoint_descriptor  *descs[3];
 
        u8                              num;
-
-       int                             status; /* P: epfile->mutex */
 };
 
 struct ffs_epfile {
@@ -227,6 +225,9 @@ struct ffs_io_data {
        bool use_sg;
 
        struct ffs_data *ffs;
+
+       int status;
+       struct completion done;
 };
 
 struct ffs_desc_helper {
@@ -707,12 +708,15 @@ static const struct file_operations ffs_ep0_operations = {
 
 static void ffs_epfile_io_complete(struct usb_ep *_ep, struct usb_request *req)
 {
+       struct ffs_io_data *io_data = req->context;
+
        ENTER();
-       if (req->context) {
-               struct ffs_ep *ep = _ep->driver_data;
-               ep->status = req->status ? req->status : req->actual;
-               complete(req->context);
-       }
+       if (req->status)
+               io_data->status = req->status;
+       else
+               io_data->status = req->actual;
+
+       complete(&io_data->done);
 }
 
 static ssize_t ffs_copy_to_iter(void *data, int data_len, struct iov_iter *iter)
@@ -1050,7 +1054,6 @@ static ssize_t ffs_epfile_io(struct file *file, struct ffs_io_data *io_data)
                WARN(1, "%s: data_len == -EINVAL\n", __func__);
                ret = -EINVAL;
        } else if (!io_data->aio) {
-               DECLARE_COMPLETION_ONSTACK(done);
                bool interrupted = false;
 
                req = ep->req;
@@ -1066,7 +1069,8 @@ static ssize_t ffs_epfile_io(struct file *file, struct ffs_io_data *io_data)
 
                io_data->buf = data;
 
-               req->context  = &done;
+               init_completion(&io_data->done);
+               req->context  = io_data;
                req->complete = ffs_epfile_io_complete;
 
                ret = usb_ep_queue(ep->ep, req, GFP_ATOMIC);
@@ -1075,7 +1079,7 @@ static ssize_t ffs_epfile_io(struct file *file, struct ffs_io_data *io_data)
 
                spin_unlock_irq(&epfile->ffs->eps_lock);
 
-               if (wait_for_completion_interruptible(&done)) {
+               if (wait_for_completion_interruptible(&io_data->done)) {
                        /*
                         * To avoid race condition with ffs_epfile_io_complete,
                         * dequeue the request first then check
@@ -1083,17 +1087,17 @@ static ssize_t ffs_epfile_io(struct file *file, struct ffs_io_data *io_data)
                         * condition with req->complete callback.
                         */
                        usb_ep_dequeue(ep->ep, req);
-                       wait_for_completion(&done);
-                       interrupted = ep->status < 0;
+                       wait_for_completion(&io_data->done);
+                       interrupted = io_data->status < 0;
                }
 
                if (interrupted)
                        ret = -EINTR;
-               else if (io_data->read && ep->status > 0)
-                       ret = __ffs_epfile_read_data(epfile, data, ep->status,
+               else if (io_data->read && io_data->status > 0)
+                       ret = __ffs_epfile_read_data(epfile, data, io_data->status,
                                                     &io_data->data);
                else
-                       ret = ep->status;
+                       ret = io_data->status;
                goto error_mutex;
        } else if (!(req = usb_ep_alloc_request(ep->ep, GFP_ATOMIC))) {
                ret = -ENOMEM;