syzbot has found an uninit-value bug triggered by the dm9601 driver [1].
This error happens because the variable res is not updated if the call
to dm_read_shared_word returns an error. In this particular case -EPROTO
was returned and res stayed uninitialized.
This can be avoided by checking the return value of dm_read_shared_word
and propagating the error if the read operation failed.
[1] https://syzkaller.appspot.com/bug?extid=1f53a30781af65d2c955
Signed-off-by: Javier Carrasco <[email protected]>
Reported-and-tested-by: [email protected]
---
Changes in v2:
- Remove unnecessary 'err == 0' case
- Link to v1: https://lore.kernel.org/r/20231009-topic-dm9601_uninit_mdio_read-v1-1-d4d775e24e3b@gmail.com
---
drivers/net/usb/dm9601.c | 7 ++++++-
1 file changed, 6 insertions(+), 1 deletion(-)
diff --git a/drivers/net/usb/dm9601.c b/drivers/net/usb/dm9601.c
index 48d7d278631e..99ec1d4a972d 100644
--- a/drivers/net/usb/dm9601.c
+++ b/drivers/net/usb/dm9601.c
@@ -222,13 +222,18 @@ static int dm9601_mdio_read(struct net_device *netdev, int phy_id, int loc)
struct usbnet *dev = netdev_priv(netdev);
__le16 res;
+ int err;
if (phy_id) {
netdev_dbg(dev->net, "Only internal phy supported\n");
return 0;
}
- dm_read_shared_word(dev, 1, loc, &res);
+ err = dm_read_shared_word(dev, 1, loc, &res);
+ if (err < 0) {
+ netdev_err(dev->net, "MDIO read error: %d\n", err);
+ return err;
+ }
netdev_dbg(dev->net,
"dm9601_mdio_read() phy_id=0x%02x, loc=0x%02x, returns=0x%04x\n",
---
base-commit: 94f6f0550c625fab1f373bb86a6669b45e9748b3
change-id: 20231009-topic-dm9601_uninit_mdio_read-a15918ffbd6f
Best regards,
--
Javier Carrasco <[email protected]>
On Tue, Oct 10, 2023 at 12:26:14AM +0200, Javier Carrasco wrote:
> syzbot has found an uninit-value bug triggered by the dm9601 driver [1].
>
> This error happens because the variable res is not updated if the call
> to dm_read_shared_word returns an error. In this particular case -EPROTO
> was returned and res stayed uninitialized.
>
> This can be avoided by checking the return value of dm_read_shared_word
> and propagating the error if the read operation failed.
>
> [1] https://syzkaller.appspot.com/bug?extid=1f53a30781af65d2c955
>
> Signed-off-by: Javier Carrasco <[email protected]>
> Reported-and-tested-by: [email protected]
> ---
> Changes in v2:
> - Remove unnecessary 'err == 0' case
> - Link to v1: https://lore.kernel.org/r/20231009-topic-dm9601_uninit_mdio_read-v1-1-d4d775e24e3b@gmail.com
> ---
> drivers/net/usb/dm9601.c | 7 ++++++-
> 1 file changed, 6 insertions(+), 1 deletion(-)
What commit id does this fix?
thanks,
greg k-h
>>>>> "Greg" == Greg KH <[email protected]> writes:
> On Tue, Oct 10, 2023 at 12:26:14AM +0200, Javier Carrasco wrote:
>> syzbot has found an uninit-value bug triggered by the dm9601 driver [1].
>>
>> This error happens because the variable res is not updated if the call
>> to dm_read_shared_word returns an error. In this particular case -EPROTO
>> was returned and res stayed uninitialized.
>>
>> This can be avoided by checking the return value of dm_read_shared_word
>> and propagating the error if the read operation failed.
>>
>> [1] https://syzkaller.appspot.com/bug?extid=1f53a30781af65d2c955
>>
>> Signed-off-by: Javier Carrasco <[email protected]>
>> Reported-and-tested-by: [email protected]
>> ---
>> Changes in v2:
>> - Remove unnecessary 'err == 0' case
>> - Link to v1: https://lore.kernel.org/r/20231009-topic-dm9601_uninit_mdio_read-v1-1-d4d775e24e3b@gmail.com
>> ---
>> drivers/net/usb/dm9601.c | 7 ++++++-
>> 1 file changed, 6 insertions(+), 1 deletion(-)
> What commit id does this fix?
It has been there since the beginning, so:
Fixes: d0374f4f9c35cdfbee0 ("USB: Davicom DM9601 usbnet driver")
Acked-by: Peter Korsgaard <[email protected]>
--
Bye, Peter Korsgaard
On Tue, Oct 10, 2023 at 08:19:48AM +0200, Peter Korsgaard wrote:
> >>>>> "Greg" == Greg KH <[email protected]> writes:
>
> > On Tue, Oct 10, 2023 at 12:26:14AM +0200, Javier Carrasco wrote:
> >> syzbot has found an uninit-value bug triggered by the dm9601 driver [1].
> >>
> >> This error happens because the variable res is not updated if the call
> >> to dm_read_shared_word returns an error. In this particular case -EPROTO
> >> was returned and res stayed uninitialized.
> >>
> >> This can be avoided by checking the return value of dm_read_shared_word
> >> and propagating the error if the read operation failed.
> >>
> >> [1] https://syzkaller.appspot.com/bug?extid=1f53a30781af65d2c955
> >>
> >> Signed-off-by: Javier Carrasco <[email protected]>
> >> Reported-and-tested-by: [email protected]
> >> ---
> >> Changes in v2:
> >> - Remove unnecessary 'err == 0' case
> >> - Link to v1: https://lore.kernel.org/r/20231009-topic-dm9601_uninit_mdio_read-v1-1-d4d775e24e3b@gmail.com
> >> ---
> >> drivers/net/usb/dm9601.c | 7 ++++++-
> >> 1 file changed, 6 insertions(+), 1 deletion(-)
>
> > What commit id does this fix?
>
> It has been there since the beginning, so:
>
> Fixes: d0374f4f9c35cdfbee0 ("USB: Davicom DM9601 usbnet driver")
>
> Acked-by: Peter Korsgaard <[email protected]>
Great, can someone add a cc: stable@ tag for this too please?
thanks,
greg k-h
>>>>> "Greg" == Greg KH <[email protected]> writes:
> On Tue, Oct 10, 2023 at 08:19:48AM +0200, Peter Korsgaard wrote:
>> >>>>> "Greg" == Greg KH <[email protected]> writes:
>>
>> > On Tue, Oct 10, 2023 at 12:26:14AM +0200, Javier Carrasco wrote:
>> >> syzbot has found an uninit-value bug triggered by the dm9601 driver [1].
>> >>
>> >> This error happens because the variable res is not updated if the call
>> >> to dm_read_shared_word returns an error. In this particular case -EPROTO
>> >> was returned and res stayed uninitialized.
>> >>
>> >> This can be avoided by checking the return value of dm_read_shared_word
>> >> and propagating the error if the read operation failed.
>> >>
>> >> [1] https://syzkaller.appspot.com/bug?extid=1f53a30781af65d2c955
>> >>
>> >> Signed-off-by: Javier Carrasco <[email protected]>
>> >> Reported-and-tested-by: [email protected]
>> >> ---
>> >> Changes in v2:
>> >> - Remove unnecessary 'err == 0' case
>> >> - Link to v1: https://lore.kernel.org/r/20231009-topic-dm9601_uninit_mdio_read-v1-1-d4d775e24e3b@gmail.com
>> >> ---
>> >> drivers/net/usb/dm9601.c | 7 ++++++-
>> >> 1 file changed, 6 insertions(+), 1 deletion(-)
>>
>> > What commit id does this fix?
>>
>> It has been there since the beginning, so:
>>
>> Fixes: d0374f4f9c35cdfbee0 ("USB: Davicom DM9601 usbnet driver")
>>
>> Acked-by: Peter Korsgaard <[email protected]>
> Great, can someone add a cc: stable@ tag for this too please?
Cc: [email protected]
--
Bye, Peter Korsgaard
On Tue, Oct 10, 2023 at 09:22:49AM +0200, Peter Korsgaard wrote:
> >>>>> "Greg" == Greg KH <[email protected]> writes:
>
> > On Tue, Oct 10, 2023 at 08:19:48AM +0200, Peter Korsgaard wrote:
> >> >>>>> "Greg" == Greg KH <[email protected]> writes:
> >>
> >> > On Tue, Oct 10, 2023 at 12:26:14AM +0200, Javier Carrasco wrote:
> >> >> syzbot has found an uninit-value bug triggered by the dm9601 driver [1].
> >> >>
> >> >> This error happens because the variable res is not updated if the call
> >> >> to dm_read_shared_word returns an error. In this particular case -EPROTO
> >> >> was returned and res stayed uninitialized.
> >> >>
> >> >> This can be avoided by checking the return value of dm_read_shared_word
> >> >> and propagating the error if the read operation failed.
> >> >>
> >> >> [1] https://syzkaller.appspot.com/bug?extid=1f53a30781af65d2c955
> >> >>
> >> >> Signed-off-by: Javier Carrasco <[email protected]>
> >> >> Reported-and-tested-by: [email protected]
> >> >> ---
> >> >> Changes in v2:
> >> >> - Remove unnecessary 'err == 0' case
> >> >> - Link to v1: https://lore.kernel.org/r/20231009-topic-dm9601_uninit_mdio_read-v1-1-d4d775e24e3b@gmail.com
> >> >> ---
> >> >> drivers/net/usb/dm9601.c | 7 ++++++-
> >> >> 1 file changed, 6 insertions(+), 1 deletion(-)
> >>
> >> > What commit id does this fix?
> >>
> >> It has been there since the beginning, so:
> >>
> >> Fixes: d0374f4f9c35cdfbee0 ("USB: Davicom DM9601 usbnet driver")
> >>
> >> Acked-by: Peter Korsgaard <[email protected]>
>
> > Great, can someone add a cc: stable@ tag for this too please?
>
> Cc: [email protected]
<formletter>
This is not the correct way to submit patches for inclusion in the
stable kernel tree. Please read:
https://www.kernel.org/doc/html/latest/process/stable-kernel-rules.html
for how to do this properly.
</formletter>
Hello:
This patch was applied to netdev/net.git (main)
by Jakub Kicinski <[email protected]>:
On Tue, 10 Oct 2023 00:26:14 +0200 you wrote:
> syzbot has found an uninit-value bug triggered by the dm9601 driver [1].
>
> This error happens because the variable res is not updated if the call
> to dm_read_shared_word returns an error. In this particular case -EPROTO
> was returned and res stayed uninitialized.
>
> This can be avoided by checking the return value of dm_read_shared_word
> and propagating the error if the read operation failed.
>
> [...]
Here is the summary with links:
- [v2] net: usb: dm9601: fix uninitialized variable use in dm9601_mdio_read
https://git.kernel.org/netdev/net/c/8f8abb863fa5
You are awesome, thank you!
--
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html