Attempting to unbind a dvbdevice that is in the process of feeding
data causes a UAF as we free the underlying device without
stopping the feed first. Fix this by stopping the stream first using
vidtv_stop_streaming(). However, our codepath in the reproducer
(mentioned in the below reply) does not decrement our users
(dmxdev->dvr_dvbdev->users) to 1 after it has been incremented to 2
by our read() in the reproducer, that is only possible on .release.
This can cause a task hang as dvb_dmxdev_release() uses wait_event()
in the wait_queue unless we use a close(fd)(in the reproducer).
Is this a problem? Please advise.

Reported-by: syzbot+c7fc4794e59786f5b4dc@syzkaller.appspotmail.com
Closes: https://syzkaller.appspot.com/bug?extid=c7fc4794e59786f5b4dc
Fixes: f90cf6079bf6 ("media: vidtv: add a bridge driver")
Cc: stable@vger.kernel.org
Signed-off-by: Jeffin Philip <jeffinphilip14@gmail.com>
---
 drivers/media/test-drivers/vidtv/vidtv_bridge.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/drivers/media/test-drivers/vidtv/vidtv_bridge.c b/drivers/media/test-drivers/vidtv/vidtv_bridge.c
index fd69b4ee16f4..98d918c7f0ff 100644
--- a/drivers/media/test-drivers/vidtv/vidtv_bridge.c
+++ b/drivers/media/test-drivers/vidtv/vidtv_bridge.c
@@ -550,6 +550,8 @@ static void vidtv_bridge_remove(struct platform_device *pdev)
 
 	mutex_destroy(&dvb->feed_lock);
 
