Skip to content

Commit a786e31

Browse files
pskrgagdavem330
authored andcommitted
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 <[email protected]> Fixes: d9fe64e ("net: asix: Add in_pm parameter") Reported-by: [email protected] Signed-off-by: Pavel Skripkin <[email protected]> Signed-off-by: David S. Miller <[email protected]>
1 parent 0163404 commit a786e31

File tree

1 file changed

+30
-40
lines changed

1 file changed

+30
-40
lines changed

drivers/net/usb/asix_common.c

Lines changed: 30 additions & 40 deletions
Original file line numberDiff line numberDiff line change
@@ -63,6 +63,29 @@ void asix_write_cmd_async(struct usbnet *dev, u8 cmd, u16 value, u16 index,
6363
value, index, data, size);
6464
}
6565

66+
static int asix_check_host_enable(struct usbnet *dev, int in_pm)
67+
{
68+
int i, ret;
69+
u8 smsr;
70+
71+
for (i = 0; i < 30; ++i) {
72+
ret = asix_set_sw_mii(dev, in_pm);
73+
if (ret == -ENODEV || ret == -ETIMEDOUT)
74+
break;
75+
usleep_range(1000, 1100);
76+
ret = asix_read_cmd(dev, AX_CMD_STATMNGSTS_REG,
77+
0, 0, 1, &smsr, in_pm);
78+
if (ret == -ENODEV)
79+
break;
80+
else if (ret < 0)
81+
continue;
82+
else if (smsr & AX_HOST_EN)
83+
break;
84+
}
85+
86+
return ret;
87+
}
88+
6689
static void reset_asix_rx_fixup_info(struct asix_rx_fixup_info *rx)
6790
{
6891
/* 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)
467490
{
468491
struct usbnet *dev = netdev_priv(netdev);
469492
__le16 res;
470-
u8 smsr;
471-
int i = 0;
472493
int ret;
473494

474495
mutex_lock(&dev->phy_mutex);
475-
do {
476-
ret = asix_set_sw_mii(dev, 0);
477-
if (ret == -ENODEV || ret == -ETIMEDOUT)
478-
break;
479-
usleep_range(1000, 1100);
480-
ret = asix_read_cmd(dev, AX_CMD_STATMNGSTS_REG,
481-
0, 0, 1, &smsr, 0);
482-
} while (!(smsr & AX_HOST_EN) && (i++ < 30) && (ret != -ENODEV));
496+
497+
ret = asix_check_host_enable(dev, 0);
483498
if (ret == -ENODEV || ret == -ETIMEDOUT) {
484499
mutex_unlock(&dev->phy_mutex);
485500
return ret;
@@ -505,23 +520,14 @@ static int __asix_mdio_write(struct net_device *netdev, int phy_id, int loc,
505520
{
506521
struct usbnet *dev = netdev_priv(netdev);
507522
__le16 res = cpu_to_le16(val);
508-
u8 smsr;
509-
int i = 0;
510523
int ret;
511524

512525
netdev_dbg(dev->net, "asix_mdio_write() phy_id=0x%02x, loc=0x%02x, val=0x%04x\n",
513526
phy_id, loc, val);
514527

515528
mutex_lock(&dev->phy_mutex);
516-
do {
517-
ret = asix_set_sw_mii(dev, 0);
518-
if (ret == -ENODEV)
519-
break;
520-
usleep_range(1000, 1100);
521-
ret = asix_read_cmd(dev, AX_CMD_STATMNGSTS_REG,
522-
0, 0, 1, &smsr, 0);
523-
} while (!(smsr & AX_HOST_EN) && (i++ < 30) && (ret != -ENODEV));
524529

530+
ret = asix_check_host_enable(dev, 0);
525531
if (ret == -ENODEV)
526532
goto out;
527533

@@ -561,19 +567,11 @@ int asix_mdio_read_nopm(struct net_device *netdev, int phy_id, int loc)
561567
{
562568
struct usbnet *dev = netdev_priv(netdev);
563569
__le16 res;
564-
u8 smsr;
565-
int i = 0;
566570
int ret;
567571

568572
mutex_lock(&dev->phy_mutex);
569-
do {
570-
ret = asix_set_sw_mii(dev, 1);
571-
if (ret == -ENODEV || ret == -ETIMEDOUT)
572-
break;
573-
usleep_range(1000, 1100);
574-
ret = asix_read_cmd(dev, AX_CMD_STATMNGSTS_REG,
575-
0, 0, 1, &smsr, 1);
576-
} while (!(smsr & AX_HOST_EN) && (i++ < 30) && (ret != -ENODEV));
573+
574+
ret = asix_check_host_enable(dev, 1);
577575
if (ret == -ENODEV || ret == -ETIMEDOUT) {
578576
mutex_unlock(&dev->phy_mutex);
579577
return ret;
@@ -595,22 +593,14 @@ asix_mdio_write_nopm(struct net_device *netdev, int phy_id, int loc, int val)
595593
{
596594
struct usbnet *dev = netdev_priv(netdev);
597595
__le16 res = cpu_to_le16(val);
598-
u8 smsr;
599-
int i = 0;
600596
int ret;
601597

602598
netdev_dbg(dev->net, "asix_mdio_write() phy_id=0x%02x, loc=0x%02x, val=0x%04x\n",
603599
phy_id, loc, val);
604600

605601
mutex_lock(&dev->phy_mutex);
606-
do {
607-
ret = asix_set_sw_mii(dev, 1);
608-
if (ret == -ENODEV)
609-
break;
610-
usleep_range(1000, 1100);
611-
ret = asix_read_cmd(dev, AX_CMD_STATMNGSTS_REG,
612-
0, 0, 1, &smsr, 1);
613-
} while (!(smsr & AX_HOST_EN) && (i++ < 30) && (ret != -ENODEV));
602+
603+
ret = asix_check_host_enable(dev, 1);
614604
if (ret == -ENODEV) {
615605
mutex_unlock(&dev->phy_mutex);
616606
return;

0 commit comments

Comments
 (0)