Thread (3 messages) 3 messages, 2 authors, 2021-01-23

Re: Duplicate crash reports related with smsc75xx/smsc95xx and root cause analysis

From: 慕冬亮 <hidden>
Date: 2021-01-23 06:00:00
Also in: lkml, netdev

On Sat, Jan 23, 2021 at 1:40 PM 慕冬亮 [off-list ref] wrote:
quoted hunk ↗ jump to hunk
Dear kernel developers,

I found that on the syzbot dashboard, “KMSAN: uninit-value in
smsc75xx_read_eeprom (2)” [1],
"KMSAN: uninit-value in smsc95xx_read_eeprom (2)" [2], "KMSAN:
uninit-value in smsc75xx_bind" [3],
"KMSAN: uninit-value in smsc95xx_reset" [4], "KMSAN: uninit-value in
smsc95xx_wait_eeprom (2)" [5]
should share the same root cause.

## Root Cause Analysis && Different behaviors

The root cause of these crash reports resides in the
"__smsc75xx_read_reg/__smsc95xx_read_reg". Take __smsc95xx_read_reg as
an example,

-----------------------------------------------------------------------------------------------------------------
static int __must_check __smsc95xx_read_reg(struct usbnet *dev, u32 index,
                                            u32 *data, int in_pm)
{
        u32 buf;
        int ret;
        int (*fn)(struct usbnet *, u8, u8, u16, u16, void *, u16);

        BUG_ON(!dev);

        if (!in_pm)
                fn = usbnet_read_cmd;
        else
                fn = usbnet_read_cmd_nopm;

        ret = fn(dev, USB_VENDOR_REQUEST_READ_REGISTER, USB_DIR_IN
                 | USB_TYPE_VENDOR | USB_RECIP_DEVICE,
                 0, index, &buf, 4);
        if (unlikely(ret < 0)) {
                netdev_warn(dev->net, "Failed to read reg index 0x%08x: %d\n",
                            index, ret);
                return ret;
        }

        le32_to_cpus(&buf);
        *data = buf;

        return ret;
}


static int __must_check smsc95xx_eeprom_confirm_not_busy(struct usbnet *dev)
{
        unsigned long start_time = jiffies;
        u32 val;
        int ret;

        do {
                ret = smsc95xx_read_reg(dev, E2P_CMD, &val);
                if (ret < 0) {
                        netdev_warn(dev->net, "Error reading E2P_CMD\n");
                        return ret;
                }

                if (!(val & E2P_CMD_BUSY_))
                        return 0;
        ......
}
-----------------------------------------------------------------------------------------------------------------

In a special situation, local variable "buf" is not initialized with
"fn" function invocation. And the ret is bigger than zero, and buf is
assigned to "*data". In its parent function -
smsc95xx_eeprom_confirm_not_busy, KMSAN reports "uninit-value" when
accessing variable "val".
Note, due to the lack of testing environment, I don't know the
concrete reason for the uninitialization of "buf" local variable.

The reason for such different crash behaviors is that the event -
"buf" is not initialized is random when
"__smsc75xx_read_reg/__smsc95xx_read_reg" is invoked.

## Patch
diff --git a/drivers/net/usb/smsc75xx.c b/drivers/net/usb/smsc75xx.c
index 4353b370249f..a8e500d92285 100644
--- a/drivers/net/usb/smsc75xx.c
+++ b/drivers/net/usb/smsc75xx.c
@@ -76,7 +76,7 @@ static int smsc75xx_phy_gig_workaround(struct usbnet *dev);
 static int __must_check __smsc75xx_read_reg(struct usbnet *dev, u32 index,
                                            u32 *data, int in_pm)
 {
-       u32 buf;
+       u32 buf = 0;
        int ret;
        int (*fn)(struct usbnet *, u8, u8, u16, u16, void *, u16);
diff --git a/drivers/net/usb/smsc95xx.c b/drivers/net/usb/smsc95xx.c
index 4c8ee1cff4d4..dae3be723e0c 100644
--- a/drivers/net/usb/smsc95xx.c
+++ b/drivers/net/usb/smsc95xx.c
@@ -70,7 +70,7 @@ MODULE_PARM_DESC(turbo_mode, "Enable multiple frames
per Rx transaction");
 static int __must_check __smsc95xx_read_reg(struct usbnet *dev, u32 index,
                                            u32 *data, int in_pm)
 {
-       u32 buf;
+       u32 buf = 0;
        int ret;
        int (*fn)(struct usbnet *, u8, u8, u16, u16, void *, u16);

If you can have any issues with this statement or our information is
useful to you, please let us know. Thanks very much.

[1] “KMSAN: uninit-value in smsc75xx_read_eeprom (2)” - url
[2] “KMSAN: uninit-value in smsc95xx_read_eeprom (2)” - URL
[3] "KMSAN: uninit-value in smsc75xx_bind" -
[4] "KMSAN: uninit-value in smsc95xx_reset" -
[5] "KMSAN: uninit-value in smsc95xx_wait_eeprom (2)" -
Add links for all five bug reports:

[1] “KMSAN: uninit-value in smsc75xx_read_eeprom (2)” -
https://syzkaller.appspot.com/bug?id=2fb4e465ed593338d043227e7617cbdfaa03ba01
[2] “KMSAN: uninit-value in smsc95xx_read_eeprom (2)” -
https://syzkaller.appspot.com/bug?id=0629febb76ae17ff78874aa68991e542506b1351
[3] "KMSAN: uninit-value in smsc75xx_bind" -
https://syzkaller.appspot.com/bug?id=45ee70ca00699d61239bbf9ebc790e33f83add6a
[4] "KMSAN: uninit-value in smsc95xx_reset" -
https://syzkaller.appspot.com/bug?id=de07a0d125f8f1716eacb7e2ee4ceca251b21511
[5] "KMSAN: uninit-value in smsc95xx_wait_eeprom (2)" -
https://syzkaller.appspot.com/bug?id=b4eb76261b208b986ad7683e686c6be4200a7109
--
My best regards to you.

     No System Is Safe!
     Dongliang Mu
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help