From f431a2bae3b735da98f469a81e27ab0034068d7b Mon Sep 17 00:00:00 2001 From: Udipto Goswami Date: Wed, 16 Jun 2021 19:09:44 +0530 Subject: [PATCH] usb: f_fs: Prevent race between ep0_release & reset_work When ffs_func_disable is called the driver will call set_alt as part of which if FFS_DEACTIVATED is set then it schedules reset_work. It further goes and tries to call epfile_destroy where kfree is done. If within the same time, if ep0_release is also called, it will also go forward and call epfiles_destroy. This is because although the driver did kfree, but did't mark NULL, which is why the if check for epfile equals NULL will fail to prevent and will still be able to proceed. At this point the epfile instance is corrupted therefore when in epfile_destroy it goes into the if check which will trigger the BUG_ON check causing crash. Following is the illustration: CPU1 CPU2 ffs_ep0_release ffs_data_closed ffs->state = FFS_DEACTIVATED (atomic context) ffs_func_disable ffs_func_set_alt schedule_work(&ffs->reset_work) ffs_epfiles_destroy(ffs->epfiles) (running for loop not finished) ffs_reset_work (preempts) ffs_data_reset ffs_data_clear ffs_epfiles_destroy(ffs->epfiles) Fix this by protecting the epfile_destroy calls with mutex_lock and also ensuring to mark epfiles NULL within it. Change-Id: I452c61ba9f404676e08550d12cb7096b8d449706 Signed-off-by: Udipto Goswami --- drivers/usb/gadget/function/f_fs.c | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/drivers/usb/gadget/function/f_fs.c b/drivers/usb/gadget/function/f_fs.c index 5a8c8a3c2b3c..046ecba2074f 100644 --- a/drivers/usb/gadget/function/f_fs.c +++ b/drivers/usb/gadget/function/f_fs.c @@ -1936,11 +1936,13 @@ static void ffs_data_closed(struct ffs_data *ffs) if (atomic_dec_and_test(&ffs->opened)) { if (ffs->no_disconnect) { ffs->state = FFS_DEACTIVATED; + mutex_lock(&ffs->mutex); if (ffs->epfiles) { ffs_epfiles_destroy(ffs->epfiles, ffs->eps_count); ffs->epfiles = NULL; } + mutex_unlock(&ffs->mutex); if (ffs->setup_state == FFS_SETUP_PENDING) __ffs_ep0_stall(ffs); } else { @@ -2008,8 +2010,12 @@ static void ffs_data_clear(struct ffs_data *ffs) BUG_ON(ffs->gadget); - if (ffs->epfiles) + mutex_lock(&ffs->mutex); + if (ffs->epfiles) { ffs_epfiles_destroy(ffs->epfiles, ffs->eps_count); + ffs->epfiles = NULL; + } + mutex_unlock(&ffs->mutex); if (ffs->ffs_eventfd) eventfd_ctx_put(ffs->ffs_eventfd); @@ -2028,7 +2034,6 @@ static void ffs_data_reset(struct ffs_data *ffs) ffs_data_clear(ffs); - ffs->epfiles = NULL; ffs->raw_descs_data = NULL; ffs->raw_descs = NULL; ffs->raw_strings = NULL;