From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from shadbolt.e.decadent.org.uk ([88.96.1.126]:57674 "EHLO shadbolt.e.decadent.org.uk" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S932443AbeCMXjv (ORCPT ); Tue, 13 Mar 2018 19:39:51 -0400 Date: Tue, 13 Mar 2018 23:39:48 +0000 From: Ben Hutchings To: Yisen Zhuang , Salil Mehta Cc: netdev@vger.kernel.org Message-ID: <20180313233948.GG8564@decadent.org.uk> MIME-Version: 1.0 Content-Type: multipart/signed; micalg=pgp-sha512; protocol="application/pgp-signature"; boundary="1y1tiN5hVw5cPBDe" Content-Disposition: inline Subject: [PATCH net 1/2] hns: Fix string set validation in ethtool string operations Sender: netdev-owner@vger.kernel.org List-ID: --1y1tiN5hVw5cPBDe Content-Type: text/plain; charset=us-ascii Content-Disposition: inline Content-Transfer-Encoding: quoted-printable The hns driver used to assume that it would only ever be asked about ETH_SS_TEST or ETH_SS_STATS, and for any other stringset it would claim a count of 0 but if asked for names would copy over all the statistic names. This resulted in a potential heap buffer overflow. This was "fixed" some time ago by making it report the number of statistics when asked about the ETH_SS_PRIV_FLAGS stringset. This doesn't make any sense, and it left the bug still exploitable with other stringset numbers. As a minimal but complete fix, stop calling the driver-internal string operations for any stringset other than ETH_SS_STATS. Fixes: 511e6bc071db ("net: add Hisilicon Network Subsystem DSAF support") Fixes: 412b65d15a7f ("net: hns: fix ethtool_get_strings overflow ...") Signed-off-by: Ben Hutchings --- drivers/net/ethernet/hisilicon/hns/hns_ethtool.c | 17 +++++------------ 1 file changed, 5 insertions(+), 12 deletions(-) diff --git a/drivers/net/ethernet/hisilicon/hns/hns_ethtool.c b/drivers/net= /ethernet/hisilicon/hns/hns_ethtool.c index 7ea7f8a4aa2a..b836742f7ab6 100644 --- a/drivers/net/ethernet/hisilicon/hns/hns_ethtool.c +++ b/drivers/net/ethernet/hisilicon/hns/hns_ethtool.c @@ -889,11 +889,6 @@ void hns_get_strings(struct net_device *netdev, u32 st= ringset, u8 *data) struct hnae_handle *h =3D priv->ae_handle; char *buff =3D (char *)data; =20 - if (!h->dev->ops->get_strings) { - netdev_err(netdev, "h->dev->ops->get_strings is null!\n"); - return; - } - if (stringset =3D=3D ETH_SS_TEST) { if (priv->ae_handle->phy_if !=3D PHY_INTERFACE_MODE_XGMII) { memcpy(buff, hns_nic_test_strs[MAC_INTERNALLOOP_MAC], @@ -907,7 +902,7 @@ void hns_get_strings(struct net_device *netdev, u32 str= ingset, u8 *data) memcpy(buff, hns_nic_test_strs[MAC_INTERNALLOOP_PHY], ETH_GSTRING_LEN); =20 - } else { + } else if (stringset =3D=3D ETH_SS_STATS && h->dev->ops->get_strings) { snprintf(buff, ETH_GSTRING_LEN, "rx_packets"); buff =3D buff + ETH_GSTRING_LEN; snprintf(buff, ETH_GSTRING_LEN, "tx_packets"); @@ -969,7 +964,7 @@ void hns_get_strings(struct net_device *netdev, u32 str= ingset, u8 *data) /** * nic_get_sset_count - get string set count witch returned by nic_get_str= ings. * @dev: net device - * @stringset: string set index, 0: self test string; 1: statistics string. + * @stringset: string set index * * Return string set count. */ @@ -979,10 +974,6 @@ int hns_get_sset_count(struct net_device *netdev, int = stringset) struct hnae_handle *h =3D priv->ae_handle; struct hnae_ae_ops *ops =3D h->dev->ops; =20 - if (!ops->get_sset_count) { - netdev_err(netdev, "get_sset_count is null!\n"); - return -EOPNOTSUPP; - } if (stringset =3D=3D ETH_SS_TEST) { u32 cnt =3D (sizeof(hns_nic_test_strs) / ETH_GSTRING_LEN); =20 @@ -993,8 +984,10 @@ int hns_get_sset_count(struct net_device *netdev, int = stringset) cnt--; =20 return cnt; + } else if (stringset =3D=3D ETH_SS_STATS && ops->get_sset_count) { + return HNS_NET_STATS_CNT + ops->get_sset_count(h, stringset); } else { - return (HNS_NET_STATS_CNT + ops->get_sset_count(h, stringset)); + return -EOPNOTSUPP; } } =20 --1y1tiN5hVw5cPBDe Content-Type: application/pgp-signature; name="signature.asc" Content-Description: Digital signature -----BEGIN PGP SIGNATURE----- Version: GnuPG v1 iQIVAwUBWqhhROe/yOyVhhEJAQqhqRAA0pMWZTXN6+3TBW/4pEV+aDHiIeRuei3E 2l22wuUmBDrej0kcTGovpQhFJcF+ayd1s96RiZNXh1E4SFLZn0GesweHyYSRucn6 Ybd3ebVmpphQh1AnGVyyrrT92GTY4ag0ZdldqHaVMCGWOvq9iwE+N/Uki/sBbtSg VsA4YDDnhdpoD6StK+TR6u+v+rB7aiglHHy1ggyu18GSSk/BNh3oO+2sN5VUEy9T QZ3K6JKMsk7bMv21L4Fv2T8iU5oIHMFxOpWbz8XCW4cmklF4/UmJI6D82iwGfh67 qviDvxUSAVaqTNAdVB4Tn4SVPgP35iovt/iPZ2jAY2qgiv1Y3iAxWnPS8jR9ST7w +jmHvZnx+HcH+efM5er5J89R+osoIQb1h/IKwRJ/YfLJfsmC/quyyU1zi+F3EuOW COdAj7MtKY2bKtYqDSvCWa2HIMBBD1dkWEncS5s24mCZrEhtLvOZlevjxE5XJ6eq D27hLqKtOV0eREpAcN7giSmwKvC5W/A/DHwhwyjkSUrOSDKmcLD2X6Qm2b2VRu88 kgBEwMi8sQmvMevk64k+qgVErWHfWCwPg5UZs+/TbY5PyfacVtyrmOUQSNdNkM7I LhNX11Fm/KkLaRZ2B134WU62UYS3NoXQHnwMRrcEvHSrDcl8t0CKbwbZ8bGQtaLu 9ywtiI8WYEM= =ddiE -----END PGP SIGNATURE----- --1y1tiN5hVw5cPBDe--