On 5/31/21 12:13 PM, Johan Hovold wrote:
On Mon, May 31, 2021 at 12:08:20PM +0300, Eero Lehtinen wrote:
On Mon, May 31, 2021 at 11:42 AM Johan Hovold <johan@xxxxxxxxxx> wrote:
Ok, the driver just wants to know if the i2c-read vendor request exists,
and actually reading the register will not work since the register may
not even exist (e.g. depending on the demodulator).
So it seems we need to keep this zero-length read request and only
update the pipe argument to suppress the new WARN() in
usb_submit_urb().
Eero, could you try applying the below on top of -next and confirm that
it suppresses the warning without messing up the type detection?
From 2cec8fa000152bcb997dd7aeeb0917ebf744a7bd Mon Sep 17 00:00:00 2001
From: Johan Hovold <johan@xxxxxxxxxx>
Date: Mon, 24 May 2021 10:55:19 +0200
Subject: [PATCH v2] media: rtl28xxu: fix zero-length control request
The direction of the pipe argument must match the request-type direction
bit or control requests may fail depending on the host-controller-driver
implementation.
Control transfers without a data stage are treated as OUT requests by
the USB stack and should be using usb_sndctrlpipe(). Failing to do so
will now trigger a warning.
The driver uses a zero-length i2c-read request for type detection so
update the control-request code to use usb_sndctrlpipe() in this case.
Note that actually trying to read the i2c register in question does not
work as the register might not exist (e.g. depending on the demodulator)
as reported by Eero Lehtinen <debiangamer2@xxxxxxxxx>.
Reported-by: syzbot+faf11bbadc5a372564da@xxxxxxxxxxxxxxxxxxxxxxxxx
Reported-by: Eero Lehtinen <debiangamer2@xxxxxxxxx>
Fixes: d0f232e823af ("[media] rtl28xxu: add heuristic to detect chip type")
Cc: stable@xxxxxxxxxxxxxxx # 4.0
Cc: Antti Palosaari <crope@xxxxxx>
Signed-off-by: Johan Hovold <johan@xxxxxxxxxx>
---
drivers/media/usb/dvb-usb-v2/rtl28xxu.c | 11 ++++++++++-
1 file changed, 10 insertions(+), 1 deletion(-)
diff --git a/drivers/media/usb/dvb-usb-v2/rtl28xxu.c b/drivers/media/usb/dvb-usb-v2/rtl28xxu.c
index 97ed17a141bb..a6124472cb06 100644
--- a/drivers/media/usb/dvb-usb-v2/rtl28xxu.c
+++ b/drivers/media/usb/dvb-usb-v2/rtl28xxu.c
@@ -37,7 +37,16 @@ static int rtl28xxu_ctrl_msg(struct dvb_usb_device *d, struct rtl28xxu_req *req)
} else {
/* read */
requesttype = (USB_TYPE_VENDOR | USB_DIR_IN);
- pipe = usb_rcvctrlpipe(d->udev, 0);
+
+ /*
+ * Zero-length transfers must use usb_sndctrlpipe() and
+ * rtl28xxu_identify_state() uses a zero-length i2c read
+ * command to determine the chip type.
+ */
+ if (req->size)
+ pipe = usb_rcvctrlpipe(d->udev, 0);
+ else
+ pipe = usb_sndctrlpipe(d->udev, 0);
}
ret = usb_control_msg(d->udev, pipe, 0, requesttype, req->value,
--
2.31.1
I confirm that it suppresses the warning without messing up the type
detection.
Thanks for confirming. Is it ok if I add also a tested-by tag for you to
the commit message when I send this to the media people?
I can also confirm it works for both rtl2831u and rtl2832u.
regards
Antti
