[Linux-kernel-mentees][PATCH v2] net: usb: rtl8150: prevent set_ethernet_addr from setting uninit address

From: Anant Thazhemadam
Date: Thu Oct 01 2020 - 03:33:25 EST


When get_registers() fails (which happens when usb_control_msg() fails)
in set_ethernet_addr(), the uninitialized value of node_id gets copied
as the address.

Checking for the return values appropriately, and handling the case
wherein set_ethernet_addr() fails like this, helps in avoiding the
mac address being incorrectly set in this manner.

Reported-by: syzbot+abbc768b560c84d92fd3@xxxxxxxxxxxxxxxxxxxxxxxxx
Tested-by: syzbot+abbc768b560c84d92fd3@xxxxxxxxxxxxxxxxxxxxxxxxx
Signed-off-by: Anant Thazhemadam <anant.thazhemadam@xxxxxxxxx>
Acked-by: Petko Manolov <petkan@xxxxxxxxxxxxx>
---
Changes in v2:
* Modified condition checking get_registers()'s return value to
ret == sizeof(node_id)
for stricter checking in compliance with the new usb_control_msg_recv()
API
* Added Acked-by: Petko Manolov

Since Petko didn't explicitly mention an email-id in his Ack, I put the
email-id present in the MAINTAINERS file. I hope that's not an issue.


drivers/net/usb/rtl8150.c | 24 ++++++++++++++++--------
1 file changed, 16 insertions(+), 8 deletions(-)

diff --git a/drivers/net/usb/rtl8150.c b/drivers/net/usb/rtl8150.c
index 733f120c852b..e542a9ab2ff8 100644
--- a/drivers/net/usb/rtl8150.c
+++ b/drivers/net/usb/rtl8150.c
@@ -150,7 +150,7 @@ static const char driver_name [] = "rtl8150";
** device related part of the code
**
*/
-static int get_registers(rtl8150_t * dev, u16 indx, u16 size, void *data)
+static int get_registers(rtl8150_t *dev, u16 indx, u16 size, void *data)
{
void *buf;
int ret;
@@ -274,12 +274,17 @@ static int write_mii_word(rtl8150_t * dev, u8 phy, __u8 indx, u16 reg)
return 1;
}

-static inline void set_ethernet_addr(rtl8150_t * dev)
+static bool set_ethernet_addr(rtl8150_t *dev)
{
u8 node_id[6];
+ int ret;

- get_registers(dev, IDR, sizeof(node_id), node_id);
- memcpy(dev->netdev->dev_addr, node_id, sizeof(node_id));
+ ret = get_registers(dev, IDR, sizeof(node_id), node_id);
+ if (ret == sizeof(node_id)) {
+ memcpy(dev->netdev->dev_addr, node_id, sizeof(node_id));
+ return true;
+ }
+ return false;
}

static int rtl8150_set_mac_address(struct net_device *netdev, void *p)
@@ -909,21 +914,24 @@ static int rtl8150_probe(struct usb_interface *intf,
goto out1;
}
fill_skb_pool(dev);
- set_ethernet_addr(dev);
-
+ if (!set_ethernet_addr(dev)) {
+ dev_err(&intf->dev, "couldn't set the ethernet address for the device\n");
+ goto out2;
+ }
usb_set_intfdata(intf, dev);
SET_NETDEV_DEV(netdev, &intf->dev);
if (register_netdev(netdev) != 0) {
dev_err(&intf->dev, "couldn't register the device\n");
- goto out2;
+ goto out3;
}

dev_info(&intf->dev, "%s: rtl8150 is detected\n", netdev->name);

return 0;

-out2:
+out3:
usb_set_intfdata(intf, NULL);
+out2:
free_skb_pool(dev);
out1:
free_all_urbs(dev);
--
2.25.1