Re: [PATCH] media: rtl28xxu: add type-detection instrumentation

[Johan Hovold <johan@kernel.org>]

On Wed, Jun 02, 2021 at 02:01:02PM +0300, Antti Palosaari wrote:
> 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@kernel.org> wrote:

> >>>  From 2cec8fa000152bcb997dd7aeeb0917ebf744a7bd Mon Sep 17 00:00:00 2001
> >>> From: Johan Hovold <johan@kernel.org>
> >>> 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@gmail.com>.
> >>>
> >>> Reported-by: syzbot+faf11bbadc5a372564da@syzkaller.appspotmail.com
> >>> Reported-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 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.

Thanks for testing, Antti.

Johan