* [PATCH 1/2] asix: fix uninit-value in asix_mdio_read()
@ 2021-12-21 19:39 Pavel Skripkin
2021-12-21 19:40 ` [PATCH 2/2] asix: fix wrong return value in asix_check_host_enable() Pavel Skripkin
2021-12-21 19:42 ` [PATCH 1/2] asix: fix uninit-value in asix_mdio_read() Andrew Lunn
0 siblings, 2 replies; 5+ messages in thread
From: Pavel Skripkin @ 2021-12-21 19:39 UTC (permalink / raw)
To: davem, kuba, linux, andrew, robert.foss, freddy
Cc: linux-usb, netdev, linux-kernel, Pavel Skripkin,
syzbot+f44badb06036334e867a
asix_read_cmd() may read less than sizeof(smsr) bytes and in this case
smsr will be uninitialized.
Fail log:
BUG: KMSAN: uninit-value in asix_check_host_enable drivers/net/usb/asix_common.c:82 [inline]
BUG: KMSAN: uninit-value in asix_check_host_enable drivers/net/usb/asix_common.c:82 [inline] drivers/net/usb/asix_common.c:497
BUG: KMSAN: uninit-value in asix_mdio_read+0x3c1/0xb00 drivers/net/usb/asix_common.c:497 drivers/net/usb/asix_common.c:497
asix_check_host_enable drivers/net/usb/asix_common.c:82 [inline]
asix_check_host_enable drivers/net/usb/asix_common.c:82 [inline] drivers/net/usb/asix_common.c:497
asix_mdio_read+0x3c1/0xb00 drivers/net/usb/asix_common.c:497 drivers/net/usb/asix_common.c:497
Fixes: d9fe64e51114 ("net: asix: Add in_pm parameter")
Reported-and-tested-by: syzbot+f44badb06036334e867a@syzkaller.appspotmail.com
Signed-off-by: Pavel Skripkin <paskripkin@gmail.com>
---
drivers/net/usb/asix_common.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/net/usb/asix_common.c b/drivers/net/usb/asix_common.c
index 42ba4af68090..06823d7141b6 100644
--- a/drivers/net/usb/asix_common.c
+++ b/drivers/net/usb/asix_common.c
@@ -77,7 +77,7 @@ static int asix_check_host_enable(struct usbnet *dev, int in_pm)
0, 0, 1, &smsr, in_pm);
if (ret == -ENODEV)
break;
- else if (ret < 0)
+ else if (ret < sizeof(smsr))
continue;
else if (smsr & AX_HOST_EN)
break;
--
2.34.1
^ permalink raw reply [flat|nested] 5+ messages in thread* [PATCH 2/2] asix: fix wrong return value in asix_check_host_enable()
2021-12-21 19:39 [PATCH 1/2] asix: fix uninit-value in asix_mdio_read() Pavel Skripkin
@ 2021-12-21 19:40 ` Pavel Skripkin
2021-12-21 19:44 ` Andrew Lunn
2021-12-21 19:42 ` [PATCH 1/2] asix: fix uninit-value in asix_mdio_read() Andrew Lunn
1 sibling, 1 reply; 5+ messages in thread
From: Pavel Skripkin @ 2021-12-21 19:40 UTC (permalink / raw)
To: davem, kuba, linux, andrew, robert.foss, freddy
Cc: linux-usb, netdev, linux-kernel, Pavel Skripkin
If asix_read_cmd() returns 0 on 30th interation, 0 will be returned from
asix_check_host_enable(), which is logically wrong. Fix it by returning
-ETIMEDOUT explicitly if we have exceeded 30 iterations
Fixes: a786e3195d6a ("net: asix: fix uninit value bugs")
Reported-by: Andrew Lunn <andrew@lunn.ch>
Signed-off-by: Pavel Skripkin <paskripkin@gmail.com>
---
drivers/net/usb/asix_common.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/net/usb/asix_common.c b/drivers/net/usb/asix_common.c
index 06823d7141b6..8c61d410a123 100644
--- a/drivers/net/usb/asix_common.c
+++ b/drivers/net/usb/asix_common.c
@@ -83,7 +83,7 @@ static int asix_check_host_enable(struct usbnet *dev, int in_pm)
break;
}
- return ret;
+ return i >= 30? -ETIMEDOUT: ret;
}
static void reset_asix_rx_fixup_info(struct asix_rx_fixup_info *rx)
--
2.34.1
^ permalink raw reply [flat|nested] 5+ messages in thread* Re: [PATCH 2/2] asix: fix wrong return value in asix_check_host_enable()
2021-12-21 19:40 ` [PATCH 2/2] asix: fix wrong return value in asix_check_host_enable() Pavel Skripkin
@ 2021-12-21 19:44 ` Andrew Lunn
2021-12-21 20:00 ` Pavel Skripkin
0 siblings, 1 reply; 5+ messages in thread
From: Andrew Lunn @ 2021-12-21 19:44 UTC (permalink / raw)
To: Pavel Skripkin
Cc: davem, kuba, linux, robert.foss, freddy, linux-usb, netdev, linux-kernel
On Tue, Dec 21, 2021 at 10:40:05PM +0300, Pavel Skripkin wrote:
> If asix_read_cmd() returns 0 on 30th interation, 0 will be returned from
> asix_check_host_enable(), which is logically wrong. Fix it by returning
> -ETIMEDOUT explicitly if we have exceeded 30 iterations
>
> Fixes: a786e3195d6a ("net: asix: fix uninit value bugs")
> Reported-by: Andrew Lunn <andrew@lunn.ch>
> Signed-off-by: Pavel Skripkin <paskripkin@gmail.com>
> ---
> drivers/net/usb/asix_common.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/net/usb/asix_common.c b/drivers/net/usb/asix_common.c
> index 06823d7141b6..8c61d410a123 100644
> --- a/drivers/net/usb/asix_common.c
> +++ b/drivers/net/usb/asix_common.c
> @@ -83,7 +83,7 @@ static int asix_check_host_enable(struct usbnet *dev, int in_pm)
> break;
> }
>
> - return ret;
> + return i >= 30? -ETIMEDOUT: ret;
I think the coding style guidelines would recommend a space before the ?
I would also replace the 30 with a #define, both here and in the for
loop.
Andrew
^ permalink raw reply [flat|nested] 5+ messages in thread* Re: [PATCH 2/2] asix: fix wrong return value in asix_check_host_enable()
2021-12-21 19:44 ` Andrew Lunn
@ 2021-12-21 20:00 ` Pavel Skripkin
0 siblings, 0 replies; 5+ messages in thread
From: Pavel Skripkin @ 2021-12-21 20:00 UTC (permalink / raw)
To: Andrew Lunn
Cc: davem, kuba, linux, robert.foss, freddy, linux-usb, netdev, linux-kernel
On 12/21/21 22:44, Andrew Lunn wrote:
> On Tue, Dec 21, 2021 at 10:40:05PM +0300, Pavel Skripkin wrote:
>> If asix_read_cmd() returns 0 on 30th interation, 0 will be returned from
>> asix_check_host_enable(), which is logically wrong. Fix it by returning
>> -ETIMEDOUT explicitly if we have exceeded 30 iterations
>>
>> Fixes: a786e3195d6a ("net: asix: fix uninit value bugs")
>> Reported-by: Andrew Lunn <andrew@lunn.ch>
>> Signed-off-by: Pavel Skripkin <paskripkin@gmail.com>
>> ---
>> drivers/net/usb/asix_common.c | 2 +-
>> 1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/drivers/net/usb/asix_common.c b/drivers/net/usb/asix_common.c
>> index 06823d7141b6..8c61d410a123 100644
>> --- a/drivers/net/usb/asix_common.c
>> +++ b/drivers/net/usb/asix_common.c
>> @@ -83,7 +83,7 @@ static int asix_check_host_enable(struct usbnet *dev, int in_pm)
>> break;
>> }
>>
>> - return ret;
>> + return i >= 30? -ETIMEDOUT: ret;
>
> I think the coding style guidelines would recommend a space before the ?
>
Ah, yes, I forgot to run chechpatch on 2nd one, sorry. Will fix in v2
> I would also replace the 30 with a #define, both here and in the for
> loop.
Will fix in v2 as well. Thanks for review!
With regards,
Pavel Skripkin
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH 1/2] asix: fix uninit-value in asix_mdio_read()
2021-12-21 19:39 [PATCH 1/2] asix: fix uninit-value in asix_mdio_read() Pavel Skripkin
2021-12-21 19:40 ` [PATCH 2/2] asix: fix wrong return value in asix_check_host_enable() Pavel Skripkin
@ 2021-12-21 19:42 ` Andrew Lunn
1 sibling, 0 replies; 5+ messages in thread
From: Andrew Lunn @ 2021-12-21 19:42 UTC (permalink / raw)
To: Pavel Skripkin
Cc: davem, kuba, linux, robert.foss, freddy, linux-usb, netdev,
linux-kernel, syzbot+f44badb06036334e867a
On Tue, Dec 21, 2021 at 10:39:32PM +0300, Pavel Skripkin wrote:
> asix_read_cmd() may read less than sizeof(smsr) bytes and in this case
> smsr will be uninitialized.
>
> Fail log:
> BUG: KMSAN: uninit-value in asix_check_host_enable drivers/net/usb/asix_common.c:82 [inline]
> BUG: KMSAN: uninit-value in asix_check_host_enable drivers/net/usb/asix_common.c:82 [inline] drivers/net/usb/asix_common.c:497
> BUG: KMSAN: uninit-value in asix_mdio_read+0x3c1/0xb00 drivers/net/usb/asix_common.c:497 drivers/net/usb/asix_common.c:497
> asix_check_host_enable drivers/net/usb/asix_common.c:82 [inline]
> asix_check_host_enable drivers/net/usb/asix_common.c:82 [inline] drivers/net/usb/asix_common.c:497
> asix_mdio_read+0x3c1/0xb00 drivers/net/usb/asix_common.c:497 drivers/net/usb/asix_common.c:497
>
> Fixes: d9fe64e51114 ("net: asix: Add in_pm parameter")
> Reported-and-tested-by: syzbot+f44badb06036334e867a@syzkaller.appspotmail.com
> Signed-off-by: Pavel Skripkin <paskripkin@gmail.com>
Reviewed-by: Andrew Lunn <andrew@lunn.ch>
Andrew
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2021-12-21 20:00 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-12-21 19:39 [PATCH 1/2] asix: fix uninit-value in asix_mdio_read() Pavel Skripkin
2021-12-21 19:40 ` [PATCH 2/2] asix: fix wrong return value in asix_check_host_enable() Pavel Skripkin
2021-12-21 19:44 ` Andrew Lunn
2021-12-21 20:00 ` Pavel Skripkin
2021-12-21 19:42 ` [PATCH 1/2] asix: fix uninit-value in asix_mdio_read() Andrew Lunn
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox
all inboxes | Powered by JetHome®