[PATCH] media: dib0700: reject zero-length I2C reads in both transfer paths

Luyao Bai posted 1 patch 1 day ago
drivers/media/usb/dvb-usb/dib0700_core.c | 14 ++++++++++++++
1 file changed, 14 insertions(+)
[PATCH] media: dib0700: reject zero-length I2C reads in both transfer paths
Posted by Luyao Bai 1 day ago
A zero-length I2C read (I2C_M_RD with len == 0), issued from user space
through /dev/i2c-N with the I2C_RDWR ioctl, is translated by the dib0700
bridge into a control-IN usb_control_msg() whose wLength is the I2C
message length, i.e. 0. usb_submit_urb() treats a zero-length control
transfer as OUT, so the pipe direction (IN) and the setup-packet
direction (bRequestType = 0xc0, IN) disagree and trip the WARN() in
usb_submit_urb():

  usb 4-1: BOGUS control dir, pipe 80000280 doesn't match bRequestType c0
  WARNING: drivers/usb/core/urb.c:411 at usb_submit_urb+0x1573/0x1910
   usb_submit_urb
   usb_start_wait_urb
   usb_control_msg
   dib0700_ctrl_rd drivers/media/usb/dvb-usb/dib0700_core.c:95
   dib0700_i2c_xfer_legacy drivers/media/usb/dvb-usb/dib0700_core.c:315
   dib0700_i2c_xfer drivers/media/usb/dvb-usb/dib0700_core.c:361
   __i2c_transfer
   i2c_transfer
   i2cdev_ioctl_rdwr drivers/i2c/i2c-dev.c:306

The .master_xfer entry point dib0700_i2c_xfer() dispatches to one of two
transfer routines depending on the device firmware: dib0700_i2c_xfer_new()
(firmware >= 1.20) and dib0700_i2c_xfer_legacy(). Both build a control-IN
usb_control_msg() from the I2C read length, so both can submit the bogus
zero-length transfer; the syzbot reproducer happens to take the legacy
path. Guarding a single path therefore leaves the other exposed.

Reject zero-length reads in the shared dispatcher, before the firmware
split, so neither path can submit such a transfer. -EOPNOTSUPP is
returned because the adapter cannot express a zero-length read, matching
the error the i2c core returns for the I2C_AQ_NO_ZERO_LEN_READ quirk.

The i2c core can enforce this centrally via the I2C_AQ_NO_ZERO_LEN_READ
adapter quirk, but the dib0700 bridge reuses the i2c adapter registered
by the dvb-usb core (dvb-usb-i2c.c), and the dvb-usb framework provides
no hook for an individual driver to set adapter quirks; using the quirk
would require a dvb-usb framework change. Rejecting the read in the
driver's own .master_xfer keeps the fix local and minimal.

Reported-by: syzbot+0cd0fb4cf3f4722d6663@syzkaller.appspotmail.com
Closes: https://syzkaller.appspot.com/bug?extid=0cd0fb4cf3f4722d6663
Fixes: b7f54910ce01 ("V4L/DVB (4647): Added module for DiB0700 based devices")
Signed-off-by: Luyao Bai <bailuyao1997@gmail.com>
---
This is an alternative to Henri A's patch for the same syzbot report [1],
which places the guard inside dib0700_i2c_xfer_legacy(). That fix is
correct for the reproducer, but dib0700_i2c_xfer_new() builds its
control-IN transfer from the read length in the same way, so a device
running firmware >= 1.20 can still submit the bogus zero-length transfer.
Checking in the shared .master_xfer entry point covers both paths with a
single guard.

Henri: happy for you to fold the second path into a v2 of yours instead if
you prefer, whichever lands the complete fix.

Tested with the syzbot reproducer under QEMU: the "BOGUS control dir"
WARNING at drivers/usb/core/urb.c fires on a clean mainline build and no
longer fires with this patch applied.

[1] https://lore.kernel.org/linux-media/20260621192222.337738-1-contact@henrialfonso.com/

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

diff --git a/drivers/media/usb/dvb-usb/dib0700_core.c b/drivers/media/usb/dvb-usb/dib0700_core.c
index 1caabb51ea47..084c56e734e6 100644
--- a/drivers/media/usb/dvb-usb/dib0700_core.c
+++ b/drivers/media/usb/dvb-usb/dib0700_core.c
@@ -352,6 +352,20 @@ static int dib0700_i2c_xfer(struct i2c_adapter *adap, struct i2c_msg *msg,
 {
 	struct dvb_usb_device *d = i2c_get_adapdata(adap);
 	struct dib0700_state *st = d->priv;
+	int i;
+
+	/*
+	 * Both transfer paths translate an I2C read into a control-IN
+	 * usb_control_msg() whose wLength is the message length. A
+	 * zero-length read produces a control URB whose pipe direction
+	 * (IN) disagrees with a zero-length setup packet (which
+	 * usb_submit_urb() treats as OUT), tripping its "BOGUS control
+	 * dir" WARN(). Reject such reads up front so neither path can
+	 * submit one.
+	 */
+	for (i = 0; i < num; i++)
+		if ((msg[i].flags & I2C_M_RD) && msg[i].len == 0)
+			return -EOPNOTSUPP;

 	if (st->fw_use_new_i2c_api == 1) {
 		/* User running at least fw 1.20 */
--
2.43.0