From 8a160e2e9aeb8318159b48701ad8a6e22274372d Mon Sep 17 00:00:00 2001
From: Petko Manolov <petkan@nucleusys.com>
Date: Tue, 3 Aug 2021 20:25:23 +0300
Subject: net: usb: pegasus: Check the return value of get_geristers() and
friends;
Certain call sites of get_geristers() did not do proper error handling. This
could be a problem as get_geristers() typically return the data via pointer to a
buffer. If an error occurred the code is carelessly manipulating the wrong data.
Signed-off-by: Petko Manolov <petkan@nucleusys.com>
Reviewed-by: Pavel Skripkin <paskripkin@gmail.com>
Signed-off-by: David S. Miller <davem@davemloft.net>
---
drivers/net/usb/pegasus.c | 108 ++++++++++++++++++++++++++++++++--------------
1 file changed, 75 insertions(+), 33 deletions(-)
(limited to 'drivers/net/usb/pegasus.c')
diff --git a/drivers/net/usb/pegasus.c b/drivers/net/usb/pegasus.c
index 9a907182569c..22353bab76c8 100644
--- a/drivers/net/usb/pegasus.c
+++ b/drivers/net/usb/pegasus.c
@@ -132,9 +132,15 @@ static int get_registers(pegasus_t *pegasus, __u16 indx, __u16 size, void *data)
static int set_registers(pegasus_t *pegasus, __u16 indx, __u16 size,
const void *data)
{
- return usb_control_msg_send(pegasus->usb, 0, PEGASUS_REQ_SET_REGS,
+ int ret;
+
+ ret = usb_control_msg_send(pegasus->usb, 0, PEGASUS_REQ_SET_REGS,
PEGASUS_REQT_WRITE, 0, indx, data, size,
1000, GFP_NOIO);
+ if (ret < 0)
+ netif_dbg(pegasus, drv, pegasus->net, "%s failed with %d\n", __func__, ret);
+
+ return ret;
}
/*
@@ -145,10 +151,15 @@ static int set_registers(pegasus_t *pegasus, __u16 indx, __u16 size,
static int set_register(pegasus_t *pegasus, __u16 indx, __u8 data)
{
void *buf = &data;
+ int ret;
- return usb_control_msg_send(pegasus->usb, 0, PEGASUS_REQ_SET_REG,
+ ret = usb_control_msg_send(pegasus->usb, 0, PEGASUS_REQ_SET_REG,
PEGASUS_REQT_WRITE, data, indx, buf, 1,
1000, GFP_NOIO);
+ if (ret < 0)
+ netif_dbg(pegasus, drv, pegasus->net, "%s failed with %d\n", __func__, ret);
+
+ return ret;
}
static int update_eth_regs_async(pegasus_t *pegasus)
@@ -188,10 +199,9 @@ static int update_eth_regs_async(pegasus_t *pegasus)
static int __mii_op(pegasus_t *p, __u8 phy, __u8 indx, __u16 *regd, __u8 cmd)
{
- int i;
- __u8 data[4] = { phy, 0, 0, indx };
+ int i, ret;
__le16 regdi;
- int ret = -ETIMEDOUT;
+ __u8 data[4] = { phy, 0, 0, indx };
if (cmd & PHY_WRITE) {
__le16 *t = (__le16 *) & data[1];
@@ -207,12 +217,15 @@ static int __mii_op(pegasus_t *p, __u8 phy, __u8 indx, __u16 *regd, __u8 cmd)
if (data[0] & PHY_DONE)
break;
}
- if (i >= REG_TIMEOUT)
+ if (i >= REG_TIMEOUT) {
+ ret = -ETIMEDOUT;
goto fail;
+ }
if (cmd & PHY_READ) {
ret = get_registers(p, PhyData, 2, ®di);
+ if (ret < 0)
+ goto fail;
*regd = le16_to_cpu(regdi);
- return ret;
}
return 0;
fail:
@@ -235,9 +248,13 @@ static int write_mii_word(pegasus_t *pegasus, __u8 phy, __u8 indx, __u16 *regd)
static int mdio_read(struct net_device *dev, int phy_id, int loc)
{
pegasus_t *pegasus = netdev_priv(dev);
+ int ret;
u16 res;
- read_mii_word(pegasus, phy_id, loc, &res);
+ ret = read_mii_word(pegasus, phy_id, loc, &res);
+ if (ret < 0)
+ return ret;
+
return (int)res;
}
@@ -251,10 +268,9 @@ static void mdio_write(struct net_device *dev, int phy_id, int loc, int val)
static int read_eprom_word(pegasus_t *pegasus, __u8 index, __u16 *retdata)
{
- int i;
- __u8 tmp = 0;
+ int ret, i;
__le16 retdatai;
- int ret;
+ __u8 tmp = 0;
set_register(pegasus, EpromCtrl, 0);
set_register(pegasus, EpromOffset, index);
@@ -262,21 +278,25 @@ static int read_eprom_word(pegasus_t *pegasus, __u8 index, __u16 *retdata)
for (i = 0; i < REG_TIMEOUT; i++) {
ret = get_registers(pegasus, EpromCtrl, 1, &tmp);
+ if (ret < 0)
+ goto fail;
if (tmp & EPROM_DONE)
break;
- if (ret == -ESHUTDOWN)
- goto fail;
}
- if (i >= REG_TIMEOUT)
+ if (i >= REG_TIMEOUT) {
+ ret = -ETIMEDOUT;
goto fail;
+ }
ret = get_registers(pegasus, EpromData, 2, &retdatai);
+ if (ret < 0)
+ goto fail;
*retdata = le16_to_cpu(retdatai);
return ret;
fail:
- netif_warn(pegasus, drv, pegasus->net, "%s failed\n", __func__);
- return -ETIMEDOUT;
+ netif_dbg(pegasus, drv, pegasus->net, "%s failed\n", __func__);
+ return ret;
}
#ifdef PEGASUS_WRITE_EEPROM
@@ -324,10 +344,10 @@ static int write_eprom_word(pegasus_t *pegasus, __u8 index, __u16 data)
return ret;
fail:
- netif_warn(pegasus, drv, pegasus->net, "%s failed\n", __func__);
+ netif_dbg(pegasus, drv, pegasus->net, "%s failed\n", __func__);
return -ETIMEDOUT;
}
-#endif /* PEGASUS_WRITE_EEPROM */
+#endif /* PEGASUS_WRITE_EEPROM */
static inline int get_node_id(pegasus_t *pegasus, u8 *id)
{
@@ -367,19 +387,21 @@ static void set_ethernet_addr(pegasus_t *pegasus)
return;
err:
eth_hw_addr_random(pegasus->net);
- dev_info(&pegasus->intf->dev, "software assigned MAC address.\n");
+ netif_dbg(pegasus, drv, pegasus->net, "software assigned MAC address.\n");
return;
}
static inline int reset_mac(pegasus_t *pegasus)
{
+ int ret, i;
__u8 data = 0x8;
- int i;
set_register(pegasus, EthCtrl1, data);
for (i = 0; i < REG_TIMEOUT; i++) {
- get_registers(pegasus, EthCtrl1, 1, &data);
+ ret = get_registers(pegasus, EthCtrl1, 1, &data);
+ if (ret < 0)
+ goto fail;
if (~data & 0x08) {
if (loopback)
break;
@@ -402,22 +424,29 @@ static inline int reset_mac(pegasus_t *pegasus)
}
if (usb_dev_id[pegasus->dev_index].vendor == VENDOR_ELCON) {
__u16 auxmode;
- read_mii_word(pegasus, 3, 0x1b, &auxmode);
+ ret = read_mii_word(pegasus, 3, 0x1b, &auxmode);
+ if (ret < 0)
+ goto fail;
auxmode |= 4;
write_mii_word(pegasus, 3, 0x1b, &auxmode);
}
return 0;
+fail:
+ netif_dbg(pegasus, drv, pegasus->net, "%s failed\n", __func__);
+ return ret;
}
static int enable_net_traffic(struct net_device *dev, struct usb_device *usb)
{
- __u16 linkpart;
- __u8 data[4];
pegasus_t *pegasus = netdev_priv(dev);
int ret;
+ __u16 linkpart;
+ __u8 data[4];
- read_mii_word(pegasus, pegasus->phy, MII_LPA, &linkpart);
+ ret = read_mii_word(pegasus, pegasus->phy, MII_LPA, &linkpart);
+ if (ret < 0)
+ goto fail;
data[0] = 0xc8; /* TX & RX enable, append status, no CRC */
data[1] = 0;
if (linkpart & (ADVERTISE_100FULL | ADVERTISE_10FULL))
@@ -435,11 +464,16 @@ static int enable_net_traffic(struct net_device *dev, struct usb_device *usb)
usb_dev_id[pegasus->dev_index].vendor == VENDOR_LINKSYS2 ||
usb_dev_id[pegasus->dev_index].vendor == VENDOR_DLINK) {
u16 auxmode;
- read_mii_word(pegasus, 0, 0x1b, &auxmode);
+ ret = read_mii_word(pegasus, 0, 0x1b, &auxmode);
+ if (ret < 0)
+ goto fail;
auxmode |= 4;
write_mii_word(pegasus, 0, 0x1b, &auxmode);
}
+ return 0;
+fail:
+ netif_dbg(pegasus, drv, pegasus->net, "%s failed\n", __func__);
return ret;
}
@@ -447,9 +481,9 @@ static void read_bulk_callback(struct urb *urb)
{
pegasus_t *pegasus = urb->context;
struct net_device *net;
+ u8 *buf = urb->transfer_buffer;
int rx_status, count = urb->actual_length;
int status = urb->status;
- u8 *buf = urb->transfer_buffer;
__u16 pkt_len;
if (!pegasus)
@@ -998,8 +1032,7 @@ static int pegasus_ioctl(struct net_device *net, struct ifreq *rq, int cmd)
data[0] = pegasus->phy;
fallthrough;
case SIOCDEVPRIVATE + 1:
- read_mii_word(pegasus, data[0], data[1] & 0x1f, &data[3]);
- res = 0;
+ res = read_mii_word(pegasus, data[0], data[1] & 0x1f, &data[3]);
break;
case SIOCDEVPRIVATE + 2:
if (!capable(CAP_NET_ADMIN))
@@ -1033,22 +1066,25 @@ static void pegasus_set_multicast(struct net_device *net)
static __u8 mii_phy_probe(pegasus_t *pegasus)
{
- int i;
+ int i, ret;
__u16 tmp;
for (i = 0; i < 32; i++) {
- read_mii_word(pegasus, i, MII_BMSR, &tmp);
+ ret = read_mii_word(pegasus, i, MII_BMSR, &tmp);
+ if (ret < 0)
+ goto fail;
if (tmp == 0 || tmp == 0xffff || (tmp & BMSR_MEDIA) == 0)
continue;
else
return i;
}
-
+fail:
return 0xff;
}
static inline void setup_pegasus_II(pegasus_t *pegasus)
{
+ int ret;
__u8 data = 0xa5;
set_register(pegasus, Reg1d, 0);
@@ -1060,7 +1096,9 @@ static inline void setup_pegasus_II(pegasus_t *pegasus)
set_register(pegasus, Reg7b, 2);
set_register(pegasus, 0x83, data);
- get_registers(pegasus, 0x83, 1, &data);
+ ret = get_registers(pegasus, 0x83, 1, &data);
+ if (ret < 0)
+ goto fail;
if (data == 0xa5)
pegasus->chip = 0x8513;
@@ -1075,6 +1113,10 @@ static inline void setup_pegasus_II(pegasus_t *pegasus)
set_register(pegasus, Reg81, 6);
else
set_register(pegasus, Reg81, 2);
+
+ return;
+fail:
+ netif_dbg(pegasus, drv, pegasus->net, "%s failed\n", __func__);
}
static void check_carrier(struct work_struct *work)
--
cgit v1.2.3
From bc65bacf239d0bc1d00d92cd535a4031921dd78a Mon Sep 17 00:00:00 2001
From: Petko Manolov <petkan@nucleusys.com>
Date: Tue, 3 Aug 2021 20:25:24 +0300
Subject: net: usb: pegasus: Remove the changelog and DRIVER_VERSION.
These are now deemed redundant.
Signed-off-by: Petko Manolov <petkan@nucleusys.com>
Acked-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Signed-off-by: David S. Miller <davem@davemloft.net>
---
drivers/net/usb/pegasus.c | 30 ++----------------------------
1 file changed, 2 insertions(+), 28 deletions(-)
(limited to 'drivers/net/usb/pegasus.c')
diff --git a/drivers/net/usb/pegasus.c b/drivers/net/usb/pegasus.c
index 22353bab76c8..f18b03be1b87 100644
--- a/drivers/net/usb/pegasus.c
+++ b/drivers/net/usb/pegasus.c
@@ -1,31 +1,7 @@
// SPDX-License-Identifier: GPL-2.0-only
/*
- * Copyright (c) 1999-2013 Petko Manolov (petkan@nucleusys.com)
+ * Copyright (c) 1999-2021 Petko Manolov (petkan@nucleusys.com)
*
- * ChangeLog:
- * .... Most of the time spent on reading sources & docs.
- * v0.2.x First official release for the Linux kernel.
- * v0.3.0 Beutified and structured, some bugs fixed.
- * v0.3.x URBifying bulk requests and bugfixing. First relatively
- * stable release. Still can touch device's registers only
- * from top-halves.
- * v0.4.0 Control messages remained unurbified are now URBs.
- * Now we can touch the HW at any time.
- * v0.4.9 Control urbs again use process context to wait. Argh...
- * Some long standing bugs (enable_net_traffic) fixed.
- * Also nasty trick about resubmiting control urb from
- * interrupt context used. Please let me know how it
- * behaves. Pegasus II support added since this version.
- * TODO: suppressing HCD warnings spewage on disconnect.
- * v0.4.13 Ethernet address is now set at probe(), not at open()
- * time as this seems to break dhcpd.
- * v0.5.0 branch to 2.5.x kernels
- * v0.5.1 ethtool support added
- * v0.5.5 rx socket buffers are in a pool and the their allocation
- * is out of the interrupt routine.
- * ...
- * v0.9.3 simplified [get|set]_register(s), async update registers
- * logic revisited, receive skb_pool removed.
*/
#include <linux/sched.h>
@@ -45,7 +21,6 @@
/*
* Version Information
*/
-#define DRIVER_VERSION "v0.9.3 (2013/04/25)"
#define DRIVER_AUTHOR "Petko Manolov <petkan@nucleusys.com>"
#define DRIVER_DESC "Pegasus/Pegasus II USB Ethernet driver"
@@ -914,7 +889,6 @@ static void pegasus_get_drvinfo(struct net_device *dev,
pegasus_t *pegasus = netdev_priv(dev);
strlcpy(info->driver, driver_name, sizeof(info->driver));
- strlcpy(info->version, DRIVER_VERSION, sizeof(info->version));
usb_make_path(pegasus->usb, info->bus_info, sizeof(info->bus_info));
}
@@ -1338,7 +1312,7 @@ static void __init parse_id(char *id)
static int __init pegasus_init(void)
{
- pr_info("%s: %s, " DRIVER_DESC "\n", driver_name, DRIVER_VERSION);
+ pr_info("%s: " DRIVER_DESC "\n", driver_name);
if (devid)
parse_id(devid);
return usb_register(&pegasus_driver);
--
cgit v1.2.3
From af35fc37354cda3c9c8cc4961b1d24bdc9d27903 Mon Sep 17 00:00:00 2001
From: Pavel Skripkin <paskripkin@gmail.com>
Date: Wed, 4 Aug 2021 17:30:05 +0300
Subject: net: pegasus: fix uninit-value in get_interrupt_interval
Syzbot reported uninit value pegasus_probe(). The problem was in missing
error handling.
get_interrupt_interval() internally calls read_eprom_word() which can
fail in some cases. For example: failed to receive usb control message.
These cases should be handled to prevent uninit value bug, since
read_eprom_word() will not initialize passed stack variable in case of
internal failure.
Fail log:
BUG: KMSAN: uninit-value in get_interrupt_interval drivers/net/usb/pegasus.c:746 [inline]
BUG: KMSAN: uninit-value in pegasus_probe+0x10e7/0x4080 drivers/net/usb/pegasus.c:1152
CPU: 1 PID: 825 Comm: kworker/1:1 Not tainted 5.12.0-rc6-syzkaller #0
...
Workqueue: usb_hub_wq hub_event
Call Trace:
__dump_stack lib/dump_stack.c:79 [inline]
dump_stack+0x24c/0x2e0 lib/dump_stack.c:120
kmsan_report+0xfb/0x1e0 mm/kmsan/kmsan_report.c:118
__msan_warning+0x5c/0xa0 mm/kmsan/kmsan_instr.c:197
get_interrupt_interval drivers/net/usb/pegasus.c:746 [inline]
pegasus_probe+0x10e7/0x4080 drivers/net/usb/pegasus.c:1152
....
Local variable ----data.i@pegasus_probe created at:
get_interrupt_interval drivers/net/usb/pegasus.c:1151 [inline]
pegasus_probe+0xe57/0x4080 drivers/net/usb/pegasus.c:1152
get_interrupt_interval drivers/net/usb/pegasus.c:1151 [inline]
pegasus_probe+0xe57/0x4080 drivers/net/usb/pegasus.c:1152
Reported-and-tested-by: syzbot+02c9f70f3afae308464a@syzkaller.appspotmail.com
Fixes: 1da177e4c3f4 ("Linux-2.6.12-rc2")
Signed-off-by: Pavel Skripkin <paskripkin@gmail.com>
Link: https://lore.kernel.org/r/20210804143005.439-1-paskripkin@gmail.com
Signed-off-by: Jakub Kicinski <kuba@kernel.org>
---
drivers/net/usb/pegasus.c | 14 +++++++++++---
1 file changed, 11 insertions(+), 3 deletions(-)
(limited to 'drivers/net/usb/pegasus.c')
diff --git a/drivers/net/usb/pegasus.c b/drivers/net/usb/pegasus.c
index f18b03be1b87..652e9fcf0b77 100644
--- a/drivers/net/usb/pegasus.c
+++ b/drivers/net/usb/pegasus.c
@@ -744,12 +744,16 @@ static inline void disable_net_traffic(pegasus_t *pegasus)
set_registers(pegasus, EthCtrl0, sizeof(tmp), &tmp);
}
-static inline void get_interrupt_interval(pegasus_t *pegasus)
+static inline int get_interrupt_interval(pegasus_t *pegasus)
{
u16 data;
u8 interval;
+ int ret;
+
+ ret = read_eprom_word(pegasus, 4, &data);
+ if (ret < 0)
+ return ret;
- read_eprom_word(pegasus, 4, &data);
interval = data >> 8;
if (pegasus->usb->speed != USB_SPEED_HIGH) {
if (interval < 0x80) {
@@ -764,6 +768,8 @@ static inline void get_interrupt_interval(pegasus_t *pegasus)
}
}
pegasus->intr_interval = interval;
+
+ return 0;
}
static void set_carrier(struct net_device *net)
@@ -1165,7 +1171,9 @@ static int pegasus_probe(struct usb_interface *intf,
| NETIF_MSG_PROBE | NETIF_MSG_LINK);
pegasus->features = usb_dev_id[dev_index].private;
- get_interrupt_interval(pegasus);
+ res = get_interrupt_interval(pegasus);
+ if (res)
+ goto out2;
if (reset_mac(pegasus)) {
dev_err(&intf->dev, "can't reset MAC\n");
res = -EIO;
--
cgit v1.2.3