Re: [PATCH] video: fbdev: smscufx: Fix use-after-free in ufx_ops_open()

[Hyunwoo Kim <imv4bel@gmail.com>]

On Sun, Sep 25, 2022 at 01:57:46PM +0200, Helge Deller wrote:
> On 9/25/22 13:03, Hyunwoo Kim wrote:
> > A race condition may occur if the user physically removes the
> > USB device while calling open() for this device node.
> > 
> > This is a race condition between the ufx_ops_open() function and
> > the ufx_usb_disconnect() function, which may eventually result in UAF.
> > 
> > So, add a mutex to the ufx_ops_open() and ufx_usb_disconnect() functions
> > to avoid race contidion of krefs.
> > 
> > Signed-off-by: Hyunwoo Kim <imv4bel@gmail.com>
> > ---
> >   drivers/video/fbdev/smscufx.c | 10 ++++++++++
> >   1 file changed, 10 insertions(+)
> > 
> > diff --git a/drivers/video/fbdev/smscufx.c b/drivers/video/fbdev/smscufx.c
> > index d7aa5511c361..a4378a7241f7 100644
> > --- a/drivers/video/fbdev/smscufx.c
> > +++ b/drivers/video/fbdev/smscufx.c
> > @@ -1065,6 +1067,8 @@ static int ufx_ops_open(struct fb_info *info, int user)
> >   {
> >   	struct ufx_data *dev = info->par;
> > 
> > +	mutex_lock(&disconnect_mutex);
> > +
> 
> The next few lines show:
>         if (user == 0 && !console)
>                 return -EBUSY;
> 
> in this case this function exits with the mutex held.
> 
> Please check all possible exit paths and unlock the mutex
> where necessary.

yes. I made a mistake.
I submitted a fixed v2 patch.


Best Regards,
Hyunwoo Kim.