Skip to content

Commit 24729b3

Browse files
wcheng-enggregkh
authored andcommitted
usb: gadget: f_fs: Fix race between aio_cancel() and AIO request complete
FFS based applications can utilize the aio_cancel() callback to dequeue pending USB requests submitted to the UDC. There is a scenario where the FFS application issues an AIO cancel call, while the UDC is handling a soft disconnect. For a DWC3 based implementation, the callstack looks like the following: DWC3 Gadget FFS Application dwc3_gadget_soft_disconnect() ... --> dwc3_stop_active_transfers() --> dwc3_gadget_giveback(-ESHUTDOWN) --> ffs_epfile_async_io_complete() ffs_aio_cancel() --> usb_ep_free_request() --> usb_ep_dequeue() There is currently no locking implemented between the AIO completion handler and AIO cancel, so the issue occurs if the completion routine is running in parallel to an AIO cancel call coming from the FFS application. As the completion call frees the USB request (io_data->req) the FFS application is also referencing it for the usb_ep_dequeue() call. This can lead to accessing a stale/hanging pointer. commit b566d38 ("usb: gadget: f_fs: use io_data->status consistently") relocated the usb_ep_free_request() into ffs_epfile_async_io_complete(). However, in order to properly implement locking to mitigate this issue, the spinlock can't be added to ffs_epfile_async_io_complete(), as usb_ep_dequeue() (if successfully dequeuing a USB request) will call the function driver's completion handler in the same context. Hence, leading into a deadlock. Fix this issue by moving the usb_ep_free_request() back to ffs_user_copy_worker(), and ensuring that it explicitly sets io_data->req to NULL after freeing it within the ffs->eps_lock. This resolves the race condition above, as the ffs_aio_cancel() routine will not continue attempting to dequeue a request that has already been freed, or the ffs_user_copy_work() not freeing the USB request until the AIO cancel is done referencing it. This fix depends on commit b566d38 ("usb: gadget: f_fs: use io_data->status consistently") Fixes: 2e4c755 ("usb: gadget: f_fs: add aio support") Cc: stable <[email protected]> # b566d38 ("usb: gadget: f_fs: use io_data->status consistently") Signed-off-by: Wesley Cheng <[email protected]> Link: https://lore.kernel.org/r/[email protected] Signed-off-by: Greg Kroah-Hartman <[email protected]>
1 parent ed30a4a commit 24729b3

File tree

1 file changed

+6
-1
lines changed
  • drivers/usb/gadget/function

1 file changed

+6
-1
lines changed

drivers/usb/gadget/function/f_fs.c

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -852,6 +852,7 @@ static void ffs_user_copy_worker(struct work_struct *work)
852852
work);
853853
int ret = io_data->status;
854854
bool kiocb_has_eventfd = io_data->kiocb->ki_flags & IOCB_EVENTFD;
855+
unsigned long flags;
855856

856857
if (io_data->read && ret > 0) {
857858
kthread_use_mm(io_data->mm);
@@ -864,6 +865,11 @@ static void ffs_user_copy_worker(struct work_struct *work)
864865
if (io_data->ffs->ffs_eventfd && !kiocb_has_eventfd)
865866
eventfd_signal(io_data->ffs->ffs_eventfd);
866867

868+
spin_lock_irqsave(&io_data->ffs->eps_lock, flags);
869+
usb_ep_free_request(io_data->ep, io_data->req);
870+
io_data->req = NULL;
871+
spin_unlock_irqrestore(&io_data->ffs->eps_lock, flags);
872+
867873
if (io_data->read)
868874
kfree(io_data->to_free);
869875
ffs_free_buffer(io_data);
@@ -877,7 +883,6 @@ static void ffs_epfile_async_io_complete(struct usb_ep *_ep,
877883
struct ffs_data *ffs = io_data->ffs;
878884

879885
io_data->status = req->status ? req->status : req->actual;
880-
usb_ep_free_request(_ep, req);
881886

882887
INIT_WORK(&io_data->work, ffs_user_copy_worker);
883888
queue_work(ffs->io_completion_wq, &io_data->work);

0 commit comments

Comments
 (0)