| Message ID | 20231009-topic-dm9601_uninit_mdio_read-v2-1-f2fe39739b6c@gmail.com (mailing list archive) |
|---|---|
| State | Accepted |
| Commit | 8f8abb863fa5a4cc18955c6a0e17af0ded3e4a76 |
| Headers | show |
| Series | [v2] net: usb: dm9601: fix uninitialized variable use in dm9601_mdio_read | expand |
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 <javier.carrasco.cruz@gmail.com> > Reported-and-tested-by: syzbot+1f53a30781af65d2c955@syzkaller.appspotmail.com > --- > 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 <gregkh@linuxfoundation.org> 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 <javier.carrasco.cruz@gmail.com> >> Reported-and-tested-by: syzbot+1f53a30781af65d2c955@syzkaller.appspotmail.com >> --- >> 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 <peter@korsgaard.com>
On Tue, Oct 10, 2023 at 08:19:48AM +0200, Peter Korsgaard wrote: > >>>>> "Greg" == Greg KH <gregkh@linuxfoundation.org> 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 <javier.carrasco.cruz@gmail.com> > >> Reported-and-tested-by: syzbot+1f53a30781af65d2c955@syzkaller.appspotmail.com > >> --- > >> 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 <peter@korsgaard.com> Great, can someone add a cc: stable@ tag for this too please? thanks, greg k-h
>>>>> "Greg" == Greg KH <gregkh@linuxfoundation.org> writes: > On Tue, Oct 10, 2023 at 08:19:48AM +0200, Peter Korsgaard wrote: >> >>>>> "Greg" == Greg KH <gregkh@linuxfoundation.org> 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 <javier.carrasco.cruz@gmail.com> >> >> Reported-and-tested-by: syzbot+1f53a30781af65d2c955@syzkaller.appspotmail.com >> >> --- >> >> 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 <peter@korsgaard.com> > Great, can someone add a cc: stable@ tag for this too please? Cc: stable@vger.kernel.org
On Tue, Oct 10, 2023 at 09:22:49AM +0200, Peter Korsgaard wrote: > >>>>> "Greg" == Greg KH <gregkh@linuxfoundation.org> writes: > > > On Tue, Oct 10, 2023 at 08:19:48AM +0200, Peter Korsgaard wrote: > >> >>>>> "Greg" == Greg KH <gregkh@linuxfoundation.org> 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 <javier.carrasco.cruz@gmail.com> > >> >> Reported-and-tested-by: syzbot+1f53a30781af65d2c955@syzkaller.appspotmail.com > >> >> --- > >> >> 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 <peter@korsgaard.com> > > > Great, can someone add a cc: stable@ tag for this too please? > > Cc: stable@vger.kernel.org <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 <kuba@kernel.org>: 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!
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",
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 <javier.carrasco.cruz@gmail.com> Reported-and-tested-by: syzbot+1f53a30781af65d2c955@syzkaller.appspotmail.com --- 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(-) --- base-commit: 94f6f0550c625fab1f373bb86a6669b45e9748b3 change-id: 20231009-topic-dm9601_uninit_mdio_read-a15918ffbd6f Best regards,