Hi David, On 4/4/22 18:10, David Kahurani wrote:
Reads that are lesser than the requested size lead to uninit-value bugs. Qualify such reads as errors and handle them correctly. ax88179_178a 2-1:0.35 (unnamed net_device) (uninitialized): Failed to read reg index 0x0001: -71 ax88179_178a 2-1:0.35 (unnamed net_device) (uninitialized): Failed to read reg index 0x0002: -71 ===================================================== BUG: KMSAN: uninit-value in ax88179_check_eeprom drivers/net/usb/ax88179_178a.c:1074 [inline] BUG: KMSAN: uninit-value in ax88179_led_setting+0x884/0x30b0 drivers/net/usb/ax88179_178a.c:1168 ax88179_check_eeprom drivers/net/usb/ax88179_178a.c:1074 [inline] ax88179_led_setting+0x884/0x30b0 drivers/net/usb/ax88179_178a.c:1168 ax88179_bind+0xe75/0x1990 drivers/net/usb/ax88179_178a.c:1411 usbnet_probe+0x1284/0x4140 drivers/net/usb/usbnet.c:1747 usb_probe_interface+0xf19/0x1600 drivers/usb/core/driver.c:396 really_probe+0x67d/0x1510 drivers/base/dd.c:596 __driver_probe_device+0x3e9/0x530 drivers/base/dd.c:751 driver_probe_device drivers/base/dd.c:781 [inline] __device_attach_driver+0x79f/0x1120 drivers/base/dd.c:898 bus_for_each_drv+0x2d6/0x3f0 drivers/base/bus.c:427 __device_attach+0x593/0x8e0 drivers/base/dd.c:969 device_initial_probe+0x4a/0x60 drivers/base/dd.c:1016 bus_probe_device+0x17b/0x3e0 drivers/base/bus.c:487 device_add+0x1d3e/0x2400 drivers/base/core.c:3394 usb_set_configuration+0x37e9/0x3ed0 drivers/usb/core/message.c:2170 usb_generic_driver_probe+0x13c/0x300 drivers/usb/core/generic.c:238 usb_probe_device+0x309/0x570 drivers/usb/core/driver.c:293 really_probe+0x67d/0x1510 drivers/base/dd.c:596 __driver_probe_device+0x3e9/0x530 drivers/base/dd.c:751 driver_probe_device drivers/base/dd.c:781 [inline] __device_attach_driver+0x79f/0x1120 drivers/base/dd.c:898 bus_for_each_drv+0x2d6/0x3f0 drivers/base/bus.c:427 __device_attach+0x593/0x8e0 drivers/base/dd.c:969 device_initial_probe+0x4a/0x60 drivers/base/dd.c:1016
I'd personally cut this log a bit and would add this part of the initial report
Local variable eeprom.i created at: ax88179_check_eeprom drivers/net/usb/ax88179_178a.c:1045 [inline] ax88179_led_setting+0x2e2/0x30b0 drivers/net/usb/ax88179_178a.c:1168 ax88179_bind+0xe75/0x1990 drivers/net/usb/ax88179_178a.c:1411 Since it shows exactly where problem comes from. I do not insist, just IMO
Signed-off-by: David Kahurani <k.kahurani@xxxxxxxxx> Reported-by: syzbot+d3dbdf31fbe9d8f5f311@xxxxxxxxxxxxxxxxxxxxxxxxx ---
It's indeed a bug fix, so fixes tag is neededFixes: e2ca90c276e1 ("ax88179_178a: ASIX AX88179_178A USB 3.0/2.0 to gigabit ethernet adapter driver")
drivers/net/usb/ax88179_178a.c | 255 +++++++++++++++++++++++++++------ 1 file changed, 213 insertions(+), 42 deletions(-) diff --git a/drivers/net/usb/ax88179_178a.c b/drivers/net/usb/ax88179_178a.c index e2fa56b92..b5e114bed 100644 --- a/drivers/net/usb/ax88179_178a.c +++ b/drivers/net/usb/ax88179_178a.c @@ -185,8 +185,9 @@ static const struct { {7, 0xcc, 0x4c, 0x18, 8}, };-static int __ax88179_read_cmd(struct usbnet *dev, u8 cmd, u16 value, u16 index,- u16 size, void *data, int in_pm) +static int __must_check __ax88179_read_cmd(struct usbnet *dev, u8 cmd, + u16 value, u16 index, u16 size, + void *data, int in_pm) { int ret; int (*fn)(struct usbnet *, u8, u8, u16, u16, void *, u16); @@ -201,9 +202,12 @@ static int __ax88179_read_cmd(struct usbnet *dev, u8 cmd, u16 value, u16 index, ret = fn(dev, cmd, USB_DIR_IN | USB_TYPE_VENDOR | USB_RECIP_DEVICE, value, index, data, size);- if (unlikely(ret < 0))+ if (unlikely(ret < size)) { + ret = ret < 0 ? ret : -ENODATA; + netdev_warn(dev->net, "Failed to read reg index 0x%04x: %d\n", index, ret); + }return ret;} @@ -249,19 +253,26 @@ static void ax88179_write_cmd_async(struct usbnet *dev, u8 cmd, u16 value, } }-static int ax88179_read_cmd_nopm(struct usbnet *dev, u8 cmd, u16 value,- u16 index, u16 size, void *data) +static int __must_check ax88179_read_cmd_nopm(struct usbnet *dev, u8 cmd, + u16 value, u16 index, u16 size, + void *data) { int ret;if (2 == size) {u16 buf; ret = __ax88179_read_cmd(dev, cmd, value, index, size, &buf, 1); + + if (ret < 0) + return ret;
Empty line after assignment and before check looks redundant.
le16_to_cpus(&buf);
*((u16 *)data) = buf;
} else if (4 == size) {
u32 buf;
ret = __ax88179_read_cmd(dev, cmd, value, index, size, &buf, 1);
+
+ if (ret < 0)
+ return ret;
Same hereIn general, looks good, but, please, fix up indenting and other small issues.
With regards, Pavel Skripkin
Attachment:
OpenPGP_signature
Description: OpenPGP digital signature
