Commit a786e319 authored by Pavel Skripkin's avatar Pavel Skripkin Committed by David S. Miller

net: asix: fix uninit value bugs

Syzbot reported uninit-value in asix_mdio_read(). The problem was in
missing error handling. asix_read_cmd() should initialize passed stack
variable smsr, but it can fail in some cases. Then while condidition
checks possibly uninit smsr variable.

Since smsr is uninitialized stack variable, driver can misbehave,
because smsr will be random in case of asix_read_cmd() failure.
Fix it by adding error handling and just continue the loop instead of
checking uninit value.

Added helper function for checking Host_En bit, since wrong loop was used
in 4 functions and there is no need in copy-pasting code parts.

Cc: Robert Foss <robert.foss@collabora.com>
Fixes: d9fe64e5 ("net: asix: Add in_pm parameter")
Reported-by: syzbot+a631ec9e717fb0423053@syzkaller.appspotmail.com
Signed-off-by: default avatarPavel Skripkin <paskripkin@gmail.com>
Signed-off-by: default avatarDavid S. Miller <davem@davemloft.net>
parent 01634047
...@@ -63,6 +63,29 @@ void asix_write_cmd_async(struct usbnet *dev, u8 cmd, u16 value, u16 index, ...@@ -63,6 +63,29 @@ void asix_write_cmd_async(struct usbnet *dev, u8 cmd, u16 value, u16 index,
value, index, data, size); value, index, data, size);
} }
static int asix_check_host_enable(struct usbnet *dev, int in_pm)
{
int i, ret;
u8 smsr;
for (i = 0; i < 30; ++i) {
ret = asix_set_sw_mii(dev, in_pm);
if (ret == -ENODEV || ret == -ETIMEDOUT)
break;
usleep_range(1000, 1100);
ret = asix_read_cmd(dev, AX_CMD_STATMNGSTS_REG,
0, 0, 1, &smsr, in_pm);
if (ret == -ENODEV)
break;
else if (ret < 0)
continue;
else if (smsr & AX_HOST_EN)
break;
}
return ret;
}
static void reset_asix_rx_fixup_info(struct asix_rx_fixup_info *rx) static void reset_asix_rx_fixup_info(struct asix_rx_fixup_info *rx)
{ {
/* Reset the variables that have a lifetime outside of /* Reset the variables that have a lifetime outside of
...@@ -467,19 +490,11 @@ int asix_mdio_read(struct net_device *netdev, int phy_id, int loc) ...@@ -467,19 +490,11 @@ int asix_mdio_read(struct net_device *netdev, int phy_id, int loc)
{ {
struct usbnet *dev = netdev_priv(netdev); struct usbnet *dev = netdev_priv(netdev);
__le16 res; __le16 res;
u8 smsr;
int i = 0;
int ret; int ret;
mutex_lock(&dev->phy_mutex); mutex_lock(&dev->phy_mutex);
do {
ret = asix_set_sw_mii(dev, 0); ret = asix_check_host_enable(dev, 0);
if (ret == -ENODEV || ret == -ETIMEDOUT)
break;
usleep_range(1000, 1100);
ret = asix_read_cmd(dev, AX_CMD_STATMNGSTS_REG,
0, 0, 1, &smsr, 0);
} while (!(smsr & AX_HOST_EN) && (i++ < 30) && (ret != -ENODEV));
if (ret == -ENODEV || ret == -ETIMEDOUT) { if (ret == -ENODEV || ret == -ETIMEDOUT) {
mutex_unlock(&dev->phy_mutex); mutex_unlock(&dev->phy_mutex);
return ret; return ret;
...@@ -505,23 +520,14 @@ static int __asix_mdio_write(struct net_device *netdev, int phy_id, int loc, ...@@ -505,23 +520,14 @@ static int __asix_mdio_write(struct net_device *netdev, int phy_id, int loc,
{ {
struct usbnet *dev = netdev_priv(netdev); struct usbnet *dev = netdev_priv(netdev);
__le16 res = cpu_to_le16(val); __le16 res = cpu_to_le16(val);
u8 smsr;
int i = 0;
int ret; int ret;
netdev_dbg(dev->net, "asix_mdio_write() phy_id=0x%02x, loc=0x%02x, val=0x%04x\n", netdev_dbg(dev->net, "asix_mdio_write() phy_id=0x%02x, loc=0x%02x, val=0x%04x\n",
phy_id, loc, val); phy_id, loc, val);
mutex_lock(&dev->phy_mutex); mutex_lock(&dev->phy_mutex);
do {
ret = asix_set_sw_mii(dev, 0);
if (ret == -ENODEV)
break;
usleep_range(1000, 1100);
ret = asix_read_cmd(dev, AX_CMD_STATMNGSTS_REG,
0, 0, 1, &smsr, 0);
} while (!(smsr & AX_HOST_EN) && (i++ < 30) && (ret != -ENODEV));
ret = asix_check_host_enable(dev, 0);
if (ret == -ENODEV) if (ret == -ENODEV)
goto out; goto out;
...@@ -561,19 +567,11 @@ int asix_mdio_read_nopm(struct net_device *netdev, int phy_id, int loc) ...@@ -561,19 +567,11 @@ int asix_mdio_read_nopm(struct net_device *netdev, int phy_id, int loc)
{ {
struct usbnet *dev = netdev_priv(netdev); struct usbnet *dev = netdev_priv(netdev);
__le16 res; __le16 res;
u8 smsr;
int i = 0;
int ret; int ret;
mutex_lock(&dev->phy_mutex); mutex_lock(&dev->phy_mutex);
do {
ret = asix_set_sw_mii(dev, 1); ret = asix_check_host_enable(dev, 1);
if (ret == -ENODEV || ret == -ETIMEDOUT)
break;
usleep_range(1000, 1100);
ret = asix_read_cmd(dev, AX_CMD_STATMNGSTS_REG,
0, 0, 1, &smsr, 1);
} while (!(smsr & AX_HOST_EN) && (i++ < 30) && (ret != -ENODEV));
if (ret == -ENODEV || ret == -ETIMEDOUT) { if (ret == -ENODEV || ret == -ETIMEDOUT) {
mutex_unlock(&dev->phy_mutex); mutex_unlock(&dev->phy_mutex);
return ret; return ret;
...@@ -595,22 +593,14 @@ asix_mdio_write_nopm(struct net_device *netdev, int phy_id, int loc, int val) ...@@ -595,22 +593,14 @@ asix_mdio_write_nopm(struct net_device *netdev, int phy_id, int loc, int val)
{ {
struct usbnet *dev = netdev_priv(netdev); struct usbnet *dev = netdev_priv(netdev);
__le16 res = cpu_to_le16(val); __le16 res = cpu_to_le16(val);
u8 smsr;
int i = 0;
int ret; int ret;
netdev_dbg(dev->net, "asix_mdio_write() phy_id=0x%02x, loc=0x%02x, val=0x%04x\n", netdev_dbg(dev->net, "asix_mdio_write() phy_id=0x%02x, loc=0x%02x, val=0x%04x\n",
phy_id, loc, val); phy_id, loc, val);
mutex_lock(&dev->phy_mutex); mutex_lock(&dev->phy_mutex);
do {
ret = asix_set_sw_mii(dev, 1); ret = asix_check_host_enable(dev, 1);
if (ret == -ENODEV)
break;
usleep_range(1000, 1100);
ret = asix_read_cmd(dev, AX_CMD_STATMNGSTS_REG,
0, 0, 1, &smsr, 1);
} while (!(smsr & AX_HOST_EN) && (i++ < 30) && (ret != -ENODEV));
if (ret == -ENODEV) { if (ret == -ENODEV) {
mutex_unlock(&dev->phy_mutex); mutex_unlock(&dev->phy_mutex);
return; return;
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment