[PATCH v2] media: dvb-usb-v2: disallow 0-length I2C reads

[PATCH v2] media: dvb-usb-v2: disallow 0-length I2C reads

[Nikita Zhandarovich]

Syzkaller reported via syzbot a warning (see [1]) that occurs
when the fuzzer manages to craft a I2C transfer with a 0-length read
request. This in turn leads to an attempt at execution of a
USB 0-length read (which is forbidden by USB protocol) leading to
it being interpreted as a write.

Enable I2C_AQ_NO_ZERO_LEN_READ adapter quirk for all devices
managed by dvb-usb-v2 thus forbidding 0-length read messages
altogether.

[1] Syzbot report
usb 2-1: BOGUS control dir, pipe 80000280 doesn't match bRequestType c0
WARNING: CPU: 0 PID: 5845 at drivers/usb/core/urb.c:413 usb_submit_urb+0x11dd/0x18c0 drivers/usb/core/urb.c:411
...
Call Trace:
 <TASK>
 usb_start_wait_urb+0x11a/0x530 drivers/usb/core/message.c:59
 usb_internal_control_msg drivers/usb/core/message.c:103 [inline]
 usb_control_msg+0x2b3/0x4c0 drivers/usb/core/message.c:154
 gl861_ctrl_msg+0x332/0x6f0 drivers/media/usb/dvb-usb-v2/gl861.c:58
 gl861_i2c_master_xfer+0x3b4/0x650 drivers/media/usb/dvb-usb-v2/gl861.c:144
 __i2c_transfer+0x859/0x2250 drivers/i2c/i2c-core-base.c:-1
 i2c_transfer+0x2c2/0x430 drivers/i2c/i2c-core-base.c:2315
 i2cdev_ioctl_rdwr+0x488/0x780 drivers/i2c/i2c-dev.c:306
 i2cdev_ioctl+0x78a/0xa20 drivers/i2c/i2c-dev.c:467
 vfs_ioctl fs/ioctl.c:51 [inline]
 __do_sys_ioctl fs/ioctl.c:906 [inline]
 __se_sys_ioctl+0xf1/0x160 fs/ioctl.c:892
 do_syscall_x64 arch/x86/entry/syscall_64.c:63 [inline]
 do_syscall_64+0xf3/0x210 arch/x86/entry/syscall_64.c:94
 entry_SYSCALL_64_after_hwframe+0x77/0x7f
...

Reported-by: syzbot+721071c10f3c7e4e5dcb@syzkaller.appspotmail.com
Closes: https://syzkaller.appspot.com/bug?extid=721071c10f3c7e4e5dcb
Tested-by: syzbot+721071c10f3c7e4e5dcb@syzkaller.appspotmail.com
Fixes: 776338e121b9 ("[PATCH] dvb: Add generalized dvb-usb driver")
Signed-off-by: Nikita Zhandarovich <n.zhandarovich@fintech.ru>
---
v1 -> v2: fix description due to overly long lines.

P.S. This issue is eerily similar to the one in dib0700
driver, see [2]. Alan suggested a solution which hasn't been committed
yet in [3]. Mine is essentialy a copy of his, only for dvb-usb-v2
devices. As far as I know, no I2C core level protection against
such issues has been implemented either.
[2] https://syzkaller.appspot.com/bug?extid=c38e5e60d0041a99dbf5
[3] https://lore.kernel.org/all/c7f67d3b-f1e6-4d68-99aa-e462fdcb315f@rowland.harvard.edu/

P.P.S. While this driver seems to be orphaned, I decided to
send a patch anyway, perhaps someone will deem it worthy...

 drivers/media/usb/dvb-usb-v2/dvb_usb_core.c | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/drivers/media/usb/dvb-usb-v2/dvb_usb_core.c b/drivers/media/usb/dvb-usb-v2/dvb_usb_core.c
