Re: [PATCH v2] media: dvb-core: fix use-after-free in dvb_frontend_open()

[Hillf Danton]

On Tue, 19 May 2026 13:10:20 +0800 Yun Zhou wrote:
> dvb_frontend_open() calls dvb_generic_release() in its error path after
> dvb_generic_open() succeeds. dvb_generic_release() drops the device
> reference via dvb_device_put(), and then dvb_device_open() drops it again
> in its error handling, causing a use-after-free and refcount underflow.
> 
> Fix this by introducing __dvb_generic_release() which only restores the
> users/readers/writers counters without dropping the device reference. Use
> it in dvb_frontend_open()'s error path so that dvb_device_open() remains
> the sole owner of the dvb_device_put() on open failure.
> 
Given the relevant code snippet

	dvb_device_open()
	  mutex_lock(&dvbdev_mutex);
	  down_read(&minor_rwsem);
	  dvbdev = dvb_minors[minor];
	  if (dvbdev && dvbdev->fops) {
	    dvb_device_get(dvbdev);
	    err = file->f_op->open(inode, file);
	    up_read(&minor_rwsem);
	    mutex_unlock(&dvbdev_mutex);
	    if (err)
	       dvb_device_put(dvbdev);
	  }

a) the frontend open callback is invoked with refcount incremented, so
why could a single put in the err path drop refcount to ground?
b) worse dvbdev is freed without clearing dvb_minors[minor].

One explanation sounds like

	dvb_device_open(); // err with refcount dropped but
			   // without clearing dvb_minors[minor]
	dvb_device_open(); // single put frees dvbdev

so a simpler fix looks like incrementing refcount before
dvb_generic_release() in the err path.

> Reported-by: syzbot+40339ea82afa8184ad5d@xxxxxxxxxxxxxxxxxxxxxxxxx
> Closes: https://syzkaller.appspot.com/bug?extid=40339ea82afa8184ad5d
> Cc: stable@xxxxxxxxxxxxxxx
> Fixes: 0fc044b2b5e2 ("media: dvbdev: adopts refcnt to avoid UAF")
> Signed-off-by: Yun Zhou <yun.zhou@xxxxxxxxxxxxx>
> ---
> v2:
>  - Fix Fixes tag commit title
>  - Add Closes: link after Reported-by
>  - Cc stable@xxxxxxxxxxxxxxx
> 
>  drivers/media/dvb-core/dvb_frontend.c |  2 +-
>  drivers/media/dvb-core/dvbdev.c       | 17 ++++++++++++-----
>  include/media/dvbdev.h                | 12 ++++++++++++
>  3 files changed, 25 insertions(+), 6 deletions(-)
> 
> diff --git a/drivers/media/dvb-core/dvb_frontend.c b/drivers/media/dvb-core/dvb_frontend.c
> index d082b6c57c76..497f5920b267 100644
> --- a/drivers/media/dvb-core/dvb_frontend.c
> +++ b/drivers/media/dvb-core/dvb_frontend.c
> @@ -2887,7 +2887,7 @@ static int dvb_frontend_open(struct inode *inode, struct file *file)
>  	mutex_unlock(&fe->dvb->mdev_lock);
>  err2:
>  #endif
> -	dvb_generic_release(inode, file);
> +	__dvb_generic_release(inode, file);
>  err1:
>  	if (dvbdev->users == -1 && fe->ops.ts_bus_ctrl)
>  		fe->ops.ts_bus_ctrl(fe, 0);
> diff --git a/drivers/media/dvb-core/dvbdev.c b/drivers/media/dvb-core/dvbdev.c
> index d753d329502a..3e0ad67b79a3 100644
> --- a/drivers/media/dvb-core/dvbdev.c
> +++ b/drivers/media/dvb-core/dvbdev.c
> @@ -152,19 +152,26 @@ int dvb_generic_open(struct inode *inode, struct file *file)
>  }
>  EXPORT_SYMBOL(dvb_generic_open);
>  
> -int dvb_generic_release(struct inode *inode, struct file *file)
> +void __dvb_generic_release(struct inode *inode, struct file *file)
>  {
>  	struct dvb_device *dvbdev = file->private_data;
>  
> -	if (!dvbdev)
> -		return -ENODEV;
> -
>  	if ((file->f_flags & O_ACCMODE) == O_RDONLY)
>  		dvbdev->readers++;
>  	else
>  		dvbdev->writers++;
> -
>  	dvbdev->users++;
> +}
> +EXPORT_SYMBOL(__dvb_generic_release);
> +
> +int dvb_generic_release(struct inode *inode, struct file *file)
> +{
> +	struct dvb_device *dvbdev = file->private_data;
> +
> +	if (!dvbdev)
> +		return -ENODEV;
> +
> +	__dvb_generic_release(inode, file);
>  
>  	dvb_device_put(dvbdev);
>  
> diff --git a/include/media/dvbdev.h b/include/media/dvbdev.h
> index e5a00d126612..9e6e5cb43dcb 100644
> --- a/include/media/dvbdev.h
> +++ b/include/media/dvbdev.h
> @@ -343,6 +343,18 @@ int dvb_create_media_graph(struct dvb_adapter *adap,
>   */
>  int dvb_generic_open(struct inode *inode, struct file *file);
>  
> +/*
> + * __dvb_generic_release - Undo dvb_generic_open() counters WITHOUT
> + *      dropping the device reference.
> + *
> + * @inode: pointer to &struct inode.
> + * @file: pointer to &struct file.
> + *
> + * Used in cases where the caller handles dvb_device_put() and ensures
> + * that dvbdev is valid.
> + */
> +void __dvb_generic_release(struct inode *inode, struct file *file);
> +
>  /**
>   * dvb_generic_release - Digital TV close function, used by DVB devices
>   *
> -- 
> 2.43.0