[<prev] [next>] [thread-next>] [day] [month] [year] [list]
Message-Id: <20250218002443.11731-1-qasdev00@gmail.com>
Date: Tue, 18 Feb 2025 00:24:43 +0000
From: Qasim Ijaz <qasdev00@...il.com>
To: andrew+netdev@...n.ch,
davem@...emloft.net,
edumazet@...gle.com,
kuba@...nel.org,
pabeni@...hat.com
Cc: netdev@...r.kernel.org,
linux-kernel@...r.kernel.org,
linux-usb@...r.kernel.org,
syzbot <syzbot+3361c2d6f78a3e0892f9@...kaller.appspotmail.com>,
stable@...r.kernel.org
Subject: [PATCH] net: fix uninitialised access in mii_nway_restart()
In mii_nway_restart() during the line:
bmcr = mii->mdio_read(mii->dev, mii->phy_id, MII_BMCR);
The code attempts to call mii->mdio_read which is ch9200_mdio_read().
ch9200_mdio_read() utilises a local buffer, which is initialised
with control_read():
unsigned char buff[2];
However buff is conditionally initialised inside control_read():
if (err == size) {
memcpy(data, buf, size);
}
If the condition of "err == size" is not met, then buff remains
uninitialised. Once this happens the uninitialised buff is accessed
and returned during ch9200_mdio_read():
return (buff[0] | buff[1] << 8);
The problem stems from the fact that ch9200_mdio_read() ignores the
return value of control_read(), leading to uinit-access of buff.
To fix this we should check the return value of control_read()
and return early on error.
Signed-off-by: Qasim Ijaz <qasdev00@...il.com>
Reported-by: syzbot <syzbot+3361c2d6f78a3e0892f9@...kaller.appspotmail.com>
Tested-by: syzbot <syzbot+3361c2d6f78a3e0892f9@...kaller.appspotmail.com>
Closes: https://syzkaller.appspot.com/bug?extid=3361c2d6f78a3e0892f9
Fixes: 4a476bd6d1d9 ("usbnet: New driver for QinHeng CH9200 devices")
Cc: stable@...r.kernel.org
---
drivers/net/mii.c | 2 ++
drivers/net/usb/ch9200.c | 7 +++++--
2 files changed, 7 insertions(+), 2 deletions(-)
diff --git a/drivers/net/mii.c b/drivers/net/mii.c
index 37bc3131d31a..e305bf0f1d04 100644
--- a/drivers/net/mii.c
+++ b/drivers/net/mii.c
@@ -464,6 +464,8 @@ int mii_nway_restart (struct mii_if_info *mii)
/* if autoneg is off, it's an error */
bmcr = mii->mdio_read(mii->dev, mii->phy_id, MII_BMCR);
+ if (bmcr < 0)
+ return bmcr;
if (bmcr & BMCR_ANENABLE) {
bmcr |= BMCR_ANRESTART;
diff --git a/drivers/net/usb/ch9200.c b/drivers/net/usb/ch9200.c
index f69d9b902da0..e32d3c282dc1 100644
--- a/drivers/net/usb/ch9200.c
+++ b/drivers/net/usb/ch9200.c
@@ -178,6 +178,7 @@ static int ch9200_mdio_read(struct net_device *netdev, int phy_id, int loc)
{
struct usbnet *dev = netdev_priv(netdev);
unsigned char buff[2];
+ int ret;
netdev_dbg(netdev, "%s phy_id:%02x loc:%02x\n",
__func__, phy_id, loc);
@@ -185,8 +186,10 @@ static int ch9200_mdio_read(struct net_device *netdev, int phy_id, int loc)
if (phy_id != 0)
return -ENODEV;
- control_read(dev, REQUEST_READ, 0, loc * 2, buff, 0x02,
- CONTROL_TIMEOUT_MS);
+ ret = control_read(dev, REQUEST_READ, 0, loc * 2, buff, 0x02,
+ CONTROL_TIMEOUT_MS);
+ if (ret != 2)
+ return ret < 0 ? ret : -EINVAL;
return (buff[0] | buff[1] << 8);
}
--
2.39.5
Powered by blists - more mailing lists