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

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