[syzbot] [usb?] WARNING in dib0700_i2c_xfer/usb_submit_urb

[syzbot] [usb?] WARNING in dib0700_i2c_xfer/usb_submit_urb

[syzbot]

Hello,

syzbot found the following issue on:

HEAD commit:    5fc319360819 Merge tag 'net-6.14-rc8' of git://git.kernel...
git tree:       upstream
console+strace: https://syzkaller.appspot.com/x/log.txt?x=15445e98580000
kernel config:  https://syzkaller.appspot.com/x/.config?x=27515cfdbafbb90d
dashboard link: https://syzkaller.appspot.com/bug?extid=c38e5e60d0041a99dbf5
compiler:       Debian clang version 15.0.6, GNU ld (GNU Binutils for Debian) 2.40
syz repro:      https://syzkaller.appspot.com/x/repro.syz?x=13ea4c4c580000
C reproducer:   https://syzkaller.appspot.com/x/repro.c?x=15435004580000

Downloadable assets:
disk image: https://storage.googleapis.com/syzbot-assets/1c90f739fd77/disk-5fc31936.raw.xz
vmlinux: https://storage.googleapis.com/syzbot-assets/1949bfaaa2fe/vmlinux-5fc31936.xz
kernel image: https://storage.googleapis.com/syzbot-assets/dc1e147ca5d4/bzImage-5fc31936.xz

IMPORTANT: if you fix the issue, please add the following tag to the commit:
Reported-by: syzbot+c38e5e60d0041a99dbf5@syzkaller.appspotmail.com

------------[ cut here ]------------
usb 1-1: BOGUS control dir, pipe 80000f80 doesn't match bRequestType c0
WARNING: CPU: 1 PID: 5901 at drivers/usb/core/urb.c:413 usb_submit_urb+0x11d9/0x18c0 drivers/usb/core/urb.c:411
Modules linked in:
CPU: 1 UID: 0 PID: 5901 Comm: syz-executor773 Not tainted 6.14.0-rc7-syzkaller-00137-g5fc319360819 #0
Hardware name: Google Google Compute Engine/Google Compute Engine, BIOS Google 02/12/2025
RIP: 0010:usb_submit_urb+0x11d9/0x18c0 drivers/usb/core/urb.c:411
Code: 48 8b 4c 24 08 0f b6 04 01 84 c0 0f 85 52 05 00 00 45 0f b6 07 48 c7 c7 e0 4b d1 8c 4c 89 f6 48 89 da 89 e9 e8 e8 9b 09 fa 90 <0f> 0b 90 90 e9 c9 f3 ff ff 89 e9 80 e1 07 80 c1 03 38 c1 0f 8c 57
RSP: 0018:ffffc900041cf808 EFLAGS: 00010246
RAX: fe52edbd68e0e800 RBX: ffff888021693020 RCX: ffff8880267d5a00
RDX: 0000000000000000 RSI: 0000000000000001 RDI: 0000000000000000
RBP: 0000000080000f80 R08: ffffffff81819e52 R09: 1ffff92000839e9c
R10: dffffc0000000000 R11: fffff52000839e9d R12: ffff88802a23f0a8
R13: ffff88801e6cfa00 R14: ffffffff8cd1b600 R15: ffff888033956e60
FS:  0000555580b9b380(0000) GS:ffff8880b8700000(0000) knlGS:0000000000000000
CS:  0010 DS: 0000 ES: 0000 CR0: 0000000080050033
CR2: 00007fa8c1abb0f0 CR3: 00000000282c0000 CR4: 00000000003526f0
DR0: 0000000000000000 DR1: 0000000000000000 DR2: 0000000000000000
DR3: 0000000000000000 DR6: 00000000fffe0ff0 DR7: 0000000000000400
Call Trace:
 <TASK>
 usb_start_wait_urb+0x113/0x520 drivers/usb/core/message.c:59
 usb_internal_control_msg drivers/usb/core/message.c:103 [inline]
 usb_control_msg+0x2b1/0x4c0 drivers/usb/core/message.c:154
 dib0700_ctrl_rd drivers/media/usb/dvb-usb/dib0700_core.c:95 [inline]
 dib0700_i2c_xfer_legacy drivers/media/usb/dvb-usb/dib0700_core.c:315 [inline]
 dib0700_i2c_xfer+0xc53/0x1060 drivers/media/usb/dvb-usb/dib0700_core.c:361
 __i2c_transfer+0x866/0x2220
 i2c_transfer+0x271/0x3b0 drivers/i2c/i2c-core-base.c:2315
 i2cdev_ioctl_rdwr+0x452/0x710 drivers/i2c/i2c-dev.c:306
 i2cdev_ioctl+0x759/0x9f0 drivers/i2c/i2c-dev.c:467
 vfs_ioctl fs/ioctl.c:51 [inline]
 __do_sys_ioctl fs/ioctl.c:906 [inline]
 __se_sys_ioctl+0xf5/0x170 fs/ioctl.c:892
 do_syscall_x64 arch/x86/entry/common.c:52 [inline]
 do_syscall_64+0xf3/0x230 arch/x86/entry/common.c:83
 entry_SYSCALL_64_after_hwframe+0x77/0x7f
