From 4925b858acf890af2e73b0aa9b70c671317404ef Mon Sep 17 00:00:00 2001 From: Udipto Goswami Date: Thu, 16 Dec 2021 16:07:31 +0530 Subject: [PATCH] usb: f_fs: Fix Double free from ffs_data_clear Suppose if the userspace using ffs failed to open ep0, it will issue a ep0_release and continuously try to do ep0_open until it gets through. The general operation of ep0_release is the it will destroy the epfile and free the structures. Whole thing follows this path: ffs_ep0_release ffs_data_reset ffs_data_clear kfree(epfiles) mark NULL kfree(raw_desc) raw_desc =NULL Now the last few steps of the release process is done without any mutex. In one functions we do kfree and another we mark NULL. This created a potential double free scenario, if a ep0_release process got preempted before kfree, meanwhile another ep0_release gets through and freed up the structures but didn't mark NULL and within that time the preempted process wakes up and tried to kfree again, due to structure not marked NULL will lead to double free/invalid free. Following is the illustration: CPU2 CPU3 ffs_ep0_release ffs_data_reset ffs_data_clear kfree(epfiles) epfiles = NULL --preempted-- ffs_ep0_release ffs_data_reset ffs_data_clear kfree(epfiles) epfiles = NULL kfree(ffs->raw_descs_data) kfree(ffs->raw_strings) kfree(ffs->stringtabs) --woke-up-- kfree(ffs->raw_descs_data) raw_desc =NULL Fix this by performing kfree and NULL operations under ffs_data_clear within a mutex lock. Change-Id: I1c8d92ff99c30165b06bafdd00bc9eb610f3bb76 Signed-off-by: Udipto Goswami --- drivers/usb/gadget/function/f_fs.c | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/drivers/usb/gadget/function/f_fs.c b/drivers/usb/gadget/function/f_fs.c index 6b2237d0bc36..3647616f8d69 100644 --- a/drivers/usb/gadget/function/f_fs.c +++ b/drivers/usb/gadget/function/f_fs.c @@ -2012,14 +2012,17 @@ static void ffs_data_clear(struct ffs_data *ffs) 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); kfree(ffs->raw_descs_data); + ffs->raw_descs_data = NULL; kfree(ffs->raw_strings); + ffs->raw_strings = NULL; kfree(ffs->stringtabs); + ffs->stringtabs = NULL; + mutex_unlock(&ffs->mutex); } static void ffs_data_reset(struct ffs_data *ffs) @@ -2031,10 +2034,7 @@ static void ffs_data_reset(struct ffs_data *ffs) ffs_data_clear(ffs); - ffs->raw_descs_data = NULL; ffs->raw_descs = NULL; - ffs->raw_strings = NULL; - ffs->stringtabs = NULL; ffs->raw_descs_length = 0; ffs->fs_descs_count = 0;