Re: [PATCH v4 01/19] media: rc: Ensure registered is cleared in error path

[Sean Young <sean@mess.org>]

On Fri, Sep 11, 2026 at 09:53:10AM +0200, Hans Verkuil wrote:
> On 08/09/2026 17:51, Sean Young wrote:
> > If rc_register_device() fails, ensure that registered is not set to true.
> > If lirc_register() succeeded, then userspace could have an open file
> > descriptor open. This leads to a use-after-free.
> > 
> > Fixes: dccc0c3ddf8f ("media: rc: fix race between unregister and urb/irq callbacks")
> > Signed-off-by: Sean Young <sean@mess.org>
> > Cc: stable@vger.kernel.org
> > ---
> >  drivers/media/rc/rc-main.c | 5 ++++-
> >  1 file changed, 4 insertions(+), 1 deletion(-)
> > 
> > diff --git a/drivers/media/rc/rc-main.c b/drivers/media/rc/rc-main.c
> > index d93e98189c1a..1f99540456f1 100644
> > --- a/drivers/media/rc/rc-main.c
> > +++ b/drivers/media/rc/rc-main.c
> > @@ -1934,7 +1934,8 @@ int rc_register_device(struct rc_dev *dev)
> >  			goto out_raw;
> >  	}
> >  
> > -	dev->registered = true;
> > +	scoped_guard(mutex, &dev->lock)
> > +		dev->registered = true;
> 
> Is the guard actually needed?

Yes, we read registered under lock in e.g. in lirc_open(). It s
racey without.

> >  	rc = device_add(&dev->dev);
> >  	if (rc)
> > @@ -1982,6 +1983,8 @@ int rc_register_device(struct rc_dev *dev)
> >  out_dev:
> >  	device_del(&dev->dev);
> >  out_rx_free:
> > +	scoped_guard(mutex, &dev->lock)
> > +		dev->registered = false;
> 
> Same question here. I think the real fix is setting dev->registered to
> false, but I'm not sure the guard makes a difference.
> 
> The commit log mentions nothing about that change either.

You are right, the commit message needs to mention this.


Sean