RIP: 0033:0x7fa8c1a448f9
Code: 28 00 00 00 75 05 48 83 c4 28 c3 e8 c1 17 00 00 90 48 89 f8 48 89 f7 48 89 d6 48 89 ca 4d 89 c2 4d 89 c8 4c 8b 4c 24 08 0f 05 <48> 3d 01 f0 ff ff 73 01 c3 48 c7 c1 b8 ff ff ff f7 d8 64 89 01 48
RSP: 002b:00007ffc66aa7058 EFLAGS: 00000246 ORIG_RAX: 0000000000000010
RAX: ffffffffffffffda RBX: 0000000000000000 RCX: 00007fa8c1a448f9
RDX: 00002000000004c0 RSI: 0000000000000707 RDI: 0000000000000004
RBP: 000000000002ca0e R08: 00232d6332692f76 R09: 0000000000000006
R10: 000000000000001f R11: 0000000000000246 R12: 00007ffc66aa706c
R13: 431bde82d7b634db R14: 0000000000000001 R15: 0000000000000001
 </TASK>


---
This report is generated by a bot. It may contain errors.
See https://goo.gl/tpsmEJ for more information about syzbot.
syzbot engineers can be reached at syzkaller@googlegroups.com.

syzbot will keep track of this issue. See:
https://goo.gl/tpsmEJ#status for how to communicate with syzbot.

If the report is already addressed, let syzbot know by replying with:
#syz fix: exact-commit-title

If you want syzbot to run the reproducer, reply with:
#syz test: git://repo/address.git branch-or-commit-hash
If you attach or paste a git patch, syzbot will apply it before testing.

If you want to overwrite report's subsystems, reply with:
#syz set subsystems: new-subsystem
(See the list of subsystem names on the web dashboard)

If the report is a duplicate of another one, reply with:
#syz dup: exact-subject-of-another-report

If you want to undo deduplication, reply with:
#syz undup

Re: [syzbot] [usb?] WARNING in dib0700_i2c_xfer/usb_submit_urb

[Alan Stern]

On Mon, Mar 24, 2025 at 11:18:29AM -0700, syzbot wrote:
> Hello,
> 
> syzbot found the following issue on:
> 
> HEAD commit:    5fc319360819 Merge tag 'net-6.14-rc8' of git://git.kernel...
> git tree:       upstream
> console+strace: https://syzkaller.appspot.com/x/log.txt?x=15445e98580000
> kernel config:  https://syzkaller.appspot.com/x/.config?x=27515cfdbafbb90d
> dashboard link: https://syzkaller.appspot.com/bug?extid=c38e5e60d0041a99dbf5
> compiler:       Debian clang version 15.0.6, GNU ld (GNU Binutils for Debian) 2.40
> syz repro:      https://syzkaller.appspot.com/x/repro.syz?x=13ea4c4c580000
> C reproducer:   https://syzkaller.appspot.com/x/repro.c?x=15435004580000

> ------------[ cut here ]------------
> usb 1-1: BOGUS control dir, pipe 80000f80 doesn't match bRequestType c0
> WARNING: CPU: 1 PID: 5901 at drivers/usb/core/urb.c:413 usb_submit_urb+0x11d9/0x18c0 drivers/usb/core/urb.c:411

> Call Trace:
>  <TASK>
>  usb_start_wait_urb+0x113/0x520 drivers/usb/core/message.c:59
>  usb_internal_control_msg drivers/usb/core/message.c:103 [inline]
>  usb_control_msg+0x2b1/0x4c0 drivers/usb/core/message.c:154
>  dib0700_ctrl_rd drivers/media/usb/dvb-usb/dib0700_core.c:95 [inline]
>  dib0700_i2c_xfer_legacy drivers/media/usb/dvb-usb/dib0700_core.c:315 [inline]
>  dib0700_i2c_xfer+0xc53/0x1060 drivers/media/usb/dvb-usb/dib0700_core.c:361
>  __i2c_transfer+0x866/0x2220
>  i2c_transfer+0x271/0x3b0 drivers/i2c/i2c-core-base.c:2315
>  i2cdev_ioctl_rdwr+0x452/0x710 drivers/i2c/i2c-dev.c:306
>  i2cdev_ioctl+0x759/0x9f0 drivers/i2c/i2c-dev.c:467
>  vfs_ioctl fs/ioctl.c:51 [inline]

