[3/3] media: rtl28xxu: fix zero-length control request

Message ID 20210524110920.24599-4-johan@kernel.org (mailing list archive)
State Accepted, archived
Delegated to: Hans Verkuil
Headers
Series media: fix zero-length USB control requests |

Commit Message

Johan Hovold May 24, 2021, 11:09 a.m. UTC
  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.

Fix the zero-length i2c-read request used for type detection by
attempting to read a single byte instead.

Reported-by: syzbot+faf11bbadc5a372564da@syzkaller.appspotmail.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 | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)
  

Comments

Johan Hovold May 31, 2021, 7:55 a.m. UTC | #1
On Mon, May 24, 2021 at 01:09:20PM +0200, Johan Hovold wrote:
> 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.
> 
> Fix the zero-length i2c-read request used for type detection by
> attempting to read a single byte instead.
> 
> Reported-by: syzbot+faf11bbadc5a372564da@syzkaller.appspotmail.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 | 3 ++-
>  1 file changed, 2 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..2c04ed8af0e4 100644
> --- a/drivers/media/usb/dvb-usb-v2/rtl28xxu.c
> +++ b/drivers/media/usb/dvb-usb-v2/rtl28xxu.c
> @@ -612,8 +612,9 @@ 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, 0, NULL};
> +	struct rtl28xxu_req req_demod_i2c = {0x0020, CMD_I2C_DA_RD, 1, buf};
>  
>  	dev_dbg(&d->intf->dev, "\n");

As reported here

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

this patch is causing the chip type to no longer be detected correctly,
so please drop this one for now until this has been resolved.

Johan
  
Johan Hovold June 7, 2021, 7:34 a.m. UTC | #2
On Mon, May 31, 2021 at 09:55:39AM +0200, Johan Hovold wrote:
> On Mon, May 24, 2021 at 01:09:20PM +0200, Johan Hovold wrote:
> > 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.
> > 
> > Fix the zero-length i2c-read request used for type detection by
> > attempting to read a single byte instead.
> > 
> > Reported-by: syzbot+faf11bbadc5a372564da@syzkaller.appspotmail.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 | 3 ++-
> >  1 file changed, 2 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..2c04ed8af0e4 100644
> > --- a/drivers/media/usb/dvb-usb-v2/rtl28xxu.c
> > +++ b/drivers/media/usb/dvb-usb-v2/rtl28xxu.c
> > @@ -612,8 +612,9 @@ 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, 0, NULL};
> > +	struct rtl28xxu_req req_demod_i2c = {0x0020, CMD_I2C_DA_RD, 1, buf};
> >  
> >  	dev_dbg(&d->intf->dev, "\n");
> 
> As reported here
> 
> 	https://lore.kernel.org/r/YLSVsrhMZ2oOL1vM@hovoldconsulting.com
> 
> this patch is causing the chip type to no longer be detected correctly,
> so please drop this one for now until this has been resolved.

Looks like this one was applied to the media tree a couple of days after
I sent this nonetheless.

Can you drop this one in favour of the v2 posted here:

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

or do you want me to send an incremental fix instead?

Johan
  

Patch

diff --git a/drivers/media/usb/dvb-usb-v2/rtl28xxu.c b/drivers/media/usb/dvb-usb-v2/rtl28xxu.c
index 97ed17a141bb..2c04ed8af0e4 100644
--- a/drivers/media/usb/dvb-usb-v2/rtl28xxu.c
+++ b/drivers/media/usb/dvb-usb-v2/rtl28xxu.c
@@ -612,8 +612,9 @@  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, 0, NULL};
+	struct rtl28xxu_req req_demod_i2c = {0x0020, CMD_I2C_DA_RD, 1, buf};
 
 	dev_dbg(&d->intf->dev, "\n");