* Duplicate crash reports related with smsc75xx/smsc95xx and root cause analysis
@ 2021-01-23 5:40 慕冬亮
2021-01-23 5:58 ` 慕冬亮
2021-01-23 8:09 ` Greg KH
0 siblings, 2 replies; 3+ messages in thread
From: 慕冬亮 @ 2021-01-23 5:40 UTC (permalink / raw)
To: davem, kuba, linux-kernel, linux-usb, netdev, steve.glendinning,
UNGLinuxDriver
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)" -
--
My best regards to you.
No System Is Safe!
Dongliang Mu
^ permalink raw reply related [flat|nested] 3+ messages in thread
* Re: Duplicate crash reports related with smsc75xx/smsc95xx and root cause analysis
2021-01-23 5:40 Duplicate crash reports related with smsc75xx/smsc95xx and root cause analysis 慕冬亮
@ 2021-01-23 5:58 ` 慕冬亮
2021-01-23 8:09 ` Greg KH
1 sibling, 0 replies; 3+ messages in thread
From: 慕冬亮 @ 2021-01-23 5:58 UTC (permalink / raw)
To: davem, kuba, linux-kernel, linux-usb, netdev, steve.glendinning,
UNGLinuxDriver
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
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: Duplicate crash reports related with smsc75xx/smsc95xx and root cause analysis
2021-01-23 5:40 Duplicate crash reports related with smsc75xx/smsc95xx and root cause analysis 慕冬亮
2021-01-23 5:58 ` 慕冬亮
@ 2021-01-23 8:09 ` Greg KH
1 sibling, 0 replies; 3+ messages in thread
From: Greg KH @ 2021-01-23 8:09 UTC (permalink / raw)
To: 慕冬亮
Cc: davem, kuba, linux-kernel, linux-usb, netdev, steve.glendinning,
UNGLinuxDriver
On Sat, Jan 23, 2021 at 01:40:30PM +0800, 慕冬亮 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)" -
As I asked before, please just turn this into a real patch and submit it
to the syzbot to see if it fixes the issue. If so, then submit it
normally, no need to do any huge writeup.
thnaks,
greg k-h
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2021-01-23 8:10 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-01-23 5:40 Duplicate crash reports related with smsc75xx/smsc95xx and root cause analysis 慕冬亮
2021-01-23 5:58 ` 慕冬亮
2021-01-23 8:09 ` Greg KH
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).