All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH 0/2] media: rtl28xxu: fix regression in linux-next
@ 2021-06-23  8:45 Johan Hovold
  2021-06-23  8:45 ` [PATCH 1/2] Revert "media: rtl28xxu: fix zero-length control request" Johan Hovold
  2021-06-23  8:45 ` [PATCH 2/2] media: rtl28xxu: fix zero-length control request Johan Hovold
  0 siblings, 2 replies; 3+ messages in thread
From: Johan Hovold @ 2021-06-23  8:45 UTC (permalink / raw)
  To: Mauro Carvalho Chehab, Hans Verkuil
  Cc: Antti Palosaari, Eero Lehtinen, linux-media, linux-kernel, Johan Hovold

A patch addressing a new USB core warning in rtl28xxu ended up causing a
regression. As this was caught before the patch was committed it was
superseded by a v2. Unfortunately, the first erroneous version ended up
being applied to the media tree anyway.

This reverts the first version of the patch and resends the second
version.

Mauro and Hans, I've reported this issue to you both repeatedly over the
course of three weeks but you never replied to any of mails. I'm not
sure what went wrong, but I would assume that regressions would be
handled with a bit more urgency. Are you not receiving my mails or was
this perhaps a case of it not being clear who of you should act on the
reports?

2021-05-24: v1 posted

	https://lore.kernel.org/r/20210524110920.24599-4-johan@kernel.org

2021-05-31: Request not apply v1

	https://lore.kernel.org/r/YLSWeyy1skooTmqD@hovoldconsulting.com

2021-05-31: v2 posted

	https://lore.kernel.org/r/20210531094434.12651-4-johan@kernel.org

2021-06-02: v1 applied to media tree and a request to drop it was sent
            in response to the automatic notification mail

2021-06-07: Second request to drop v1 with a question of how best to
            proceed to fix the regression otherwise

	https://lore.kernel.org/r/YL3MCGY5wTsW2kEF@hovoldconsulting.com

Johan


Johan Hovold (2):
  Revert "media: rtl28xxu: fix zero-length control request"
  media: rtl28xxu: fix zero-length control request

 drivers/media/usb/dvb-usb-v2/rtl28xxu.c | 14 +++++++++++---
 1 file changed, 11 insertions(+), 3 deletions(-)

-- 
2.31.1


^ permalink raw reply	[flat|nested] 3+ messages in thread

* [PATCH 1/2] Revert "media: rtl28xxu: fix zero-length control request"
  2021-06-23  8:45 [PATCH 0/2] media: rtl28xxu: fix regression in linux-next Johan Hovold
@ 2021-06-23  8:45 ` Johan Hovold
  2021-06-23  8:45 ` [PATCH 2/2] media: rtl28xxu: fix zero-length control request Johan Hovold
  1 sibling, 0 replies; 3+ messages in thread
From: Johan Hovold @ 2021-06-23  8:45 UTC (permalink / raw)
  To: Mauro Carvalho Chehab, Hans Verkuil
  Cc: Antti Palosaari, Eero Lehtinen, linux-media, linux-kernel, Johan Hovold

This reverts commit 25d5ce3a606a1eb23a9265d615a92a876ff9cb5f.

The patch in question causes a regression and was superseded by a second
version. Unfortunately, the first revision ended up being applied
instead of the correct one.

Link: https://lore.kernel.org/r/YL3MCGY5wTsW2kEF@hovoldconsulting.com
Signed-off-by: Johan Hovold <johan@kernel.org>
---
 drivers/media/usb/dvb-usb-v2/rtl28xxu.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/drivers/media/usb/dvb-usb-v2/rtl28xxu.c b/drivers/media/usb/dvb-usb-v2/rtl28xxu.c
index 83705730e37e..0cbdb95f8d35 100644
--- a/drivers/media/usb/dvb-usb-v2/rtl28xxu.c
+++ b/drivers/media/usb/dvb-usb-v2/rtl28xxu.c
@@ -612,9 +612,8 @@ static int rtl28xxu_read_config(struct dvb_usb_device *d)
 static int rtl28xxu_identify_state(struct dvb_usb_device *d, const char **name)
 {
 	struct rtl28xxu_dev *dev = d_to_priv(d);
-	u8 buf[1];
 	int ret;
-	struct rtl28xxu_req req_demod_i2c = {0x0020, CMD_I2C_DA_RD, 1, buf};
+	struct rtl28xxu_req req_demod_i2c = {0x0020, CMD_I2C_DA_RD, 0, NULL};
 
 	dev_dbg(&d->intf->dev, "\n");
 
-- 
2.31.1


^ permalink raw reply related	[flat|nested] 3+ messages in thread

* [PATCH 2/2] media: rtl28xxu: fix zero-length control request
  2021-06-23  8:45 [PATCH 0/2] media: rtl28xxu: fix regression in linux-next Johan Hovold
  2021-06-23  8:45 ` [PATCH 1/2] Revert "media: rtl28xxu: fix zero-length control request" Johan Hovold
@ 2021-06-23  8:45 ` Johan Hovold
  1 sibling, 0 replies; 3+ messages in thread
From: Johan Hovold @ 2021-06-23  8:45 UTC (permalink / raw)
  To: Mauro Carvalho Chehab, Hans Verkuil
  Cc: Antti Palosaari, Eero Lehtinen, linux-media, linux-kernel,
	Johan Hovold, syzbot+faf11bbadc5a372564da, stable

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@gmail.com>.

Reported-by: syzbot+faf11bbadc5a372564da@syzkaller.appspotmail.com
Reported-by: Eero Lehtinen <debiangamer2@gmail.com>
Tested-by: Eero Lehtinen <debiangamer2@gmail.com>
Fixes: d0f232e823af ("[media] rtl28xxu: add heuristic to detect chip type")
Cc: stable@vger.kernel.org      # 4.0
Cc: Antti Palosaari <crope@iki.fi>
Signed-off-by: Johan Hovold <johan@kernel.org>
---
 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 0cbdb95f8d35..795a012d4020 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


^ permalink raw reply related	[flat|nested] 3+ messages in thread

end of thread, other threads:[~2021-06-23  8:46 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-06-23  8:45 [PATCH 0/2] media: rtl28xxu: fix regression in linux-next Johan Hovold
2021-06-23  8:45 ` [PATCH 1/2] Revert "media: rtl28xxu: fix zero-length control request" Johan Hovold
2021-06-23  8:45 ` [PATCH 2/2] media: rtl28xxu: fix zero-length control request Johan Hovold

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.