It appears that this problem was caused by the fuzzer requesting an i2c 
transfer containing a 0-length read (I2C_M_RD) message.  The dib0700 
driver translates this more or less literally into a USB read request of 
length 0.  But the USB protocol does not allow such things; a 
request of length 0 is always a write.  Hence the WARNING above.

As far as I can tell from the source code, the dib0700 simply isn't able 
to handle 0-length reads.  Should the dib0700_ctrl_rd() routine be 
changed simply to return 0 in such cases?

Alan Stern

Re: [syzbot] [usb?] WARNING in dib0700_i2c_xfer/usb_submit_urb

[Wolfram Sang]

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


> As far as I can tell from the source code, the dib0700 simply isn't able 
> to handle 0-length reads.  Should the dib0700_ctrl_rd() routine be 
> changed simply to return 0 in such cases?

The adapter (I assume the one in dvb-usb-i2c.c) should populate an
i2c_adapter_quirks struct with I2C_AQ_NO_ZERO_LEN and then the core will
bail out for you.


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

Re: [syzbot] [usb?] WARNING in dib0700_i2c_xfer/usb_submit_urb

[Alan Stern]

On Mon, Mar 24, 2025 at 08:19:32PM +0100, Wolfram Sang wrote:
> 
> > As far as I can tell from the source code, the dib0700 simply isn't able 
> > to handle 0-length reads.  Should the dib0700_ctrl_rd() routine be 
> > changed simply to return 0 in such cases?
> 
> The adapter (I assume the one in dvb-usb-i2c.c) should populate an
> i2c_adapter_quirks struct with I2C_AQ_NO_ZERO_LEN and then the core will
> bail out for you.

Or the I2C_AQ_NO_ZERO_LEN_READ flag bit.

