Message ID | 20210702184957.4479-2-andrew_gabbasov@mentor.com (mailing list archive) |
---|---|
State | Superseded |
Headers | show |
Series | [v4.14] usb: gadget: f_fs: Fix setting of device and driver data cross-references | expand |
On Fri, Jul 02, 2021 at 01:49:57PM -0500, Andrew Gabbasov wrote: > Fixes: 4b187fceec3c ("usb: gadget: FunctionFS: add devices management code") > Fixes: 3262ad824307 ("usb: gadget: f_fs: Stop ffs_closed NULL pointer dereference") > Fixes: cdafb6d8b8da ("usb: gadget: f_fs: Fix use-after-free in ffs_free_inst") > Reported-by: Bhuvanesh Surachari <bhuvanesh_surachari@mentor.com> > Tested-by: Eugeniu Rosca <erosca@de.adit-jv.com> > Reviewed-by: Eugeniu Rosca <erosca@de.adit-jv.com> > Signed-off-by: Andrew Gabbasov <andrew_gabbasov@mentor.com> > Link: https://lore.kernel.org/r/20210603171507.22514-1-andrew_gabbasov@mentor.com > Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org> > (cherry-picked from commit ecfbd7b9054bddb12cea07fda41bb3a79a7b0149) There is no such commit id in Linus's tree :( Please resubmit with the correct id. thanks, greg k-h
Hello Greg, > -----Original Message----- > From: Greg Kroah-Hartman <gregkh@linuxfoundation.org> > Sent: Monday, July 05, 2021 10:08 AM > To: Gabbasov, Andrew <Andrew_Gabbasov@mentor.com> > Cc: Macpaul Lin <macpaul.lin@mediatek.com>; Eugeniu Rosca <erosca@de.adit-jv.com>; linux-usb@vger.kernel.org; > linux-kernel@vger.kernel.org; stable@vger.kernel.org; Felipe Balbi <balbi@kernel.org>; Eugeniu Rosca > <roscaeugeniu@gmail.com>; Eddie Hung <eddie.hung@mediatek.com> > Subject: Re: [PATCH v4.14] usb: gadget: f_fs: Fix setting of device and driver data cross-references > > On Fri, Jul 02, 2021 at 01:49:57PM -0500, Andrew Gabbasov wrote: > > Fixes: 4b187fceec3c ("usb: gadget: FunctionFS: add devices management code") > > Fixes: 3262ad824307 ("usb: gadget: f_fs: Stop ffs_closed NULL pointer dereference") > > Fixes: cdafb6d8b8da ("usb: gadget: f_fs: Fix use-after-free in ffs_free_inst") > > Reported-by: Bhuvanesh Surachari <bhuvanesh_surachari@mentor.com> > > Tested-by: Eugeniu Rosca <erosca@de.adit-jv.com> > > Reviewed-by: Eugeniu Rosca <erosca@de.adit-jv.com> > > Signed-off-by: Andrew Gabbasov <andrew_gabbasov@mentor.com> > > Link: https://lore.kernel.org/r/20210603171507.22514-1-andrew_gabbasov@mentor.com > > Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org> > > (cherry-picked from commit ecfbd7b9054bddb12cea07fda41bb3a79a7b0149) > > There is no such commit id in Linus's tree :( > > Please resubmit with the correct id. This commit is not yet included to the mainline, it only exists in linux-next: https://git.kernel.org/pub/scm/linux/kernel/git/next/linux-next.git/commit/?id=ecfbd7b9054bddb12cea07fda41bb3a79a7b0149 Could you please advise if I need to somehow denote the linux-next repo in the "cherry picked from" line, or just remove this line, or so far wait and re-submit the patch after the original commit is merged to Linus' tree? BTW, I just noticed that the line contains incorrect "cherry-picked" instead of "cherry picked", so I'll have to re-submit the patch anyway
On Mon, Jul 05, 2021 at 01:24:10PM +0300, Andrew Gabbasov wrote: > Hello Greg, > > > -----Original Message----- > > From: Greg Kroah-Hartman <gregkh@linuxfoundation.org> > > Sent: Monday, July 05, 2021 10:08 AM > > To: Gabbasov, Andrew <Andrew_Gabbasov@mentor.com> > > Cc: Macpaul Lin <macpaul.lin@mediatek.com>; Eugeniu Rosca <erosca@de.adit-jv.com>; linux-usb@vger.kernel.org; > > linux-kernel@vger.kernel.org; stable@vger.kernel.org; Felipe Balbi <balbi@kernel.org>; Eugeniu Rosca > > <roscaeugeniu@gmail.com>; Eddie Hung <eddie.hung@mediatek.com> > > Subject: Re: [PATCH v4.14] usb: gadget: f_fs: Fix setting of device and driver data cross-references > > > > On Fri, Jul 02, 2021 at 01:49:57PM -0500, Andrew Gabbasov wrote: > > > Fixes: 4b187fceec3c ("usb: gadget: FunctionFS: add devices management code") > > > Fixes: 3262ad824307 ("usb: gadget: f_fs: Stop ffs_closed NULL pointer dereference") > > > Fixes: cdafb6d8b8da ("usb: gadget: f_fs: Fix use-after-free in ffs_free_inst") > > > Reported-by: Bhuvanesh Surachari <bhuvanesh_surachari@mentor.com> > > > Tested-by: Eugeniu Rosca <erosca@de.adit-jv.com> > > > Reviewed-by: Eugeniu Rosca <erosca@de.adit-jv.com> > > > Signed-off-by: Andrew Gabbasov <andrew_gabbasov@mentor.com> > > > Link: https://lore.kernel.org/r/20210603171507.22514-1-andrew_gabbasov@mentor.com > > > Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org> > > > (cherry-picked from commit ecfbd7b9054bddb12cea07fda41bb3a79a7b0149) > > > > There is no such commit id in Linus's tree :( > > > > Please resubmit with the correct id. > > This commit is not yet included to the mainline, it only exists in linux-next: > https://git.kernel.org/pub/scm/linux/kernel/git/next/linux-next.git/commit/?id=ecfbd7b9054bddb12cea07fda41bb3a79a7b0149 > > Could you please advise if I need to somehow denote the linux-next repo in the "cherry picked from" line, > or just remove this line, or so far wait and re-submit the patch after the original commit is merged to Linus' tree? > BTW, I just noticed that the line contains incorrect "cherry-picked" instead of "cherry picked", > so I'll have to re-submit the patch anyway
Hello Greg, > -----Original Message----- > From: 'Greg Kroah-Hartman' <gregkh@linuxfoundation.org> > Sent: Monday, July 05, 2021 1:42 PM > To: Gabbasov, Andrew <Andrew_Gabbasov@mentor.com> > Cc: Macpaul Lin <macpaul.lin@mediatek.com>; Eugeniu Rosca <erosca@de.adit-jv.com>; linux-usb@vger.kernel.org; > linux-kernel@vger.kernel.org; stable@vger.kernel.org; Felipe Balbi <balbi@kernel.org>; Eugeniu Rosca > <roscaeugeniu@gmail.com>; Eddie Hung <eddie.hung@mediatek.com> > Subject: Re: [PATCH v4.14] usb: gadget: f_fs: Fix setting of device and driver data cross-references > > On Mon, Jul 05, 2021 at 01:24:10PM +0300, Andrew Gabbasov wrote: > > Hello Greg, > > > > > -----Original Message----- > > > From: Greg Kroah-Hartman <gregkh@linuxfoundation.org> > > > Sent: Monday, July 05, 2021 10:08 AM > > > To: Gabbasov, Andrew <Andrew_Gabbasov@mentor.com> > > > Cc: Macpaul Lin <macpaul.lin@mediatek.com>; Eugeniu Rosca <erosca@de.adit-jv.com>; linux-usb@vger.kernel.org; > > > linux-kernel@vger.kernel.org; stable@vger.kernel.org; Felipe Balbi <balbi@kernel.org>; Eugeniu Rosca > > > <roscaeugeniu@gmail.com>; Eddie Hung <eddie.hung@mediatek.com> > > > Subject: Re: [PATCH v4.14] usb: gadget: f_fs: Fix setting of device and driver data cross-references > > > > > > On Fri, Jul 02, 2021 at 01:49:57PM -0500, Andrew Gabbasov wrote: > > > > Fixes: 4b187fceec3c ("usb: gadget: FunctionFS: add devices management code") > > > > Fixes: 3262ad824307 ("usb: gadget: f_fs: Stop ffs_closed NULL pointer dereference") > > > > Fixes: cdafb6d8b8da ("usb: gadget: f_fs: Fix use-after-free in ffs_free_inst") > > > > Reported-by: Bhuvanesh Surachari <bhuvanesh_surachari@mentor.com> > > > > Tested-by: Eugeniu Rosca <erosca@de.adit-jv.com> > > > > Reviewed-by: Eugeniu Rosca <erosca@de.adit-jv.com> > > > > Signed-off-by: Andrew Gabbasov <andrew_gabbasov@mentor.com> > > > > Link: https://lore.kernel.org/r/20210603171507.22514-1-andrew_gabbasov@mentor.com > > > > Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org> > > > > (cherry-picked from commit ecfbd7b9054bddb12cea07fda41bb3a79a7b0149) > > > > > > There is no such commit id in Linus's tree :( > > > > > > Please resubmit with the correct id. > > > > This commit is not yet included to the mainline, it only exists in linux-next: > > https://git.kernel.org/pub/scm/linux/kernel/git/next/linux- > next.git/commit/?id=ecfbd7b9054bddb12cea07fda41bb3a79a7b0149 > > > > Could you please advise if I need to somehow denote the linux-next repo in the "cherry picked from" line, > > or just remove this line, or so far wait and re-submit the patch after the original commit is merged to Linus' > tree? > > BTW, I just noticed that the line contains incorrect "cherry-picked" instead of "cherry picked", > > so I'll have to re-submit the patch anyway
diff --git a/drivers/usb/gadget/function/f_fs.c b/drivers/usb/gadget/function/f_fs.c index 458c5dc296ac..5dfe926c251a 100644 --- a/drivers/usb/gadget/function/f_fs.c +++ b/drivers/usb/gadget/function/f_fs.c @@ -247,8 +247,8 @@ EXPORT_SYMBOL_GPL(ffs_lock); static struct ffs_dev *_ffs_find_dev(const char *name); static struct ffs_dev *_ffs_alloc_dev(void); static void _ffs_free_dev(struct ffs_dev *dev); -static void *ffs_acquire_dev(const char *dev_name); -static void ffs_release_dev(struct ffs_data *ffs_data); +static int ffs_acquire_dev(const char *dev_name, struct ffs_data *ffs_data); +static void ffs_release_dev(struct ffs_dev *ffs_dev); static int ffs_ready(struct ffs_data *ffs); static void ffs_closed(struct ffs_data *ffs); @@ -1505,7 +1505,6 @@ ffs_fs_mount(struct file_system_type *t, int flags, }; struct dentry *rv; int ret; - void *ffs_dev; struct ffs_data *ffs; ENTER(); @@ -1526,19 +1525,16 @@ ffs_fs_mount(struct file_system_type *t, int flags, return ERR_PTR(-ENOMEM); } - ffs_dev = ffs_acquire_dev(dev_name); - if (IS_ERR(ffs_dev)) { + ret = ffs_acquire_dev(dev_name, ffs); + if (ret) { ffs_data_put(ffs); - return ERR_CAST(ffs_dev); + return ERR_PTR(ret); } - ffs->private_data = ffs_dev; data.ffs_data = ffs; rv = mount_nodev(t, flags, &data, ffs_sb_fill); - if (IS_ERR(rv) && data.ffs_data) { - ffs_release_dev(data.ffs_data); + if (IS_ERR(rv) && data.ffs_data) ffs_data_put(data.ffs_data); - } return rv; } @@ -1548,10 +1544,8 @@ ffs_fs_kill_sb(struct super_block *sb) ENTER(); kill_litter_super(sb); - if (sb->s_fs_info) { - ffs_release_dev(sb->s_fs_info); + if (sb->s_fs_info) ffs_data_closed(sb->s_fs_info); - } } static struct file_system_type ffs_fs_type = { @@ -1620,6 +1614,7 @@ static void ffs_data_put(struct ffs_data *ffs) if (unlikely(refcount_dec_and_test(&ffs->ref))) { pr_info("%s(): freeing\n", __func__); ffs_data_clear(ffs); + ffs_release_dev(ffs->private_data); BUG_ON(waitqueue_active(&ffs->ev.waitq) || waitqueue_active(&ffs->ep0req_completion.wait) || waitqueue_active(&ffs->wait)); @@ -2924,6 +2919,7 @@ static inline struct f_fs_opts *ffs_do_functionfs_bind(struct usb_function *f, struct ffs_function *func = ffs_func_from_usb(f); struct f_fs_opts *ffs_opts = container_of(f->fi, struct f_fs_opts, func_inst); + struct ffs_data *ffs_data; int ret; ENTER(); @@ -2938,12 +2934,13 @@ static inline struct f_fs_opts *ffs_do_functionfs_bind(struct usb_function *f, if (!ffs_opts->no_configfs) ffs_dev_lock(); ret = ffs_opts->dev->desc_ready ? 0 : -ENODEV; - func->ffs = ffs_opts->dev->ffs_data; + ffs_data = ffs_opts->dev->ffs_data; if (!ffs_opts->no_configfs) ffs_dev_unlock(); if (ret) return ERR_PTR(ret); + func->ffs = ffs_data; func->conf = c; func->gadget = c->cdev->gadget; @@ -3398,6 +3395,7 @@ static void ffs_free_inst(struct usb_function_instance *f) struct f_fs_opts *opts; opts = to_f_fs_opts(f); + ffs_release_dev(opts->dev); ffs_dev_lock(); _ffs_free_dev(opts->dev); ffs_dev_unlock(); @@ -3585,47 +3583,48 @@ static void _ffs_free_dev(struct ffs_dev *dev) { list_del(&dev->entry); - /* Clear the private_data pointer to stop incorrect dev access */ - if (dev->ffs_data) - dev->ffs_data->private_data = NULL; - kfree(dev); if (list_empty(&ffs_devices)) functionfs_cleanup(); } -static void *ffs_acquire_dev(const char *dev_name) +static int ffs_acquire_dev(const char *dev_name, struct ffs_data *ffs_data) { + int ret = 0; struct ffs_dev *ffs_dev; ENTER(); ffs_dev_lock(); ffs_dev = _ffs_find_dev(dev_name); - if (!ffs_dev) - ffs_dev = ERR_PTR(-ENOENT); - else if (ffs_dev->mounted) - ffs_dev = ERR_PTR(-EBUSY); - else if (ffs_dev->ffs_acquire_dev_callback && - ffs_dev->ffs_acquire_dev_callback(ffs_dev)) - ffs_dev = ERR_PTR(-ENOENT); - else + if (!ffs_dev) { + ret = -ENOENT; + } else if (ffs_dev->mounted) { + ret = -EBUSY; + } else if (ffs_dev->ffs_acquire_dev_callback && + ffs_dev->ffs_acquire_dev_callback(ffs_dev)) { + ret = -ENOENT; + } else { ffs_dev->mounted = true; + ffs_dev->ffs_data = ffs_data; + ffs_data->private_data = ffs_dev; + } ffs_dev_unlock(); - return ffs_dev; + return ret; } -static void ffs_release_dev(struct ffs_data *ffs_data) +static void ffs_release_dev(struct ffs_dev *ffs_dev) { - struct ffs_dev *ffs_dev; - ENTER(); ffs_dev_lock(); - ffs_dev = ffs_data->private_data; - if (ffs_dev) { + if (ffs_dev && ffs_dev->mounted) { ffs_dev->mounted = false; + if (ffs_dev->ffs_data) { + ffs_dev->ffs_data->private_data = NULL; + ffs_dev->ffs_data = NULL; + } if (ffs_dev->ffs_release_dev_callback) ffs_dev->ffs_release_dev_callback(ffs_dev); @@ -3653,7 +3652,6 @@ static int ffs_ready(struct ffs_data *ffs) } ffs_obj->desc_ready = true; - ffs_obj->ffs_data = ffs; if (ffs_obj->ffs_ready_callback) { ret = ffs_obj->ffs_ready_callback(ffs); @@ -3681,7 +3679,6 @@ static void ffs_closed(struct ffs_data *ffs) goto done; ffs_obj->desc_ready = false; - ffs_obj->ffs_data = NULL; if (test_and_clear_bit(FFS_FL_CALL_CLOSED_CALLBACK, &ffs->flags) && ffs_obj->ffs_closed_callback)