lkml.org 
[lkml]   [2021]   [Jan]   [23]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    From
    Date
    SubjectRe: Duplicate crash reports related with smsc75xx/smsc95xx and root cause analysis
    On Sat, Jan 23, 2021 at 1:40 PM 慕冬亮 <mudongliangabcd@gmail.com> wrote:
    >
    > 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

    \
     
     \ /
      Last update: 2021-01-23 07:01    [W:5.076 / U:0.012 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site