+	vidtv_stop_streaming(dvb);
+
 	for (i = 0; i < NUM_FE; ++i) {
 		dvb_unregister_frontend(dvb->fe[i]);
 		dvb_module_release(dvb->i2c_client_tuner[i]);
-- 
2.55.0

>Attempting to unbind a dvbdevice that is in the process of feeding
>data causes a UAF as we free the underlying device without
>stopping the feed first. Fix this by stopping the stream first using
>vidtv_stop_streaming(). However, our codepath in the reproducer
>(mentioned in the below reply) does not decrement our users
>(dmxdev->dvr_dvbdev->users) to 1 after it has been incremented to 2
>by our read() in the reproducer, that is only possible on .release.
>This can cause a task hang as dvb_dmxdev_release() uses wait_event()
>in the wait_queue unless we use a close(fd)(in the reproducer).
>Is this a problem? Please advise.

Reproducer:

#include <fcntl.h>
#include <unistd.h>
#include <pthread.h>
#include <linux/dvb/dmx.h>
#include <sys/ioctl.h>

static void *feed_thread(void *arg)
{
    int i = 100;
    int fd = open("/dev/dvb/adapter0/demux0", O_RDWR | O_NONBLOCK);
    struct dmx_sct_filter_params params = {
        .pid = 0,
        .filter = { .filter = {0}, .mask = {0} },
        .flags = DMX_IMMEDIATE_START,
    };
    ioctl(fd, DMX_SET_FILTER, &params);
    char buf[188];
    read(fd, buf, sizeof(buf));
    //no close(fd) here
    return NULL;
}

int main(void)
{
    pthread_t t;
    pthread_create(&t, NULL, feed_thread, NULL);
    sleep(1);
    int fd = open("/sys/bus/platform/drivers/vidtv/unbind", O_WRONLY);
    write(fd, "vidtv.0", 7);
    pthread_join(t, NULL);
    return 0;
}

On Thu, Aug 27, 2026 at 09:56:15AM +0530, Jeffin Philip wrote:
> >Attempting to unbind a dvbdevice that is in the process of feeding
> >data causes a UAF as we free the underlying device without
> >stopping the feed first. Fix this by stopping the stream first using
> >vidtv_stop_streaming(). However, our codepath in the reproducer
> >(mentioned in the below reply) does not decrement our users
> >(dmxdev->dvr_dvbdev->users) to 1 after it has been incremented to 2
> >by our read() in the reproducer, that is only possible on .release.
> >This can cause a task hang as dvb_dmxdev_release() uses wait_event()
> >in the wait_queue unless we use a close(fd)(in the reproducer).
> >Is this a problem? Please advise.
> 
> Reproducer:
> 
> #include <fcntl.h>
> #include <unistd.h>
> #include <pthread.h>
> #include <linux/dvb/dmx.h>
> #include <sys/ioctl.h>
> 
> static void *feed_thread(void *arg)
> {
>     int i = 100;
>     int fd = open("/dev/dvb/adapter0/demux0", O_RDWR | O_NONBLOCK);
>     struct dmx_sct_filter_params params = {
>         .pid = 0,
>         .filter = { .filter = {0}, .mask = {0} },
>         .flags = DMX_IMMEDIATE_START,
>     };
>     ioctl(fd, DMX_SET_FILTER, &params);
>     char buf[188];
>     read(fd, buf, sizeof(buf));
>     //no close(fd) here
>     return NULL;
> }
> 
> int main(void)
> {
>     pthread_t t;
>     pthread_create(&t, NULL, feed_thread, NULL);
>     sleep(1);
>     int fd = open("/sys/bus/platform/drivers/vidtv/unbind", O_WRONLY);

While "fun", this is not a normal path that users ever will hit.  See
this thread where I propose tainting the kernel if you attempt to do
this:
	 https://lore.kernel.org/r/20260826-bind_taint-v1-0-52b05f4a965c@linuxfoundation.org

thanks,

greg k-h

On Thu, 27 Aug 2026 06:51:21 +0200, Greg KH wrote:

>While "fun", this is not a normal path that users ever will hit.  See
>this thread where I propose tainting the kernel if you attempt to do
>this:
>	 https://lore.kernel.org/r/20260826-bind_taint-v1-0-52b05f4a965c@linuxfoundation.org

Thanks for the reference. The task hang is not possible
then. However, the UAF will be triggered anyway as we are just simulating
an unbind. Even if the driver's .release is called naturally, if we are
streaming data, it could cause the UAF, no?

Thanks,
Jeffin.

On Thu, Aug 27, 2026 at 10:45:08AM +0530, Jeffin Philip wrote:
> On Thu, 27 Aug 2026 06:51:21 +0200, Greg KH wrote:
> 
> >While "fun", this is not a normal path that users ever will hit.  See
> >this thread where I propose tainting the kernel if you attempt to do
> >this:
> >	 https://lore.kernel.org/r/20260826-bind_taint-v1-0-52b05f4a965c@linuxfoundation.org
> 
> Thanks for the reference. The task hang is not possible
> then. However, the UAF will be triggered anyway as we are just simulating
> an unbind. Even if the driver's .release is called naturally, if we are
> streaming data, it could cause the UAF, no?

I'm not saying your patch is incorrect, just that using bind/unbind in a
way to justify code changes isn't ok on it's own (we are seeing some
crazy platform and pci driver patches being proposed because of this...)

It's up to the maintainers here, just wanted to point out that this is
NOT a normal codepath that users can ever exercise and think it's not
going to possibly cause problem.

thanks,

greg k-h

On Thu, 27 Aug 2026 07:20:31 +0200, Greg KH wrote:

>On Thu, Aug 27, 2026 at 10:45:08AM +0530, Jeffin Philip wrote:
>> On Thu, 27 Aug 2026 06:51:21 +0200, Greg KH wrote:
>> 
>> >While "fun", this is not a normal path that users ever will hit.  See
>> >this thread where I propose tainting the kernel if you attempt to do
>> >this:
>> >	 https://lore.kernel.org/r/20260826-bind_taint-v1-0-52b05f4a965c@linuxfoundation.org
>> 
>> Thanks for the reference. The task hang is not possible
>> then. However, the UAF will be triggered anyway as we are just simulating
>> an unbind. Even if the driver's .release is called naturally, if we are
>> streaming data, it could cause the UAF, no?
>
>I'm not saying your patch is incorrect, just that using bind/unbind in a
>way to justify code changes isn't ok on it's own (we are seeing some
>crazy platform and pci driver patches being proposed because of this...)
>
>It's up to the maintainers here, just wanted to point out that this is
>NOT a normal codepath that users can ever exercise and think it's not
>going to possibly cause problem.

Understood. Thanks for the review.

Thanks,
Jeffin.