Message ID | 20231009-topic-dm9601_uninit_mdio_read-v1-1-d4d775e24e3b@gmail.com |
---|---|
State | New |
Headers |
Return-Path: <linux-kernel-owner@vger.kernel.org> Delivered-To: ouuuleilei@gmail.com Received: by 2002:a59:a888:0:b0:403:3b70:6f57 with SMTP id x8csp2061771vqo; Mon, 9 Oct 2023 11:53:26 -0700 (PDT) X-Google-Smtp-Source: AGHT+IGqpd8yyqyYpYY6ijXanL4DpF0x/Q1MEBOpvcDhroM3yh3XjBvp3XEb0aswU8zCZ3SoJrUI X-Received: by 2002:a05:6808:1913:b0:3a7:1e3e:7f97 with SMTP id bf19-20020a056808191300b003a71e3e7f97mr21368407oib.4.1696877606497; Mon, 09 Oct 2023 11:53:26 -0700 (PDT) ARC-Seal: i=1; a=rsa-sha256; t=1696877606; cv=none; d=google.com; s=arc-20160816; b=ONboSiMJDspdZJ+Vb8FDSozVlYaIuZ6p0XQ2sIYtn5iU2p1MAGh9f2pAG8O024MIAy tgy/nGfDCiBYcHb4yQ+X1QS9OjuOjdqHZMeANEgjrkkmORdWwk3rjq7JHOnbKp3jq40m 3bl8OgQNto+Tt+Ilqk8UeDew8D305qRrkfssuXQ4fH9WzfkiNf7w+8Tirw/BzrKOZk9Q G96wC0t6fx0tgpP7LeTFhK/ayRh/kYjmB7BU7u3DW+7aA/NwoPemAVXjWbDmTO9sTsfK 6Z9GcKHCRMhAY2ycDbPDjdN4iH2IkvSpoX9xM7JC+BrejkhBY0I3yBmoK+pxN1iQpk4H UsZw== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=arc-20160816; h=list-id:precedence:cc:to:message-id:content-transfer-encoding :mime-version:subject:date:from:dkim-signature; bh=4iEFw2KWu0uLaXIWh95J9DkXgQ94Tm3AGN4RoICln98=; fh=okQzaQzMX02auk420YO387SZa2vWu0QtcfsnN3ik99U=; b=SWkPbVCmFPJ2P3s7NVPlR2xr7Yd5SOfwrNT0kFGXvWrldyB5tLx7foE+8mh2ktgmHN +b2exkWABi3+WvIzMo8Hv4h82WikSaGB6PvE4Q+TnWiAPvixk5hd5EDQ1u5odVDaa1AT 6nu2T2LGdin75FuUPTf73LKLR1UrioyU4qRkcNphaqsgIMcz5HaZl8aab5ZkqxdYmg/W ebbdQ7vVCDks+Mfk6DZKFEau5ZFU9iDNKgG0cOWjH2aPQ82XS8knR5VyDBtcBkF5+nXy 253VPmmJH2BgvbdLddaK9briJGhFhWBWzTy/1u3FwgPmgdOwg2er2yH9cP1ToBLbsHcQ qjUw== ARC-Authentication-Results: i=1; mx.google.com; dkim=pass header.i=@gmail.com header.s=20230601 header.b=Lv21Hteb; spf=pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::3:1 as permitted sender) smtp.mailfrom=linux-kernel-owner@vger.kernel.org; dmarc=pass (p=NONE sp=QUARANTINE dis=NONE) header.from=gmail.com Received: from morse.vger.email (morse.vger.email. [2620:137:e000::3:1]) by mx.google.com with ESMTPS id l62-20020a638841000000b005890aa054fcsi9612714pgd.400.2023.10.09.11.53.26 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Mon, 09 Oct 2023 11:53:26 -0700 (PDT) Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::3:1 as permitted sender) client-ip=2620:137:e000::3:1; Authentication-Results: mx.google.com; dkim=pass header.i=@gmail.com header.s=20230601 header.b=Lv21Hteb; spf=pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::3:1 as permitted sender) smtp.mailfrom=linux-kernel-owner@vger.kernel.org; dmarc=pass (p=NONE sp=QUARANTINE dis=NONE) header.from=gmail.com Received: from out1.vger.email (depot.vger.email [IPv6:2620:137:e000::3:0]) by morse.vger.email (Postfix) with ESMTP id 490E08104F55; Mon, 9 Oct 2023 11:53:23 -0700 (PDT) X-Virus-Status: Clean X-Virus-Scanned: clamav-milter 0.103.10 at morse.vger.email Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1378225AbjJISxE (ORCPT <rfc822;ezelljr.billy@gmail.com> + 18 others); Mon, 9 Oct 2023 14:53:04 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:54226 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1378139AbjJISxC (ORCPT <rfc822;linux-kernel@vger.kernel.org>); Mon, 9 Oct 2023 14:53:02 -0400 Received: from mail-ej1-x644.google.com (mail-ej1-x644.google.com [IPv6:2a00:1450:4864:20::644]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id A05A0AF; Mon, 9 Oct 2023 11:53:00 -0700 (PDT) Received: by mail-ej1-x644.google.com with SMTP id a640c23a62f3a-991c786369cso814835966b.1; Mon, 09 Oct 2023 11:53:00 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20230601; t=1696877579; x=1697482379; darn=vger.kernel.org; h=cc:to:message-id:content-transfer-encoding:mime-version:subject :date:from:from:to:cc:subject:date:message-id:reply-to; bh=4iEFw2KWu0uLaXIWh95J9DkXgQ94Tm3AGN4RoICln98=; b=Lv21Htebuds0cZAr+gOXmTjJMSbr2Z3yuu4bb9jt0U2CPeab+yAh31yIXeBTjmBrZb Ujn7HtoPX3MzXJQKyj4ozksyVnoMVyMsfV0JXF2FKLv7y7QHt0omlDDjWeu473ubdS3N eD2bQORfMkDZVNd9fj+kfUiwO3BKuYuKdZt5wE94D+Y9lYyJ5dgZ7GjbRGbh80ATREUE lFakJ2AvQc5RZlXz5zIJ7+DrG6pi3NypNek1LcjwdZKRvFP6k4+XrYXQeSTMI8vOCdJ9 NKGVT0hpAUFpsLcIwcyIdIncG+yRHgZn7rYjeDbt+bdChLpERT7H5lN0IQd5bGf4YoEq EDXg== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1696877579; x=1697482379; h=cc:to:message-id:content-transfer-encoding:mime-version:subject :date:from:x-gm-message-state:from:to:cc:subject:date:message-id :reply-to; bh=4iEFw2KWu0uLaXIWh95J9DkXgQ94Tm3AGN4RoICln98=; b=InXUxhHfQJx2sKjVFTjVIhgJkxqYnW1D79Oiaxs7lHQc9uy6mpFn0Nq2WFs436irET rH1oiioYu3xDHHVyQBUTbMe+rWr5nGbW2PMUhKyEW+bWiFbMZPCayw4jR9UkrMjUbWmE pXd72odMF5xacHwGe5lNrgG6+803c563E0+2FEo9H4ih55VllQaPDHcJ2KMfsMa0lA1M fSqqNq5Re7eyP+DkR0hJO0fDuwi1W/HVOmDGW6eA97uudFH4hlk+EZRnsorScntayFLr 8fETbZ9ywRa1qZwF5THzbrzQY5wchl0/CTN/B1NncFy4P0PMYiHEcdFkVd4jIo8ZsIdy +9/w== X-Gm-Message-State: AOJu0YyarLPo8JIHT1XR0xSc+Ms1htpK4JoXAls4kCUzFslCYw8jSY7X 2RKwjy5amKTnsOP00TDDkY0= X-Received: by 2002:a17:906:fe4a:b0:9ae:5120:5147 with SMTP id wz10-20020a170906fe4a00b009ae51205147mr17596835ejb.38.1696877579020; Mon, 09 Oct 2023 11:52:59 -0700 (PDT) Received: from [127.0.1.1] (2a02-8389-41cf-e200-0d7c-652f-4e74-10b8.cable.dynamic.v6.surfer.at. [2a02:8389:41cf:e200:d7c:652f:4e74:10b8]) by smtp.gmail.com with ESMTPSA id dc4-20020a170906c7c400b0098e34446464sm7115079ejb.25.2023.10.09.11.52.58 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Mon, 09 Oct 2023 11:52:58 -0700 (PDT) From: Javier Carrasco <javier.carrasco.cruz@gmail.com> Date: Mon, 09 Oct 2023 20:52:55 +0200 Subject: [PATCH] net: usb: dm9601: fix uninitialized variable use in dm9601_mdio_read MIME-Version: 1.0 Content-Type: text/plain; charset="utf-8" Content-Transfer-Encoding: 7bit Message-Id: <20231009-topic-dm9601_uninit_mdio_read-v1-1-d4d775e24e3b@gmail.com> X-B4-Tracking: v=1; b=H4sIAAZMJGUC/x2NQQrDIBAAvxL2XEFTGmq/UoqscW32oIbVlELI3 2t6HAZmdqgkTBUeww5CH65ccgdzGWBeML9JcegMox6vRmurWll5ViHZSRu3Zc7cXApcnBAGheZ mzT1GH6YIveGxkvKCeV7OSsLaSE6xCkX+/sfP13H8APazsqmIAAAA To: Peter Korsgaard <peter@korsgaard.com>, "David S. Miller" <davem@davemloft.net>, Eric Dumazet <edumazet@google.com>, Jakub Kicinski <kuba@kernel.org>, Paolo Abeni <pabeni@redhat.com>, syzbot+1f53a30781af65d2c955@syzkaller.appspotmail.com Cc: netdev@vger.kernel.org, linux-usb@vger.kernel.org, linux-kernel@vger.kernel.org, Javier Carrasco <javier.carrasco.cruz@gmail.com> X-Mailer: b4 0.12.0 X-Developer-Signature: v=1; a=ed25519-sha256; t=1696877577; l=1746; i=javier.carrasco.cruz@gmail.com; s=20230509; h=from:subject:message-id; bh=mr8zo8GkO8oFsYnOO1Ir3z/Ohnh9ZkCWi74Q+XjdrAw=; b=Ci4voJZHNqhGCYzpafxEBwQNK2Id3/J6q0wWFflh/IM3YXHIfzqgqFA9hmTzXxoS1OJAXksGz LWVNIUycrJ1C1HWJbFsXM1X/UmIAfk6Gh9utnaVXx36HBu1VipFOaVJ X-Developer-Key: i=javier.carrasco.cruz@gmail.com; a=ed25519; pk=tIGJV7M+tCizagNijF0eGMBGcOsPD+0cWGfKjl4h6K8= X-Spam-Status: No, score=3.0 required=5.0 tests=DKIM_SIGNED,DKIM_VALID, DKIM_VALID_AU,FREEMAIL_FORGED_FROMDOMAIN,FREEMAIL_FROM, HEADER_FROM_DIFFERENT_DOMAINS,MAILING_LIST_MULTI,RCVD_IN_SBL_CSS, SPF_HELO_NONE,SPF_PASS autolearn=no autolearn_force=no version=3.4.6 X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on morse.vger.email Precedence: bulk List-ID: <linux-kernel.vger.kernel.org> X-Mailing-List: linux-kernel@vger.kernel.org X-Greylist: Sender passed SPF test, not delayed by milter-greylist-4.6.4 (morse.vger.email [0.0.0.0]); Mon, 09 Oct 2023 11:53:23 -0700 (PDT) X-Spam-Level: ** X-getmail-retrieved-from-mailbox: INBOX X-GMAIL-THRID: 1779305132660662575 X-GMAIL-MSGID: 1779305132660662575 |
Series |
net: usb: dm9601: fix uninitialized variable use in dm9601_mdio_read
|
|
Commit Message
Javier Carrasco
Oct. 9, 2023, 6:52 p.m. UTC
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@gmail.com>
Reported-and-tested-by: syzbot+1f53a30781af65d2c955@syzkaller.appspotmail.com
---
drivers/net/usb/dm9601.c | 9 ++++++++-
1 file changed, 8 insertions(+), 1 deletion(-)
---
base-commit: 94f6f0550c625fab1f373bb86a6669b45e9748b3
change-id: 20231009-topic-dm9601_uninit_mdio_read-a15918ffbd6f
Best regards,
Comments
>>>>> "Javier" == Javier Carrasco <javier.carrasco.cruz@gmail.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@gmail.com> > Reported-and-tested-by: syzbot+1f53a30781af65d2c955@syzkaller.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@korsgaard.com>
Hi Peter, On 09.10.23 22:48, Peter Korsgaard 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 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@gmail.com> > > Reported-and-tested-by: syzbot+1f53a30781af65d2c955@syzkaller.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.You are right, I just simulated the err = 0 value from __usbnet_read_cmd() and in this case it is harmless because dm_read is called with length != 0 (in dm_read_shared_word either 1 or 2) and therefore it returns -EINVAL. A silly case where this would fail would be if length == 0, which would be wrong anyways. So I will remove the err == 0 case for v2. Thanks a lot for your feedback. > Acked-by: Peter Korsgaard <peter@korsgaard.com> > Best regards, Javier Carrasco
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; + 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",