From nobody Tue Sep 16 23:55:20 2025 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by smtp.lore.kernel.org (Postfix) with ESMTP id CA8D6C3DA79 for ; Mon, 26 Dec 2022 11:48:49 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S231693AbiLZLsp (ORCPT ); Mon, 26 Dec 2022 06:48:45 -0500 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:33856 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S229450AbiLZLsn (ORCPT ); Mon, 26 Dec 2022 06:48:43 -0500 Received: from forwardcorp1a.mail.yandex.net (forwardcorp1a.mail.yandex.net [178.154.239.72]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id 93A3D2FB; Mon, 26 Dec 2022 03:48:42 -0800 (PST) Received: from vla1-81430ab5870b.qloud-c.yandex.net (vla1-81430ab5870b.qloud-c.yandex.net [IPv6:2a02:6b8:c0d:35a1:0:640:8143:ab5]) by forwardcorp1a.mail.yandex.net (Yandex) with ESMTP id CEFD85FCE7; Mon, 26 Dec 2022 14:48:40 +0300 (MSK) Received: from d-tatianin-nix.yandex-team.ru (unknown [2a02:6b8:b081:1::1:f]) by vla1-81430ab5870b.qloud-c.yandex.net (smtpcorp/Yandex) with ESMTPSA id SmMqpV0Q0uQ1-IbutsOwc; Mon, 26 Dec 2022 14:48:40 +0300 X-Yandex-Fwd: 1 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=yandex-team.ru; s=default; t=1672055320; bh=Wl46n9GzEekweDAs/Z8zerV+l+QIWCUnBlczJiEGt0c=; h=Message-Id:Date:In-Reply-To:Cc:Subject:References:To:From; b=q0UggCoPnCZNu0GuMmnwmxZbLUamlM5m0fItS/NudL461zYOxkjVZNcZKAWq2o0qs Xle0rtoLgw3/FbSBJfmjoWfGUOfmnanmGbllRKf5m5E98WOn7E47L7RCIZLanO5lon MU8y+Yp6UgZbTIDelR6lxq2J/kHiUFhW1xydSGQs= Authentication-Results: vla1-81430ab5870b.qloud-c.yandex.net; dkim=pass header.i=@yandex-team.ru From: Daniil Tatianin To: "David S. Miller" Cc: Daniil Tatianin , Eric Dumazet , Jakub Kicinski , Paolo Abeni , Andrew Lunn , Sean Anderson , Jiri Pirko , Wolfram Sang , Maxim Korotkov , Gal Pressman , Vincent Mailhol , Tom Rix , Marco Bonelli , netdev@vger.kernel.org, linux-kernel@vger.kernel.org Subject: [PATCH net v2 1/3] net/ethtool/ioctl: return -EOPNOTSUPP if we have no phy stats Date: Mon, 26 Dec 2022 14:48:23 +0300 Message-Id: <20221226114825.1937189-2-d-tatianin@yandex-team.ru> X-Mailer: git-send-email 2.25.1 In-Reply-To: <20221226114825.1937189-1-d-tatianin@yandex-team.ru> References: <20221226114825.1937189-1-d-tatianin@yandex-team.ru> MIME-Version: 1.0 Content-Transfer-Encoding: quoted-printable Precedence: bulk List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Type: text/plain; charset="utf-8" It's not very useful to copy back an empty ethtool_stats struct and return 0 if we didn't actually have any stats. This also allows for further simplification of this function in the future commits. Signed-off-by: Daniil Tatianin Reviewed-by: Andrew Lunn --- net/ethtool/ioctl.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/net/ethtool/ioctl.c b/net/ethtool/ioctl.c index 57e7238a4136..e8a294b38b7b 100644 --- a/net/ethtool/ioctl.c +++ b/net/ethtool/ioctl.c @@ -2093,7 +2093,8 @@ static int ethtool_get_phy_stats(struct net_device *d= ev, void __user *useraddr) return n_stats; if (n_stats > S32_MAX / sizeof(u64)) return -ENOMEM; - WARN_ON_ONCE(!n_stats); + if (WARN_ON_ONCE(!n_stats)) + return -EOPNOTSUPP; =20 if (copy_from_user(&stats, useraddr, sizeof(stats))) return -EFAULT; --=20 2.25.1 From nobody Tue Sep 16 23:55:20 2025 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by smtp.lore.kernel.org (Postfix) with ESMTP id 976C4C3DA79 for ; Mon, 26 Dec 2022 11:49:00 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S231645AbiLZLs6 (ORCPT ); Mon, 26 Dec 2022 06:48:58 -0500 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:33862 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S229960AbiLZLso (ORCPT ); Mon, 26 Dec 2022 06:48:44 -0500 Received: from forwardcorp1a.mail.yandex.net (forwardcorp1a.mail.yandex.net [178.154.239.72]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id 0AF2EF34; Mon, 26 Dec 2022 03:48:43 -0800 (PST) Received: from vla1-81430ab5870b.qloud-c.yandex.net (vla1-81430ab5870b.qloud-c.yandex.net [IPv6:2a02:6b8:c0d:35a1:0:640:8143:ab5]) by forwardcorp1a.mail.yandex.net (Yandex) with ESMTP id 7E5585FCCF; Mon, 26 Dec 2022 14:48:42 +0300 (MSK) Received: from d-tatianin-nix.yandex-team.ru (unknown [2a02:6b8:b081:1::1:f]) by vla1-81430ab5870b.qloud-c.yandex.net (smtpcorp/Yandex) with ESMTPSA id SmMqpV0Q0uQ1-ma6jHp3e; Mon, 26 Dec 2022 14:48:41 +0300 X-Yandex-Fwd: 1 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=yandex-team.ru; s=default; t=1672055322; bh=Bc0UQBiLcPIeNMMXHPDirTDhvMlEZMdLdU3YkqOgz/I=; h=Message-Id:Date:In-Reply-To:Cc:Subject:References:To:From; b=N5T9G93zwobBipUq19jbhhBmDj8j0GgPwems6oUkB37L7dASt/SC/y8DdxCdz/DXx ZCWqVzQhJ6XOaVACPADlS/nCNQOQQzE9hVV2tEyj6CSFlW+qGu6MGuZny51XgOXiu6 HE4zINwgOn+Ld1rA8mXZLtYchhOBP8U6hUdRMBKQ= Authentication-Results: vla1-81430ab5870b.qloud-c.yandex.net; dkim=pass header.i=@yandex-team.ru From: Daniil Tatianin To: "David S. Miller" Cc: Daniil Tatianin , Eric Dumazet , Jakub Kicinski , Paolo Abeni , Andrew Lunn , Sean Anderson , Jiri Pirko , Wolfram Sang , Maxim Korotkov , Gal Pressman , Vincent Mailhol , Tom Rix , Marco Bonelli , netdev@vger.kernel.org, linux-kernel@vger.kernel.org Subject: [PATCH net v2 2/3] net/ethtool/ioctl: remove if n_stats checks from ethtool_get_phy_stats Date: Mon, 26 Dec 2022 14:48:24 +0300 Message-Id: <20221226114825.1937189-3-d-tatianin@yandex-team.ru> X-Mailer: git-send-email 2.25.1 In-Reply-To: <20221226114825.1937189-1-d-tatianin@yandex-team.ru> References: <20221226114825.1937189-1-d-tatianin@yandex-team.ru> MIME-Version: 1.0 Content-Transfer-Encoding: quoted-printable Precedence: bulk List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Type: text/plain; charset="utf-8" Now that we always early return if we don't have any stats we can remove these checks as they're no longer necessary. Signed-off-by: Daniil Tatianin Reviewed-by: Andrew Lunn --- net/ethtool/ioctl.c | 24 ++++++++++-------------- 1 file changed, 10 insertions(+), 14 deletions(-) diff --git a/net/ethtool/ioctl.c b/net/ethtool/ioctl.c index e8a294b38b7b..3379af21c29f 100644 --- a/net/ethtool/ioctl.c +++ b/net/ethtool/ioctl.c @@ -2101,28 +2101,24 @@ static int ethtool_get_phy_stats(struct net_device = *dev, void __user *useraddr) =20 stats.n_stats =3D n_stats; =20 - if (n_stats) { - data =3D vzalloc(array_size(n_stats, sizeof(u64))); - if (!data) - return -ENOMEM; + data =3D vzalloc(array_size(n_stats, sizeof(u64))); + if (!data) + return -ENOMEM; =20 - if (phydev && !ops->get_ethtool_phy_stats && - phy_ops && phy_ops->get_stats) { - ret =3D phy_ops->get_stats(phydev, &stats, data); - if (ret < 0) - goto out; - } else { - ops->get_ethtool_phy_stats(dev, &stats, data); - } + if (phydev && !ops->get_ethtool_phy_stats && + phy_ops && phy_ops->get_stats) { + ret =3D phy_ops->get_stats(phydev, &stats, data); + if (ret < 0) + goto out; } else { - data =3D NULL; + ops->get_ethtool_phy_stats(dev, &stats, data); } =20 ret =3D -EFAULT; if (copy_to_user(useraddr, &stats, sizeof(stats))) goto out; useraddr +=3D sizeof(stats); - if (n_stats && copy_to_user(useraddr, data, array_size(n_stats, sizeof(u6= 4)))) + if (copy_to_user(useraddr, data, array_size(n_stats, sizeof(u64)))) goto out; ret =3D 0; =20 --=20 2.25.1 From nobody Tue Sep 16 23:55:20 2025 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by smtp.lore.kernel.org (Postfix) with ESMTP id 5E093C3DA79 for ; Mon, 26 Dec 2022 11:49:18 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S231980AbiLZLtP (ORCPT ); Mon, 26 Dec 2022 06:49:15 -0500 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:33954 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S231688AbiLZLsv (ORCPT ); Mon, 26 Dec 2022 06:48:51 -0500 Received: from forwardcorp1a.mail.yandex.net (forwardcorp1a.mail.yandex.net [178.154.239.72]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id ED9E9E61; Mon, 26 Dec 2022 03:48:48 -0800 (PST) Received: from vla1-81430ab5870b.qloud-c.yandex.net (vla1-81430ab5870b.qloud-c.yandex.net [IPv6:2a02:6b8:c0d:35a1:0:640:8143:ab5]) by forwardcorp1a.mail.yandex.net (Yandex) with ESMTP id E52165FD70; Mon, 26 Dec 2022 14:48:44 +0300 (MSK) Received: from d-tatianin-nix.yandex-team.ru (unknown [2a02:6b8:b081:1::1:f]) by vla1-81430ab5870b.qloud-c.yandex.net (smtpcorp/Yandex) with ESMTPSA id SmMqpV0Q0uQ1-VihaauT0; Mon, 26 Dec 2022 14:48:44 +0300 X-Yandex-Fwd: 1 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=yandex-team.ru; s=default; t=1672055324; bh=Nuxuuf1lhfDfoOk+xEW3obUWSUiFsuqUm9EH4ZXnZWg=; h=Message-Id:Date:In-Reply-To:Cc:Subject:References:To:From; b=F6GmxQu06XHnODjrFlMQpZEjNhsEVaS7BQXQS+noC42KMJ1Pg/w6GKDnaEZYgyu1d XpvxgmQo+ab7DZ5AXKWqhS5dZoF3cP271Ho3L13PK3H7SzUB0vyGFxcFQ/TA4oirML m4tCb4vstm2JtrDKSNH6dnwCxu4hXy1pQ1esjSuU= Authentication-Results: vla1-81430ab5870b.qloud-c.yandex.net; dkim=pass header.i=@yandex-team.ru From: Daniil Tatianin To: "David S. Miller" Cc: Daniil Tatianin , Eric Dumazet , Jakub Kicinski , Paolo Abeni , Andrew Lunn , Sean Anderson , Jiri Pirko , Wolfram Sang , Maxim Korotkov , Gal Pressman , Vincent Mailhol , Tom Rix , Marco Bonelli , netdev@vger.kernel.org, linux-kernel@vger.kernel.org Subject: [PATCH net v2 3/3] net/ethtool/ioctl: split ethtool_get_phy_stats into multiple helpers Date: Mon, 26 Dec 2022 14:48:25 +0300 Message-Id: <20221226114825.1937189-4-d-tatianin@yandex-team.ru> X-Mailer: git-send-email 2.25.1 In-Reply-To: <20221226114825.1937189-1-d-tatianin@yandex-team.ru> References: <20221226114825.1937189-1-d-tatianin@yandex-team.ru> MIME-Version: 1.0 Content-Transfer-Encoding: quoted-printable Precedence: bulk List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Type: text/plain; charset="utf-8" So that it's easier to follow and make sense of the branching and various conditions. Stats retrieval has been split into two separate functions ethtool_get_phy_stats_phydev & ethtool_get_phy_stats_ethtool. The former attempts to retrieve the stats using phydev & phy_ops, while the latter uses ethtool_ops. Actual n_stats validation & array allocation has been moved into a new ethtool_vzalloc_stats_array helper. This also fixes a potential NULL dereference of ops->get_ethtool_phy_stats where it was getting called in an else branch unconditionally without making sure it was actually present. Found by Linux Verification Center (linuxtesting.org) with the SVACE static analysis tool. Signed-off-by: Daniil Tatianin Reviewed-by: Andrew Lunn --- net/ethtool/ioctl.c | 102 ++++++++++++++++++++++++++++++-------------- 1 file changed, 69 insertions(+), 33 deletions(-) diff --git a/net/ethtool/ioctl.c b/net/ethtool/ioctl.c index 3379af21c29f..36792633ce5f 100644 --- a/net/ethtool/ioctl.c +++ b/net/ethtool/ioctl.c @@ -2072,23 +2072,8 @@ static int ethtool_get_stats(struct net_device *dev,= void __user *useraddr) return ret; } =20 -static int ethtool_get_phy_stats(struct net_device *dev, void __user *user= addr) +static int ethtool_vzalloc_stats_array(int n_stats, u64 **data) { - const struct ethtool_phy_ops *phy_ops =3D ethtool_phy_ops; - const struct ethtool_ops *ops =3D dev->ethtool_ops; - struct phy_device *phydev =3D dev->phydev; - struct ethtool_stats stats; - u64 *data; - int ret, n_stats; - - if (!phydev && (!ops->get_ethtool_phy_stats || !ops->get_sset_count)) - return -EOPNOTSUPP; - - if (phydev && !ops->get_ethtool_phy_stats && - phy_ops && phy_ops->get_sset_count) - n_stats =3D phy_ops->get_sset_count(phydev); - else - n_stats =3D ops->get_sset_count(dev, ETH_SS_PHY_STATS); if (n_stats < 0) return n_stats; if (n_stats > S32_MAX / sizeof(u64)) @@ -2096,31 +2081,82 @@ static int ethtool_get_phy_stats(struct net_device = *dev, void __user *useraddr) if (WARN_ON_ONCE(!n_stats)) return -EOPNOTSUPP; =20 + *data =3D vzalloc(array_size(n_stats, sizeof(u64))); + if (!*data) + return -ENOMEM; + + return 0; +} + +static int ethtool_get_phy_stats_phydev(struct phy_device *phydev, + struct ethtool_stats *stats, + u64 **data) + { + const struct ethtool_phy_ops *phy_ops =3D ethtool_phy_ops; + int n_stats, ret; + + if (!phy_ops || !phy_ops->get_sset_count || !phy_ops->get_stats) + return -EOPNOTSUPP; + + n_stats =3D phy_ops->get_sset_count(phydev); + + ret =3D ethtool_vzalloc_stats_array(n_stats, data); + if (ret) + return ret; + + stats->n_stats =3D n_stats; + return phy_ops->get_stats(phydev, stats, *data); +} + +static int ethtool_get_phy_stats_ethtool(struct net_device *dev, + struct ethtool_stats *stats, + u64 **data) +{ + const struct ethtool_ops *ops =3D dev->ethtool_ops; + int n_stats, ret; + + if (!ops || !ops->get_sset_count || ops->get_ethtool_phy_stats) + return -EOPNOTSUPP; + + n_stats =3D ops->get_sset_count(dev, ETH_SS_PHY_STATS); + + ret =3D ethtool_vzalloc_stats_array(n_stats, data); + if (ret) + return ret; + + stats->n_stats =3D n_stats; + ops->get_ethtool_phy_stats(dev, stats, *data); + + return 0; +} + +static int ethtool_get_phy_stats(struct net_device *dev, void __user *user= addr) +{ + struct phy_device *phydev =3D dev->phydev; + struct ethtool_stats stats; + u64 *data =3D NULL; + int ret =3D -EOPNOTSUPP; + if (copy_from_user(&stats, useraddr, sizeof(stats))) return -EFAULT; =20 - stats.n_stats =3D n_stats; + if (phydev) + ret =3D ethtool_get_phy_stats_phydev(phydev, &stats, &data); =20 - data =3D vzalloc(array_size(n_stats, sizeof(u64))); - if (!data) - return -ENOMEM; + if (ret =3D=3D -EOPNOTSUPP) + ret =3D ethtool_get_phy_stats_ethtool(dev, &stats, &data); =20 - if (phydev && !ops->get_ethtool_phy_stats && - phy_ops && phy_ops->get_stats) { - ret =3D phy_ops->get_stats(phydev, &stats, data); - if (ret < 0) - goto out; - } else { - ops->get_ethtool_phy_stats(dev, &stats, data); - } + if (ret) + goto out; =20 - ret =3D -EFAULT; - if (copy_to_user(useraddr, &stats, sizeof(stats))) + if (copy_to_user(useraddr, &stats, sizeof(stats))) { + ret =3D -EFAULT; goto out; + } + useraddr +=3D sizeof(stats); - if (copy_to_user(useraddr, data, array_size(n_stats, sizeof(u64)))) - goto out; - ret =3D 0; + if (copy_to_user(useraddr, data, array_size(stats.n_stats, sizeof(u64)))) + ret =3D -EFAULT; =20 out: vfree(data); --=20 2.25.1