Skip to content

Commit

Permalink
net: asix: add proper error handling of usb read errors
Browse files Browse the repository at this point in the history
Syzbot once again hit uninit value in asix driver. The problem still the
same -- asix_read_cmd() reads less bytes, than was requested by caller.

Since all read requests are performed via asix_read_cmd() let's catch
usb related error there and add __must_check notation to be sure all
callers actually check return value.

So, this patch adds sanity check inside asix_read_cmd(), that simply
checks if bytes read are not less, than was requested and adds missing
error handling of asix_read_cmd() all across the driver code.

Fixes: d9fe64e ("net: asix: Add in_pm parameter")
Reported-and-tested-by: syzbot+6ca9f7867b77c2d316ac@syzkaller.appspotmail.com
Signed-off-by: Pavel Skripkin <paskripkin@gmail.com>
Tested-by: Oleksij Rempel <o.rempel@pengutronix.de>
Reviewed-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Signed-off-by: David S. Miller <davem@davemloft.net>
  • Loading branch information
Pavel Skripkin authored and David S. Miller committed Feb 7, 2022
1 parent b845bac commit 920a9fa
Show file tree
Hide file tree
Showing 3 changed files with 33 additions and 11 deletions.
4 changes: 2 additions & 2 deletions drivers/net/usb/asix.h
Original file line number Diff line number Diff line change
Expand Up @@ -192,8 +192,8 @@ extern const struct driver_info ax88172a_info;
/* ASIX specific flags */
#define FLAG_EEPROM_MAC (1UL << 0) /* init device MAC from eeprom */

int asix_read_cmd(struct usbnet *dev, u8 cmd, u16 value, u16 index,
u16 size, void *data, int in_pm);
int __must_check asix_read_cmd(struct usbnet *dev, u8 cmd, u16 value, u16 index,
u16 size, void *data, int in_pm);

int asix_write_cmd(struct usbnet *dev, u8 cmd, u16 value, u16 index,
u16 size, void *data, int in_pm);
Expand Down
19 changes: 13 additions & 6 deletions drivers/net/usb/asix_common.c
Original file line number Diff line number Diff line change
Expand Up @@ -11,8 +11,8 @@

#define AX_HOST_EN_RETRIES 30

int asix_read_cmd(struct usbnet *dev, u8 cmd, u16 value, u16 index,
u16 size, void *data, int in_pm)
int __must_check asix_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);
Expand All @@ -27,9 +27,12 @@ int asix_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;
}
Expand Down Expand Up @@ -79,7 +82,7 @@ static int asix_check_host_enable(struct usbnet *dev, int in_pm)
0, 0, 1, &smsr, in_pm);
if (ret == -ENODEV)
break;
else if (ret < sizeof(smsr))
else if (ret < 0)
continue;
else if (smsr & AX_HOST_EN)
break;
Expand Down Expand Up @@ -579,8 +582,12 @@ int asix_mdio_read_nopm(struct net_device *netdev, int phy_id, int loc)
return ret;
}

asix_read_cmd(dev, AX_CMD_READ_MII_REG, phy_id,
(__u16)loc, 2, &res, 1);
ret = asix_read_cmd(dev, AX_CMD_READ_MII_REG, phy_id,
(__u16)loc, 2, &res, 1);
if (ret < 0) {
mutex_unlock(&dev->phy_mutex);
return ret;
}
asix_set_hw_mii(dev, 1);
mutex_unlock(&dev->phy_mutex);

Expand Down
21 changes: 18 additions & 3 deletions drivers/net/usb/asix_devices.c
Original file line number Diff line number Diff line change
Expand Up @@ -755,7 +755,12 @@ static int ax88772_bind(struct usbnet *dev, struct usb_interface *intf)
priv->phy_addr = ret;
priv->embd_phy = ((priv->phy_addr & 0x1f) == 0x10);

asix_read_cmd(dev, AX_CMD_STATMNGSTS_REG, 0, 0, 1, &chipcode, 0);
ret = asix_read_cmd(dev, AX_CMD_STATMNGSTS_REG, 0, 0, 1, &chipcode, 0);
if (ret < 0) {
netdev_dbg(dev->net, "Failed to read STATMNGSTS_REG: %d\n", ret);
return ret;
}

chipcode &= AX_CHIPCODE_MASK;

ret = (chipcode == AX_AX88772_CHIPCODE) ? ax88772_hw_reset(dev, 0) :
Expand Down Expand Up @@ -919,11 +924,21 @@ static int ax88178_reset(struct usbnet *dev)
int gpio0 = 0;
u32 phyid;

asix_read_cmd(dev, AX_CMD_READ_GPIOS, 0, 0, 1, &status, 0);
ret = asix_read_cmd(dev, AX_CMD_READ_GPIOS, 0, 0, 1, &status, 0);
if (ret < 0) {
netdev_dbg(dev->net, "Failed to read GPIOS: %d\n", ret);
return ret;
}

netdev_dbg(dev->net, "GPIO Status: 0x%04x\n", status);

asix_write_cmd(dev, AX_CMD_WRITE_ENABLE, 0, 0, 0, NULL, 0);
asix_read_cmd(dev, AX_CMD_READ_EEPROM, 0x0017, 0, 2, &eeprom, 0);
ret = asix_read_cmd(dev, AX_CMD_READ_EEPROM, 0x0017, 0, 2, &eeprom, 0);
if (ret < 0) {
netdev_dbg(dev->net, "Failed to read EEPROM: %d\n", ret);
return ret;
}

asix_write_cmd(dev, AX_CMD_WRITE_DISABLE, 0, 0, 0, NULL, 0);

netdev_dbg(dev->net, "EEPROM index 0x17 is 0x%04x\n", eeprom);
Expand Down

0 comments on commit 920a9fa

Please sign in to comment.