Messages in this thread | | | Date | Fri, 6 May 2022 15:12:13 +0300 | Subject | Re: [PATCH v3 1/3] staging: rtl8712: fix uninit-value in usb_read8() and friends | From | Pavel Skripkin <> |
| |
Hi Wang,
On 5/6/22 14:59, Wang Cheng wrote: > Reported-and-tested-by: syzbot+6f5ecd144854c0d8580b@syzkaller.appspotmail.com > Signed-off-by: Wang Cheng <wanngchenng@gmail.com> > --- > drivers/staging/rtl8712/usb_ops.c | 27 ++++++++++++++++++--------- > 1 file changed, 18 insertions(+), 9 deletions(-) > > diff --git a/drivers/staging/rtl8712/usb_ops.c b/drivers/staging/rtl8712/usb_ops.c > index e64845e6adf3..af9966d03979 100644 > --- a/drivers/staging/rtl8712/usb_ops.c > +++ b/drivers/staging/rtl8712/usb_ops.c > @@ -29,7 +29,8 @@ static u8 usb_read8(struct intf_hdl *intfhdl, u32 addr) > u16 wvalue; > u16 index; > u16 len; > - __le32 data; > + int status; > + __le32 data = 0; > struct intf_priv *intfpriv = intfhdl->pintfpriv; > > request = 0x05; > @@ -37,8 +38,10 @@ static u8 usb_read8(struct intf_hdl *intfhdl, u32 addr) > index = 0; > wvalue = (u16)(addr & 0x0000ffff); > len = 1; > - r8712_usbctrl_vendorreq(intfpriv, request, wvalue, index, &data, len, > - requesttype); > + status = r8712_usbctrl_vendorreq(intfpriv, request, wvalue, index, > + &data, len, requesttype); > + if (status < 0) > + return 0;
Wait, but what about partial reads? I see that r8712_usbctrl_vendorreq() uses `usb_control_msg()` which can read less data than was requested.
So here you are just hiding these error by zeroing data
> return (u8)(le32_to_cpu(data) & 0x0ff); > } >
And as i said in previous email: caller can't see an error.
Imagine caller waits for some register to become 0. Caller sees that usb_read8() returned 0. Is it an error? Is register really become 0?
Generic read API prototype looks like following:
int read_something(struct my_cool_dev *dev, void *data, size_t size);
and it returns 0 on success and -errno on failure and data returned via passed pointer. So, if API returned an error caller should not touch @data, since likely it's uninitialized
With regards, Pavel Skripkin [unhandled content-type:application/pgp-signature] | |