Hi Wang, On 5/6/22 14:59, Wang Cheng wrote:
Reported-and-tested-by: syzbot+6f5ecd144854c0d8580b@xxxxxxxxxxxxxxxxxxxxxxxxx Signed-off-by: Wang Cheng <wanngchenng@xxxxxxxxx> --- 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
Attachment:
OpenPGP_signature
Description: OpenPGP digital signature
