Re: [PATCH] usb: gadget: f_fs: Fix epfile null pointer access after ep enable.

[Greg Kroah-Hartman <gregkh@linuxfoundation.org>]

On Fri, Sep 12, 2025 at 09:47:26PM +0800, Owen Gu wrote:
> On Fri, Sep 12, 2025 at 07:32:04AM +0200, Greg Kroah-Hartman wrote:
> > On Fri, Sep 12, 2025 at 12:05:06PM +0800, guhuinan wrote:
> > > A race condition occurs when ffs_func_eps_enable() runs concurrently
> > > with ffs_data_reset(). The ffs_data_clear() called in ffs_data_reset()
> > > sets ffs->epfiles to NULL before resetting ffs->eps_count to 0, leading
> > > to a NULL pointer dereference when accessing epfile->ep in
> > > ffs_func_eps_enable() after successful usb_ep_enable().
> > > 
> > > Signed-off-by: guhuinan <guhuinan@xiaomi.com>
> > 
> > Please use your name, not your email alias for the From: and
> > signed-off-by lines.
> > 
> Okay
> > > ---
> > >  drivers/usb/gadget/function/f_fs.c | 9 ++++++++-
> > >  1 file changed, 8 insertions(+), 1 deletion(-)
> > > 
> > > diff --git a/drivers/usb/gadget/function/f_fs.c b/drivers/usb/gadget/function/f_fs.c
> > > index 08a251df20c4..f4aae91e7864 100644
> > > --- a/drivers/usb/gadget/function/f_fs.c
> > > +++ b/drivers/usb/gadget/function/f_fs.c
> > > @@ -2407,7 +2407,13 @@ static int ffs_func_eps_enable(struct ffs_function *func)
> > >  	ep = func->eps;
> > >  	epfile = ffs->epfiles;
> > >  	count = ffs->eps_count;
> > > -	while(count--) {
> > > +	if (!epfile) {
> > > +		pr_err("%s: epfiles is NULL\n", __func__);
> > 
> > No need for this debugging line, right?
> > 
> Okay
> > > +		ret = -ENOMEM;
> > > +		goto done;
> > > +	}
> > > +
> > > +	while (count--) {
> > 
> > What prevents the pointer from changing right after you check it?  This
> > will still race :(
> > 
> > You need a lock somewhere to fix this properly.
> 
> Dear, 
> The ffs->epfiles pointer is set to NULL in both ffs_data_clear() and 
> ffs_data_close() functions, and its modification is protected by the
> spinlock ffs->eps_lock.
> And the whole ffs_func_eps_enable() function is also protected by ffs->eps_lock.

Ah, that's good.  Please mention that in the changelog text when you
resubmit a new version of this patch.

thanks,

greg k-h