index f1c79f351ec8..5c76116fd565 100644
--- a/drivers/media/usb/dvb-usb-v2/dvb_usb_core.c
+++ b/drivers/media/usb/dvb-usb-v2/dvb_usb_core.c
@@ -19,6 +19,10 @@ module_param_named(force_pid_filter_usage, dvb_usb_force_pid_filter_usage,
 MODULE_PARM_DESC(force_pid_filter_usage,
 		"force all DVB USB devices to use a PID filter, if any (default: 0)");
 
+static const struct i2c_adapter_quirks i2c_usb_quirks = {
+	.flags = I2C_AQ_NO_ZERO_LEN_READ,
+};
+
 static int dvb_usbv2_download_firmware(struct dvb_usb_device *d,
 		const char *name)
 {
@@ -63,6 +67,7 @@ static int dvb_usbv2_i2c_init(struct dvb_usb_device *d)
 
 	strscpy(d->i2c_adap.name, d->name, sizeof(d->i2c_adap.name));
 	d->i2c_adap.algo = d->props->i2c_algo;
+	d->i2c_adap.quirks = &i2c_usb_quirks;
 	d->i2c_adap.dev.parent = &d->udev->dev;
 	i2c_set_adapdata(&d->i2c_adap, d);
 

Re: [PATCH v2] media: dvb-usb-v2: disallow 0-length I2C reads

[Wolfram Sang]

[-- Attachment #1: Type: text/plain, Size: 2857 bytes --]

Hi Nikita,

thanks for your patch!

On Tue, May 20, 2025 at 04:52:15PM +0300, Nikita Zhandarovich wrote:
> Syzkaller reported via syzbot a warning (see [1]) that occurs
> when the fuzzer manages to craft a I2C transfer with a 0-length read
> request. This in turn leads to an attempt at execution of a
> USB 0-length read (which is forbidden by USB protocol) leading to
> it being interpreted as a write.
> 
> Enable I2C_AQ_NO_ZERO_LEN_READ adapter quirk for all devices
> managed by dvb-usb-v2 thus forbidding 0-length read messages
> altogether.
> 
> [1] Syzbot report
> usb 2-1: BOGUS control dir, pipe 80000280 doesn't match bRequestType c0
> WARNING: CPU: 0 PID: 5845 at drivers/usb/core/urb.c:413 usb_submit_urb+0x11dd/0x18c0 drivers/usb/core/urb.c:411
> ...
> Call Trace:
>  <TASK>
>  usb_start_wait_urb+0x11a/0x530 drivers/usb/core/message.c:59
>  usb_internal_control_msg drivers/usb/core/message.c:103 [inline]
>  usb_control_msg+0x2b3/0x4c0 drivers/usb/core/message.c:154
>  gl861_ctrl_msg+0x332/0x6f0 drivers/media/usb/dvb-usb-v2/gl861.c:58
>  gl861_i2c_master_xfer+0x3b4/0x650 drivers/media/usb/dvb-usb-v2/gl861.c:144
>  __i2c_transfer+0x859/0x2250 drivers/i2c/i2c-core-base.c:-1
>  i2c_transfer+0x2c2/0x430 drivers/i2c/i2c-core-base.c:2315
>  i2cdev_ioctl_rdwr+0x488/0x780 drivers/i2c/i2c-dev.c:306
>  i2cdev_ioctl+0x78a/0xa20 drivers/i2c/i2c-dev.c:467
>  vfs_ioctl fs/ioctl.c:51 [inline]
>  __do_sys_ioctl fs/ioctl.c:906 [inline]
>  __se_sys_ioctl+0xf1/0x160 fs/ioctl.c:892
>  do_syscall_x64 arch/x86/entry/syscall_64.c:63 [inline]
>  do_syscall_64+0xf3/0x210 arch/x86/entry/syscall_64.c:94
>  entry_SYSCALL_64_after_hwframe+0x77/0x7f
> ...
> 
> Reported-by: syzbot+721071c10f3c7e4e5dcb@syzkaller.appspotmail.com
> Closes: https://syzkaller.appspot.com/bug?extid=721071c10f3c7e4e5dcb
> Tested-by: syzbot+721071c10f3c7e4e5dcb@syzkaller.appspotmail.com
> Fixes: 776338e121b9 ("[PATCH] dvb: Add generalized dvb-usb driver")
> Signed-off-by: Nikita Zhandarovich <n.zhandarovich@fintech.ru>

Reviewed-by: Wolfram Sang <wsa+renesas@sang-engineering.com>

> driver, see [2]. Alan suggested a solution which hasn't been committed
> yet in [3]. Mine is essentialy a copy of his, only for dvb-usb-v2

Oh, it is not upstream yet? Pity!

> devices. As far as I know, no I2C core level protection against
> such issues has been implemented either.

Per the discussion with Alan, there can't be an I2C core protection,
sadly. Only drivers using ctrl msgs with no internal header added to the
msg need this quirk. The core can't know this.

> P.P.S. While this driver seems to be orphaned, I decided to
> send a patch anyway, perhaps someone will deem it worthy...

I agree. I once found 4 other drivers needing the same treatment. I
should fix them right now.

Thanks for fixing this one!


[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

Re: [PATCH v2] media: dvb-usb-v2: disallow 0-length I2C reads

[Wolfram Sang]

[-- Attachment #1: Type: text/plain, Size: 201 bytes --]


> +static const struct i2c_adapter_quirks i2c_usb_quirks = {
> +	.flags = I2C_AQ_NO_ZERO_LEN_READ,
> +};

Maybe it makes sense to add a comment like

/* would create an invalid usb_control_msg */

?


[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]