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

[Zhou, Yun]

On 5/20/26 05:15, Hillf Danton wrote:
> 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.
Yes, this is a simpler way for the current issue. But dvb_device_get()  
before
dvb_generic_release() always feels odd and easily cause confusion for 
readers.
The most elegant way is to pair open() and release(), get() and put() in 
the same
context. To achieve this goal, the changes may be significant. However, 
I will
follow your suggestion to submit a new patch.