What about all the other fields in the i2c_adapter_quirks structure?  
How should they be set?  (Note: I don't know anything about this driver 
or these devices; I'm just chasing down the syzbot bug report.)

Alan Stern

Re: [syzbot] [usb?] WARNING in dib0700_i2c_xfer/usb_submit_urb

[Wolfram Sang]

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


> > > As far as I can tell from the source code, the dib0700 simply isn't able 
> > > to handle 0-length reads.  Should the dib0700_ctrl_rd() routine be 
> > > changed simply to return 0 in such cases?
> > 
> > The adapter (I assume the one in dvb-usb-i2c.c) should populate an
> > i2c_adapter_quirks struct with I2C_AQ_NO_ZERO_LEN and then the core will
> > bail out for you.
> 
> Or the I2C_AQ_NO_ZERO_LEN_READ flag bit.

Yes, that would be more convervative. Does USB allow zero-length writes?

> What about all the other fields in the i2c_adapter_quirks structure?  
> How should they be set?  (Note: I don't know anything about this driver 
> or these devices; I'm just chasing down the syzbot bug report.)

As I also don't know the hardware, I suggest to leave them empty. 0
means "no quirk".


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

Re: [syzbot] [usb?] WARNING in dib0700_i2c_xfer/usb_submit_urb

[Alan Stern]

On Tue, Mar 25, 2025 at 05:59:10PM +0100, Wolfram Sang wrote:
> 
> > > > As far as I can tell from the source code, the dib0700 simply isn't able 
> > > > to handle 0-length reads.  Should the dib0700_ctrl_rd() routine be 
> > > > changed simply to return 0 in such cases?
> > > 
> > > The adapter (I assume the one in dvb-usb-i2c.c) should populate an
> > > i2c_adapter_quirks struct with I2C_AQ_NO_ZERO_LEN and then the core will
> > > bail out for you.
> > 
> > Or the I2C_AQ_NO_ZERO_LEN_READ flag bit.
> 
> Yes, that would be more convervative. Does USB allow zero-length writes?

It does.

> > What about all the other fields in the i2c_adapter_quirks structure?  
> > How should they be set?  (Note: I don't know anything about this driver 
> > or these devices; I'm just chasing down the syzbot bug report.)
> 
> As I also don't know the hardware, I suggest to leave them empty. 0
> means "no quirk".

Okay, let's see if this works.

Alan Stern


#syz test: git://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git 5fc319360819

Index: usb-devel/drivers/media/usb/dvb-usb/dvb-usb-i2c.c
===================================================================
--- usb-devel.orig/drivers/media/usb/dvb-usb/dvb-usb-i2c.c
+++ usb-devel/drivers/media/usb/dvb-usb/dvb-usb-i2c.c
@@ -10,6 +10,9 @@
 
 int dvb_usb_i2c_init(struct dvb_usb_device *d)
 {
+	static const struct i2c_adapter_quirks i2c_usb_quirks = {
+		.flags = I2C_AQ_NO_ZERO_LEN_READ,
+	};
 	int ret = 0;
 
 	if (!(d->props.caps & DVB_USB_IS_AN_I2C_ADAPTER))
@@ -24,6 +27,7 @@ int dvb_usb_i2c_init(struct dvb_usb_devi
 	strscpy(d->i2c_adap.name, d->desc->name, sizeof(d->i2c_adap.name));
 	d->i2c_adap.algo      = d->props.i2c_algo;
 	d->i2c_adap.algo_data = NULL;
+	d->i2c_adap.quirks    = &i2c_usb_quirks;
 	d->i2c_adap.dev.parent = &d->udev->dev;
 
 	i2c_set_adapdata(&d->i2c_adap, d);

Re: [syzbot] [usb?] WARNING in dib0700_i2c_xfer/usb_submit_urb

[syzbot]

Hello,

syzbot has tested the proposed patch and the reproducer did not trigger any issue:

Reported-by: syzbot+c38e5e60d0041a99dbf5@syzkaller.appspotmail.com
Tested-by: syzbot+c38e5e60d0041a99dbf5@syzkaller.appspotmail.com

Tested on:

commit:         5fc31936 Merge tag 'net-6.14-rc8' of git://git.kernel...
git tree:       git://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git
console output: https://syzkaller.appspot.com/x/log.txt?x=17c1324c580000
kernel config:  https://syzkaller.appspot.com/x/.config?x=27515cfdbafbb90d
dashboard link: https://syzkaller.appspot.com/bug?extid=c38e5e60d0041a99dbf5
compiler:       Debian clang version 15.0.6, GNU ld (GNU Binutils for Debian) 2.40
patch:          https://syzkaller.appspot.com/x/patch.diff?x=1459643f980000

Note: testing is done by a robot and is best-effort only.

[PATCH] media: dvb: usb: Fix WARNING in dib0700_i2c_xfer/usb_submit_urb

[Alan Stern]

The syzbot fuzzer reported a WARNING related to the dib0700 dvb-usb
driver:

usb 1-1: BOGUS control dir, pipe 80000f80 doesn't match bRequestType c0
WARNING: CPU: 1 PID: 5901 at drivers/usb/core/urb.c:413 usb_submit_urb+0x11d9/0x18c0 drivers/usb/core/urb.c:411
...
Call Trace:
 <TASK>
 usb_start_wait_urb+0x113/0x520 drivers/usb/core/message.c:59
 usb_internal_control_msg drivers/usb/core/message.c:103 [inline]
 usb_control_msg+0x2b1/0x4c0 drivers/usb/core/message.c:154
 dib0700_ctrl_rd drivers/media/usb/dvb-usb/dib0700_core.c:95 [inline]
 dib0700_i2c_xfer_legacy drivers/media/usb/dvb-usb/dib0700_core.c:315 [inline]
 dib0700_i2c_xfer+0xc53/0x1060 drivers/media/usb/dvb-usb/dib0700_core.c:361
 __i2c_transfer+0x866/0x2220
 i2c_transfer+0x271/0x3b0 drivers/i2c/i2c-core-base.c:2315
 i2cdev_ioctl_rdwr+0x452/0x710 drivers/i2c/i2c-dev.c:306
 i2cdev_ioctl+0x759/0x9f0 drivers/i2c/i2c-dev.c:467
 vfs_ioctl fs/ioctl.c:51 [inline]

Evidently the fuzzer submitted an I2C transfer containing a length-0
read message.  The dib0700 driver translated this more or less
literally into a length-0 USB read request.  But the USB protocol does
not allow reads to have length 0; all length-0 transfers are
considered to be writes.  Hence the WARNING above.

Fix the problem by adding the I2C_AQ_NO_ZERO_LEN_READ adapter quirk
flag to all the USB I2C adapter devices managed by dvb-usb-i2c.c,
following Wolfram Sang's suggestion.  This tells the I2C core not to
allow length-0 read messages.

Reported-by: syzbot+c38e5e60d0041a99dbf5@syzkaller.appspotmail.com
Tested-by: syzbot+c38e5e60d0041a99dbf5@syzkaller.appspotmail.com
Suggested-by: Wolfram Sang <wsa+renesas@sang-engineering.com>
Signed-off-by: Alan Stern <stern@rowland.harvard.edu>
Link: https://lore.kernel.org/linux-usb/67e1a1f5.050a0220.a7ebc.0029.GAE@google.com/

---

Index: usb-devel/drivers/media/usb/dvb-usb/dvb-usb-i2c.c
===================================================================
--- usb-devel.orig/drivers/media/usb/dvb-usb/dvb-usb-i2c.c
+++ usb-devel/drivers/media/usb/dvb-usb/dvb-usb-i2c.c
@@ -10,6 +10,9 @@
 
 int dvb_usb_i2c_init(struct dvb_usb_device *d)
 {
+	static const struct i2c_adapter_quirks i2c_usb_quirks = {
+		.flags = I2C_AQ_NO_ZERO_LEN_READ,
+	};
 	int ret = 0;
 
 	if (!(d->props.caps & DVB_USB_IS_AN_I2C_ADAPTER))
@@ -24,6 +27,7 @@ int dvb_usb_i2c_init(struct dvb_usb_devi
 	strscpy(d->i2c_adap.name, d->desc->name, sizeof(d->i2c_adap.name));
 	d->i2c_adap.algo      = d->props.i2c_algo;
 	d->i2c_adap.algo_data = NULL;
+	d->i2c_adap.quirks    = &i2c_usb_quirks;
 	d->i2c_adap.dev.parent = &d->udev->dev;
 
 	i2c_set_adapdata(&d->i2c_adap, d);

Re: [PATCH] media: dvb: usb: Fix WARNING in dib0700_i2c_xfer/usb_submit_urb

[Wolfram Sang]

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


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

Why didn't you create the static struct outside of probe?


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

Re: [PATCH] media: dvb: usb: Fix WARNING in dib0700_i2c_xfer/usb_submit_urb

[Alan Stern]

On Tue, Mar 25, 2025 at 08:56:57PM +0100, Wolfram Sang wrote:
> 
> > +	static const struct i2c_adapter_quirks i2c_usb_quirks = {
> > +		.flags = I2C_AQ_NO_ZERO_LEN_READ,
> > +	};
> 
> Why didn't you create the static struct outside of probe?

Because it's used only in that one function.  But if you prefer, I will 
move the definition outside of the function.  It doesn't make any real 
difference.

Alan Stern

Re: [PATCH] media: dvb: usb: Fix WARNING in dib0700_i2c_xfer/usb_submit_urb

[Wolfram Sang]

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

On Tue, Mar 25, 2025 at 05:47:53PM -0400, Alan Stern wrote:
> On Tue, Mar 25, 2025 at 08:56:57PM +0100, Wolfram Sang wrote:
> > 
> > > +	static const struct i2c_adapter_quirks i2c_usb_quirks = {
> > > +		.flags = I2C_AQ_NO_ZERO_LEN_READ,
> > > +	};
> > 
> > Why didn't you create the static struct outside of probe?
> 
> Because it's used only in that one function.  But if you prefer, I will 
> move the definition outside of the function.  It doesn't make any real 
> difference.

Then, for consistency reasons, I'd really prefer it outside probe. I
also think it doesn't really make a difference, it just looks unusual to
me.

Thanks and happy hacking!


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

[PATCH v2] media: dvb: usb: Fix WARNING in dib0700_i2c_xfer/usb_submit_urb

[Alan Stern]

The syzbot fuzzer reported a WARNING related to the dib0700 dvb-usb
driver:

usb 1-1: BOGUS control dir, pipe 80000f80 doesn't match bRequestType c0
WARNING: CPU: 1 PID: 5901 at drivers/usb/core/urb.c:413 usb_submit_urb+0x11d9/0x18c0 drivers/usb/core/urb.c:411
...
Call Trace:
 <TASK>
 usb_start_wait_urb+0x113/0x520 drivers/usb/core/message.c:59
 usb_internal_control_msg drivers/usb/core/message.c:103 [inline]
 usb_control_msg+0x2b1/0x4c0 drivers/usb/core/message.c:154
 dib0700_ctrl_rd drivers/media/usb/dvb-usb/dib0700_core.c:95 [inline]
 dib0700_i2c_xfer_legacy drivers/media/usb/dvb-usb/dib0700_core.c:315 [inline]
 dib0700_i2c_xfer+0xc53/0x1060 drivers/media/usb/dvb-usb/dib0700_core.c:361
 __i2c_transfer+0x866/0x2220
 i2c_transfer+0x271/0x3b0 drivers/i2c/i2c-core-base.c:2315
 i2cdev_ioctl_rdwr+0x452/0x710 drivers/i2c/i2c-dev.c:306
 i2cdev_ioctl+0x759/0x9f0 drivers/i2c/i2c-dev.c:467
 vfs_ioctl fs/ioctl.c:51 [inline]

Evidently the fuzzer submitted an I2C transfer containing a length-0
read message.  The dib0700 driver translated this more or less
literally into a length-0 USB read request.  But the USB protocol does
not allow reads to have length 0; all length-0 transfers are
considered to be writes.  Hence the WARNING above.

Fix the problem by adding the I2C_AQ_NO_ZERO_LEN_READ adapter quirk
flag to all the USB I2C adapter devices managed by dvb-usb-i2c.c,
following Wolfram Sang's suggestion.  This tells the I2C core not to
allow length-0 read messages.

Reported-by: syzbot+c38e5e60d0041a99dbf5@syzkaller.appspotmail.com
Tested-by: syzbot+c38e5e60d0041a99dbf5@syzkaller.appspotmail.com
Suggested-by: Wolfram Sang <wsa+renesas@sang-engineering.com>
Signed-off-by: Alan Stern <stern@rowland.harvard.edu>
Link: https://lore.kernel.org/linux-usb/67e1a1f5.050a0220.a7ebc.0029.GAE@google.com/

---

v2: Move the static definition of the i2c_usb_quirks structure outside
the dvb_usb_i2c_init() function.

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

Index: usb-devel/drivers/media/usb/dvb-usb/dvb-usb-i2c.c
===================================================================
--- usb-devel.orig/drivers/media/usb/dvb-usb/dvb-usb-i2c.c
+++ usb-devel/drivers/media/usb/dvb-usb/dvb-usb-i2c.c
@@ -8,6 +8,10 @@
  */
 #include "dvb-usb-common.h"
 
+static const struct i2c_adapter_quirks i2c_usb_quirks = {
+	.flags = I2C_AQ_NO_ZERO_LEN_READ,
+};
+
 int dvb_usb_i2c_init(struct dvb_usb_device *d)
 {
 	int ret = 0;
@@ -24,6 +28,7 @@ int dvb_usb_i2c_init(struct dvb_usb_devi
 	strscpy(d->i2c_adap.name, d->desc->name, sizeof(d->i2c_adap.name));
 	d->i2c_adap.algo      = d->props.i2c_algo;
 	d->i2c_adap.algo_data = NULL;
+	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: Fix WARNING in dib0700_i2c_xfer/usb_submit_urb

[Wolfram Sang]

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

On Wed, Mar 26, 2025 at 11:28:19AM -0400, Alan Stern wrote:
> The syzbot fuzzer reported a WARNING related to the dib0700 dvb-usb
> driver:
> 
> usb 1-1: BOGUS control dir, pipe 80000f80 doesn't match bRequestType c0
> WARNING: CPU: 1 PID: 5901 at drivers/usb/core/urb.c:413 usb_submit_urb+0x11d9/0x18c0 drivers/usb/core/urb.c:411
> ...
> Call Trace:
>  <TASK>
>  usb_start_wait_urb+0x113/0x520 drivers/usb/core/message.c:59
>  usb_internal_control_msg drivers/usb/core/message.c:103 [inline]
>  usb_control_msg+0x2b1/0x4c0 drivers/usb/core/message.c:154
>  dib0700_ctrl_rd drivers/media/usb/dvb-usb/dib0700_core.c:95 [inline]
>  dib0700_i2c_xfer_legacy drivers/media/usb/dvb-usb/dib0700_core.c:315 [inline]
>  dib0700_i2c_xfer+0xc53/0x1060 drivers/media/usb/dvb-usb/dib0700_core.c:361
>  __i2c_transfer+0x866/0x2220
>  i2c_transfer+0x271/0x3b0 drivers/i2c/i2c-core-base.c:2315
>  i2cdev_ioctl_rdwr+0x452/0x710 drivers/i2c/i2c-dev.c:306
>  i2cdev_ioctl+0x759/0x9f0 drivers/i2c/i2c-dev.c:467
>  vfs_ioctl fs/ioctl.c:51 [inline]
> 
> Evidently the fuzzer submitted an I2C transfer containing a length-0
> read message.  The dib0700 driver translated this more or less
> literally into a length-0 USB read request.  But the USB protocol does
> not allow reads to have length 0; all length-0 transfers are
> considered to be writes.  Hence the WARNING above.
> 
> Fix the problem by adding the I2C_AQ_NO_ZERO_LEN_READ adapter quirk
> flag to all the USB I2C adapter devices managed by dvb-usb-i2c.c,
> following Wolfram Sang's suggestion.  This tells the I2C core not to
> allow length-0 read messages.
> 
> Reported-by: syzbot+c38e5e60d0041a99dbf5@syzkaller.appspotmail.com
> Tested-by: syzbot+c38e5e60d0041a99dbf5@syzkaller.appspotmail.com
> Suggested-by: Wolfram Sang <wsa+renesas@sang-engineering.com>
> Signed-off-by: Alan Stern <stern@rowland.harvard.edu>
> Link: https://lore.kernel.org/linux-usb/67e1a1f5.050a0220.a7ebc.0029.GAE@google.com/

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

Thanks for taking care of it!


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

Re: [PATCH v2] media: dvb: usb: Fix WARNING in dib0700_i2c_xfer/usb_submit_urb

[Alan Stern]

On Wed, Mar 26, 2025 at 04:54:09PM +0100, Wolfram Sang wrote:
> Reviewed-by: Wolfram Sang <wsa+renesas@sang-engineering.com>
> 
> Thanks for taking care of it!

Which maintainer should this go through?  Mauro Chebab?

Alan Stern

Re: [PATCH v2] media: dvb: usb: Fix WARNING in dib0700_i2c_xfer/usb_submit_urb

[Wolfram Sang]

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


> Which maintainer should this go through?  Mauro Chebab?

I'd say yes.

$ scripts/get_maintainer.pl -f drivers/media/usb/dvb-usb/dvb-usb-i2c.c
Mauro Carvalho Chehab <mchehab@kernel.org>
linux-media@vger.kernel.org
linux-kernel@vger.kernel.org


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

[PATCH v2 resend] media: dvb: usb: Fix WARNING in dib0700_i2c_xfer/usb_submit_urb

[Alan Stern]

The syzbot fuzzer reported a WARNING related to the dib0700 dvb-usb
driver:

usb 1-1: BOGUS control dir, pipe 80000f80 doesn't match bRequestType c0
WARNING: CPU: 1 PID: 5901 at drivers/usb/core/urb.c:413 usb_submit_urb+0x11d9/0x18c0 drivers/usb/core/urb.c:411
...
Call Trace:
 <TASK>
 usb_start_wait_urb+0x113/0x520 drivers/usb/core/message.c:59
 usb_internal_control_msg drivers/usb/core/message.c:103 [inline]
 usb_control_msg+0x2b1/0x4c0 drivers/usb/core/message.c:154
 dib0700_ctrl_rd drivers/media/usb/dvb-usb/dib0700_core.c:95 [inline]
 dib0700_i2c_xfer_legacy drivers/media/usb/dvb-usb/dib0700_core.c:315 [inline]
 dib0700_i2c_xfer+0xc53/0x1060 drivers/media/usb/dvb-usb/dib0700_core.c:361
 __i2c_transfer+0x866/0x2220
 i2c_transfer+0x271/0x3b0 drivers/i2c/i2c-core-base.c:2315
 i2cdev_ioctl_rdwr+0x452/0x710 drivers/i2c/i2c-dev.c:306
 i2cdev_ioctl+0x759/0x9f0 drivers/i2c/i2c-dev.c:467
 vfs_ioctl fs/ioctl.c:51 [inline]

Evidently the fuzzer submitted an I2C transfer containing a length-0
read message.  The dib0700 driver translated this more or less
literally into a length-0 USB read request.  But the USB protocol does
not allow reads to have length 0; all length-0 transfers are
considered to be writes.  Hence the WARNING above.

Fix the problem by adding the I2C_AQ_NO_ZERO_LEN_READ adapter quirk
flag to all the USB I2C adapter devices managed by dvb-usb-i2c.c,
following Wolfram Sang's suggestion.  This tells the I2C core not to
allow length-0 read messages.

Reported-by: syzbot+c38e5e60d0041a99dbf5@syzkaller.appspotmail.com
Tested-by: syzbot+c38e5e60d0041a99dbf5@syzkaller.appspotmail.com
Suggested-by: Wolfram Sang <wsa+renesas@sang-engineering.com>
Signed-off-by: Alan Stern <stern@rowland.harvard.edu>
Reviewed-by: Wolfram Sang <wsa+renesas@sang-engineering.com>
Link: https://lore.kernel.org/linux-usb/67e1a1f5.050a0220.a7ebc.0029.GAE@google.com/

---

Resend to the media maintainer.

v2: Move the static definition of the i2c_usb_quirks structure outside
the dvb_usb_i2c_init() function.

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

Index: usb-devel/drivers/media/usb/dvb-usb/dvb-usb-i2c.c
===================================================================
--- usb-devel.orig/drivers/media/usb/dvb-usb/dvb-usb-i2c.c
+++ usb-devel/drivers/media/usb/dvb-usb/dvb-usb-i2c.c
@@ -8,6 +8,10 @@
  */
 #include "dvb-usb-common.h"
 
+static const struct i2c_adapter_quirks i2c_usb_quirks = {
+	.flags = I2C_AQ_NO_ZERO_LEN_READ,
+};
+
 int dvb_usb_i2c_init(struct dvb_usb_device *d)
 {
 	int ret = 0;
@@ -24,6 +28,7 @@ int dvb_usb_i2c_init(struct dvb_usb_devi
 	strscpy(d->i2c_adap.name, d->desc->name, sizeof(d->i2c_adap.name));
 	d->i2c_adap.algo      = d->props.i2c_algo;
 	d->i2c_adap.algo_data = NULL;
+	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 resend] media: dvb: usb: Fix WARNING in dib0700_i2c_xfer/usb_submit_urb

[Wolfram Sang]

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

Alan,

> Fix the problem by adding the I2C_AQ_NO_ZERO_LEN_READ adapter quirk
> flag to all the USB I2C adapter devices managed by dvb-usb-i2c.c,
> following Wolfram Sang's suggestion.  This tells the I2C core not to
> allow length-0 read messages.

Thank you again for fixing this issue. There are some USB-I2C bridges in
drivers/i2c/busses which also do not prevent zero len reads. Would it
make sense to put a protection into the I2C core? Can we reliably detect
that an adapter sits on a USB (maybe via the parent device), so that we
can then check if I2C_AQ_NO_ZERO_LEN_READ is set, and take action if
not?

Appreciating your input,

   Wolfram


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

Re: [PATCH v2 resend] media: dvb: usb: Fix WARNING in dib0700_i2c_xfer/usb_submit_urb

[Alan Stern]

On Fri, Mar 28, 2025 at 04:45:10PM +0100, Wolfram Sang wrote:
> Alan,
> 
> > Fix the problem by adding the I2C_AQ_NO_ZERO_LEN_READ adapter quirk
> > flag to all the USB I2C adapter devices managed by dvb-usb-i2c.c,
> > following Wolfram Sang's suggestion.  This tells the I2C core not to
> > allow length-0 read messages.
> 
> Thank you again for fixing this issue. There are some USB-I2C bridges in
> drivers/i2c/busses which also do not prevent zero len reads. Would it
> make sense to put a protection into the I2C core? Can we reliably detect
> that an adapter sits on a USB (maybe via the parent device), so that we
> can then check if I2C_AQ_NO_ZERO_LEN_READ is set, and take action if
> not?

This really should be handled by someone who knows how those bridges 
work.

In the case of dib0700, it was clear from the source code that the 
driver uses USB Control transfers to tell the hardware about I2C 
messages.  I don't know if other bridges work in the same way.  In 
theory a bridge could use USB Bulk transfers instead; they aren't 
subject to this restriction on length-0 reads.  Or a bridge could use a 
Control read transfer but include extra header material along with the 
actual data, so that a length-0 message wouldn't end up generating a 
length-0 read.

So the short answer is that you would need to find someone who really 
understands what's going on here -- which I don't.  Sorry.

Alan Stern

Re: [PATCH v2 resend] media: dvb: usb: Fix WARNING in dib0700_i2c_xfer/usb_submit_urb

[Wolfram Sang]

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


> In the case of dib0700, it was clear from the source code that the 
> driver uses USB Control transfers to tell the hardware about I2C 
> messages.  I don't know if other bridges work in the same way.  In 
> theory a bridge could use USB Bulk transfers instead; they aren't 
> subject to this restriction on length-0 reads.  Or a bridge could use a 
> Control read transfer but include extra header material along with the 
> actual data, so that a length-0 message wouldn't end up generating a 
> length-0 read.

Fully understood, thanks for your explanation.

> So the short answer is that you would need to find someone who really 
> understands what's going on here -- which I don't.  Sorry.

No worries. There are only 5 drivers or so, I will manually check if
they use a control_read and have no own header. Doesn't sound hard.


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

Re: [PATCH v2 resend] media: dvb: usb: Fix WARNING in dib0700_i2c_xfer/usb_submit_urb

[Alan Stern]

On Sat, Mar 29, 2025 at 07:05:49AM +0100, Wolfram Sang wrote:
> 
> > In the case of dib0700, it was clear from the source code that the 
> > driver uses USB Control transfers to tell the hardware about I2C 
> > messages.  I don't know if other bridges work in the same way.  In 
> > theory a bridge could use USB Bulk transfers instead; they aren't 
> > subject to this restriction on length-0 reads.  Or a bridge could use a 
> > Control read transfer but include extra header material along with the 
> > actual data, so that a length-0 message wouldn't end up generating a 
> > length-0 read.
> 
> Fully understood, thanks for your explanation.
> 
> > So the short answer is that you would need to find someone who really 
> > understands what's going on here -- which I don't.  Sorry.
> 
> No worries. There are only 5 drivers or so, I will manually check if
> they use a control_read and have no own header. Doesn't sound hard.

Good...  Feel free to ask me if you have any questions or need any other 
help.

Alan Stern