[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <87zg0rcyjl.fsf@48ers.dk>
Date: Mon, 09 Oct 2023 22:48:30 +0200
From: Peter Korsgaard <peter@...sgaard.com>
To: Javier Carrasco <javier.carrasco.cruz@...il.com>
Cc: "David S. Miller" <davem@...emloft.net>,
Eric Dumazet <edumazet@...gle.com>,
Jakub Kicinski <kuba@...nel.org>,
Paolo Abeni <pabeni@...hat.com>,
syzbot+1f53a30781af65d2c955@...kaller.appspotmail.com,
netdev@...r.kernel.org, linux-usb@...r.kernel.org,
linux-kernel@...r.kernel.org
Subject: Re: [PATCH] net: usb: dm9601: fix uninitialized variable use in
dm9601_mdio_read
>>>>> "Javier" == Javier Carrasco <javier.carrasco.cruz@...il.com> writes:
> 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 or if no data is read (see
> __usbnet_read_cmd()). 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 returning an error if the read operation failed or no data was read.
> [1] https://syzkaller.appspot.com/bug?extid=1f53a30781af65d2c955
> Signed-off-by: Javier Carrasco <javier.carrasco.cruz@...il.com>
> Reported-and-tested-by: syzbot+1f53a30781af65d2c955@...kaller.appspotmail.com
> ---
> drivers/net/usb/dm9601.c | 9 ++++++++-
> 1 file changed, 8 insertions(+), 1 deletion(-)
> diff --git a/drivers/net/usb/dm9601.c b/drivers/net/usb/dm9601.c
> index 48d7d278631e..e223daa93229 100644
> --- a/drivers/net/usb/dm9601.c
> +++ b/drivers/net/usb/dm9601.c
> @@ -222,13 +222,20 @@ 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) {
> + if (err == 0)
> + err = -ENODATA;
Looking at dm_read(), it doesn't look like we can end up here with err
== 0, but OK.
Acked-by: Peter Korsgaard <peter@...sgaard.com>
--
Bye, Peter Korsgaard
Powered by blists - more mailing lists