media: vidtv: Fix use-after-free in vidtv_bridge_dvb_init()
authorChen Zhongjin <chenzhongjin@huawei.com>
Tue, 8 Nov 2022 07:06:30 +0000 (15:06 +0800)
committerGreg Kroah-Hartman <gregkh@linuxfoundation.org>
Sat, 31 Dec 2022 12:32:13 +0000 (13:32 +0100)
[ Upstream commit ba8d9405935097e296bcf7a942c3a01df0edb865 ]

KASAN reports a use-after-free:
BUG: KASAN: use-after-free in dvb_dmxdev_release+0x4d5/0x5d0 [dvb_core]
Call Trace:
 ...
 dvb_dmxdev_release+0x4d5/0x5d0 [dvb_core]
 vidtv_bridge_probe+0x7bf/0xa40 [dvb_vidtv_bridge]
 platform_probe+0xb6/0x170
 ...
Allocated by task 1238:
 ...
 dvb_register_device+0x1a7/0xa70 [dvb_core]
 dvb_dmxdev_init+0x2af/0x4a0 [dvb_core]
 vidtv_bridge_probe+0x766/0xa40 [dvb_vidtv_bridge]
 ...
Freed by task 1238:
 dvb_register_device+0x6d2/0xa70 [dvb_core]
 dvb_dmxdev_init+0x2af/0x4a0 [dvb_core]
 vidtv_bridge_probe+0x766/0xa40 [dvb_vidtv_bridge]
 ...

It is because the error handling in vidtv_bridge_dvb_init() is wrong.

First, vidtv_bridge_dmx(dev)_init() will clean themselves when fail, but
goto fail_dmx(_dev): calls release functions again, which causes
use-after-free.

Also, in fail_fe, fail_tuner_probe and fail_demod_probe, j = i will cause
out-of-bound when i finished its loop (i == NUM_FE). And the loop
releasing is wrong, although now NUM_FE is 1 so it won't cause problem.

Fix this by correctly releasing everything.

Fixes: f90cf6079bf6 ("media: vidtv: add a bridge driver")
Signed-off-by: Chen Zhongjin <chenzhongjin@huawei.com>
Signed-off-by: Hans Verkuil <hverkuil-cisco@xs4all.nl>
Signed-off-by: Sasha Levin <sashal@kernel.org>
drivers/media/test-drivers/vidtv/vidtv_bridge.c

index 8262061..dff7265 100644 (file)
@@ -459,26 +459,20 @@ fail_dmx_conn:
        for (j = j - 1; j >= 0; --j)
                dvb->demux.dmx.remove_frontend(&dvb->demux.dmx,
                                               &dvb->dmx_fe[j]);
-fail_dmx_dev:
        dvb_dmxdev_release(&dvb->dmx_dev);
-fail_dmx:
+fail_dmx_dev:
        dvb_dmx_release(&dvb->demux);
+fail_dmx:
+fail_demod_probe:
+       for (i = i - 1; i >= 0; --i) {
+               dvb_unregister_frontend(dvb->fe[i]);
 fail_fe:
-       for (j = i; j >= 0; --j)
-               dvb_unregister_frontend(dvb->fe[j]);
+               dvb_module_release(dvb->i2c_client_tuner[i]);
 fail_tuner_probe:
-       for (j = i; j >= 0; --j)
-               if (dvb->i2c_client_tuner[j])
-                       dvb_module_release(dvb->i2c_client_tuner[j]);
-
-fail_demod_probe:
-       for (j = i; j >= 0; --j)
-               if (dvb->i2c_client_demod[j])
-                       dvb_module_release(dvb->i2c_client_demod[j]);
-
+               dvb_module_release(dvb->i2c_client_demod[i]);
+       }
 fail_adapter:
        dvb_unregister_adapter(&dvb->adapter);
-
 fail_i2c:
        i2c_del_adapter(&dvb->i2c_adapter);