Message ID | 20231005-strncpy-drivers-net-dsa-lan9303-core-c-v1-1-5a66c538147e@google.com |
---|---|
State | New |
Headers |
Return-Path: <linux-kernel-owner@vger.kernel.org> Delivered-To: ouuuleilei@gmail.com Received: by 2002:a05:612c:254a:b0:403:3b70:6f57 with SMTP id hf10csp489215vqb; Wed, 4 Oct 2023 17:31:03 -0700 (PDT) X-Google-Smtp-Source: AGHT+IGSDoJlBYoxZOC2ZzjB8cJlnVce+Zzh4ClpFbWpH6O+TberVaovM72/1cCJZogXwAzfba02 X-Received: by 2002:a92:cdad:0:b0:34c:c0ad:69e0 with SMTP id g13-20020a92cdad000000b0034cc0ad69e0mr4895074ild.1.1696465862858; Wed, 04 Oct 2023 17:31:02 -0700 (PDT) ARC-Seal: i=1; a=rsa-sha256; t=1696465862; cv=none; d=google.com; s=arc-20160816; b=dbS3r9qANSmNAHNQv1azAlrN1IFzlb4eVmE+dEW6M2jqT3EVrWMyYZipaFIHY6e4FL uvOQ99FA+OxESPK3fr63+GwBOR6TMcpfegzCyTHVBhX2uEfJkxCAXFZzoDKkM0Gw/9SC 1lF590QBJ40AOO7um69ydVP6LQ4YJ/7J3BFnJA5URsUku8bKtZMEsqi3uLsXsLbc10d2 z1nkorjXKUN8D9v8cmEKrlIp0bKjEvR+addZNkDepsBqjdXvgNdOx4WqQabsPrEjg3NK FkkP6HG83W/HexIVx0262nw/6rMt/3ilURf2b5yc1UtHaDza+NhIeQaSJvAahTGVwiO8 X27A== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=arc-20160816; h=list-id:precedence:cc:to:from:subject:message-id:mime-version:date :dkim-signature; bh=z9CoIYBwy9paxLnmDstL0tKAJy9h7YqHeLlFxLDJHMU=; fh=+nR/KBYVMP75b/5EkORvVk3R3PpmbJKGYQ7gn75EMlo=; b=AkJ8Ta1jSyXJYdjUTg9CJI+wYbXfObji8Ni/4MDVfSZawvcGpwhJMMZerjJ5sBkcR2 JAArFgkPdhfmMi2v1YPf/XmEi5aBwGvX0m+5IXGHD3rYbr0YYcH0hSGGWXeK/opZFkSn yEAXYkg/8rxpw8fm4i3H+uthCtu5XFp1sL4nflV66AV3es6SheoM8RihjOVveEtVpzTj OOR5vFlwG0tWa8RsgxLr63MKeKyBWCobdX1fAOncrSx8Mwsiz+wbY+G5DnFArd9ycA7e +wzsQnKI5SAjDOTIZCuPcswh6qM4B2IttTGfT8fpPAK00Q13+IWO8aFhB5XS/yKRn5nZ rFLA== ARC-Authentication-Results: i=1; mx.google.com; dkim=pass header.i=@google.com header.s=20230601 header.b=JEP1Q4OD; spf=pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.34 as permitted sender) smtp.mailfrom=linux-kernel-owner@vger.kernel.org; dmarc=pass (p=REJECT sp=REJECT dis=NONE) header.from=google.com Received: from howler.vger.email (howler.vger.email. [23.128.96.34]) by mx.google.com with ESMTPS id cm4-20020a056a020a0400b005775e2f160fsi248262pgb.667.2023.10.04.17.31.02 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Wed, 04 Oct 2023 17:31:02 -0700 (PDT) Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.34 as permitted sender) client-ip=23.128.96.34; Authentication-Results: mx.google.com; dkim=pass header.i=@google.com header.s=20230601 header.b=JEP1Q4OD; spf=pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.34 as permitted sender) smtp.mailfrom=linux-kernel-owner@vger.kernel.org; dmarc=pass (p=REJECT sp=REJECT dis=NONE) header.from=google.com Received: from out1.vger.email (depot.vger.email [IPv6:2620:137:e000::3:0]) by howler.vger.email (Postfix) with ESMTP id 5DA28834F752; Wed, 4 Oct 2023 17:30:48 -0700 (PDT) X-Virus-Status: Clean X-Virus-Scanned: clamav-milter 0.103.10 at howler.vger.email Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S233943AbjJEAa1 (ORCPT <rfc822;ezelljr.billy@gmail.com> + 19 others); Wed, 4 Oct 2023 20:30:27 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:35070 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S239117AbjJEAaY (ORCPT <rfc822;linux-kernel@vger.kernel.org>); Wed, 4 Oct 2023 20:30:24 -0400 Received: from mail-yw1-x1149.google.com (mail-yw1-x1149.google.com [IPv6:2607:f8b0:4864:20::1149]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id 2D527C1 for <linux-kernel@vger.kernel.org>; Wed, 4 Oct 2023 17:30:20 -0700 (PDT) Received: by mail-yw1-x1149.google.com with SMTP id 00721157ae682-5a1f12cf1ddso13067707b3.0 for <linux-kernel@vger.kernel.org>; Wed, 04 Oct 2023 17:30:20 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=20230601; t=1696465819; x=1697070619; darn=vger.kernel.org; h=cc:to:from:subject:message-id:mime-version:date:from:to:cc:subject :date:message-id:reply-to; bh=z9CoIYBwy9paxLnmDstL0tKAJy9h7YqHeLlFxLDJHMU=; b=JEP1Q4ODGE5fcLMCkUqZ0c1QuktvoQvuYVTsfhSiG4KidVawy7KKL3FptUsFCE77XV kuNwTnsXqWoW4IxZzhw8DfIAUxRNU6XY4pzURg40OSBI2GSBXq/xVY1BieRJ2pMTC12h MGZN4X6zTNgaq1NEpdrJzLhLe0iIQ/DD3hR0HJnCDm2l2rL0RdodaOWkBKlPcNMsrVzs Fv8EoVVKKkeht2Ei+cmOUKLaq6GO0J1Rtg6nRYJRG42CjKp5kvlLQtSfdZ1WQJ2E0Zr1 B8XJxHCToyJ7wN6qNNs0eHNdC+al1kHx54iHfvtBCMSLdA5a71FhrD7JHTQTWYcJA9Of rFzg== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1696465819; x=1697070619; h=cc:to:from:subject:message-id:mime-version:date:x-gm-message-state :from:to:cc:subject:date:message-id:reply-to; bh=z9CoIYBwy9paxLnmDstL0tKAJy9h7YqHeLlFxLDJHMU=; b=Ij6RKJ1H9ihyozSZWZXbhmEBtUxh8aYh6r6wc4fSqR6a6Yy9MxubsUPK0UReA6I2AX O6xowtu4RuvWSoExsvLALpPgIZP+M58J8to2jOuTjkH9CitLjY/u6gbCwkhX0o6ZuCFO CnVuLZrmfAW2PfqKSvnJQIt6c5dA+Kh7XdtY5nWyVOnXjZ26woVXHQFzGlCfM9jXfylS KqiaDLHSd7ul0BOZnlfmlCyT7fY4KUSTkYqchELDf17YIwBBtaZooE2SMtck2VZK8G6r Xpaja1DobKWdPR2q8hVxnUctHlAEaOfmO82lbeklWsKP5EK31w9ZObddYYBlM1OocZ5+ uoow== X-Gm-Message-State: AOJu0YwQ7WsTghfgOk4FHwrqr9zSWw9ROTX3WIEA/KmPv180zg9kHA00 zaBqd+9intQEQ2958lG1dP4+HE17BpaUSwHT5A== X-Received: from jstitt-linux1.c.googlers.com ([fda3:e722:ac3:cc00:2b:ff92:c0a8:23b5]) (user=justinstitt job=sendgmr) by 2002:a05:690c:c04:b0:595:7304:68e5 with SMTP id cl4-20020a05690c0c0400b00595730468e5mr25561ywb.0.1696465819306; Wed, 04 Oct 2023 17:30:19 -0700 (PDT) Date: Thu, 05 Oct 2023 00:30:18 +0000 Mime-Version: 1.0 X-B4-Tracking: v=1; b=H4sIAJkDHmUC/x3NwQrCMAyA4VcZORvoWjqqryIeSho1INlIxpiMv bvF43f5/wOcTdjhNhxgvInLrB3jZQB6V30xSuuGGGIaQ8joqyktX2wmG5uj8orNK36qXlNISLM xEk6pTCUXzhQj9Nhi/JT9P7o/zvMHaoGrR3gAAAA= X-Developer-Key: i=justinstitt@google.com; a=ed25519; pk=tC3hNkJQTpNX/gLKxTNQKDmiQl6QjBNCGKJINqAdJsE= X-Developer-Signature: v=1; a=ed25519-sha256; t=1696465818; l=1658; i=justinstitt@google.com; s=20230717; h=from:subject:message-id; bh=bsKNbLzu3Ry5CEVwYR5pkDz3ZbDhkIowy3jtYMWC4UU=; b=ibUfNayaIATafgTAMuJ6HX3fgX+SPnajRCu3PTyOpaKrLLvonFUzzZJj5z8G4VBo1QBRXRPfx GxtwDu8o5ulDTLISW/SBzvFSNNvL/dlNf+TQdK8bXC5sCzPDKsDoyae X-Mailer: b4 0.12.3 Message-ID: <20231005-strncpy-drivers-net-dsa-lan9303-core-c-v1-1-5a66c538147e@google.com> Subject: [PATCH] net: dsa: lan9303: replace deprecated strncpy with memcpy From: Justin Stitt <justinstitt@google.com> To: Andrew Lunn <andrew@lunn.ch>, Florian Fainelli <f.fainelli@gmail.com>, Vladimir Oltean <olteanv@gmail.com>, "David S. Miller" <davem@davemloft.net>, Eric Dumazet <edumazet@google.com>, Jakub Kicinski <kuba@kernel.org>, Paolo Abeni <pabeni@redhat.com> Cc: netdev@vger.kernel.org, linux-kernel@vger.kernel.org, linux-hardening@vger.kernel.org, Justin Stitt <justinstitt@google.com> Content-Type: text/plain; charset="utf-8" X-Spam-Status: No, score=-9.6 required=5.0 tests=BAYES_00,DKIMWL_WL_MED, DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,DKIM_VALID_EF, RCVD_IN_DNSWL_BLOCKED,SPF_HELO_NONE,SPF_PASS,URIBL_BLOCKED, USER_IN_DEF_DKIM_WL autolearn=ham autolearn_force=no version=3.4.6 X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on lindbergh.monkeyblade.net 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 (howler.vger.email [0.0.0.0]); Wed, 04 Oct 2023 17:30:48 -0700 (PDT) X-getmail-retrieved-from-mailbox: INBOX X-GMAIL-THRID: 1778873388911141842 X-GMAIL-MSGID: 1778873388911141842 |
Series |
net: dsa: lan9303: replace deprecated strncpy with memcpy
|
|
Commit Message
Justin Stitt
Oct. 5, 2023, 12:30 a.m. UTC
`strncpy` is deprecated for use on NUL-terminated destination strings
[1] and as such we should prefer more robust and less ambiguous
interfaces.
Let's opt for memcpy as we are copying strings into slices of length
`ETH_GSTRING_LEN` within the `data` buffer. Other similar get_strings()
implementations [2] [3] use memcpy().
Link: https://www.kernel.org/doc/html/latest/process/deprecated.html#strncpy-on-nul-terminated-strings [1]
Link: https://elixir.bootlin.com/linux/v6.3/source/drivers/infiniband/ulp/opa_vnic/opa_vnic_ethtool.c#L167 [2]
Link: https://elixir.bootlin.com/linux/v6.3/source/drivers/infiniband/ulp/ipoib/ipoib_ethtool.c#L137 [3]
Link: https://github.com/KSPP/linux/issues/90
Cc: linux-hardening@vger.kernel.org
Signed-off-by: Justin Stitt <justinstitt@google.com>
---
Note: build-tested only.
---
drivers/net/dsa/lan9303-core.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
---
base-commit: cbf3a2cb156a2c911d8f38d8247814b4c07f49a2
change-id: 20231005-strncpy-drivers-net-dsa-lan9303-core-c-6386858e5c22
Best regards,
--
Justin Stitt <justinstitt@google.com>
Comments
On Thu, Oct 05, 2023 at 12:30:18AM +0000, Justin Stitt wrote: > `strncpy` is deprecated for use on NUL-terminated destination strings > [1] and as such we should prefer more robust and less ambiguous > interfaces. > > Let's opt for memcpy as we are copying strings into slices of length > `ETH_GSTRING_LEN` within the `data` buffer. Other similar get_strings() > implementations [2] [3] use memcpy(). > > Link: https://www.kernel.org/doc/html/latest/process/deprecated.html#strncpy-on-nul-terminated-strings [1] > Link: https://elixir.bootlin.com/linux/v6.3/source/drivers/infiniband/ulp/opa_vnic/opa_vnic_ethtool.c#L167 [2] > Link: https://elixir.bootlin.com/linux/v6.3/source/drivers/infiniband/ulp/ipoib/ipoib_ethtool.c#L137 [3] > Link: https://github.com/KSPP/linux/issues/90 > Cc: linux-hardening@vger.kernel.org > Signed-off-by: Justin Stitt <justinstitt@google.com> > --- > Note: build-tested only. > --- > drivers/net/dsa/lan9303-core.c | 4 ++-- > 1 file changed, 2 insertions(+), 2 deletions(-) > > diff --git a/drivers/net/dsa/lan9303-core.c b/drivers/net/dsa/lan9303-core.c > index ee67adeb2cdb..665d69384b62 100644 > --- a/drivers/net/dsa/lan9303-core.c > +++ b/drivers/net/dsa/lan9303-core.c > @@ -1013,8 +1013,8 @@ static void lan9303_get_strings(struct dsa_switch *ds, int port, > return; > > for (u = 0; u < ARRAY_SIZE(lan9303_mib); u++) { > - strncpy(data + u * ETH_GSTRING_LEN, lan9303_mib[u].name, > - ETH_GSTRING_LEN); > + memcpy(data + u * ETH_GSTRING_LEN, lan9303_mib[u].name, > + ETH_GSTRING_LEN); This won't work because lan9303_mib entries aren't ETH_GSTRING_LEN-long strings; they're string pointers: static const struct lan9303_mib_desc lan9303_mib[] = { { .offset = LAN9303_MAC_RX_BRDCST_CNT_0, .name = "RxBroad", }, So this really does need a strcpy-family function. And, I think the vnic_gstrings_stats and ipoib_gstrings_stats examples are actually buggy -- they're copying junk into userspace... I am reminded of this patch, which correctly uses strscpy_pad(): https://lore.kernel.org/lkml/20230718-net-dsa-strncpy-v1-1-e84664747713@google.com/ I think you want to do the same here, and use strscpy_pad(). And perhaps send some fixes for the other memcpy() users? (Have I mentioned I really dislike the get_strings() API?) -Kees
On Wed, Oct 04, 2023 at 08:07:55PM -0700, Kees Cook wrote: > On Thu, Oct 05, 2023 at 12:30:18AM +0000, Justin Stitt wrote: > > `strncpy` is deprecated for use on NUL-terminated destination strings > > [1] and as such we should prefer more robust and less ambiguous > > interfaces. > > > > Let's opt for memcpy as we are copying strings into slices of length > > `ETH_GSTRING_LEN` within the `data` buffer. Other similar get_strings() > > implementations [2] [3] use memcpy(). > > > > Link: https://www.kernel.org/doc/html/latest/process/deprecated.html#strncpy-on-nul-terminated-strings [1] > > Link: https://elixir.bootlin.com/linux/v6.3/source/drivers/infiniband/ulp/opa_vnic/opa_vnic_ethtool.c#L167 [2] > > Link: https://elixir.bootlin.com/linux/v6.3/source/drivers/infiniband/ulp/ipoib/ipoib_ethtool.c#L137 [3] > > Link: https://github.com/KSPP/linux/issues/90 > > Cc: linux-hardening@vger.kernel.org > > Signed-off-by: Justin Stitt <justinstitt@google.com> > > --- > > Note: build-tested only. > > --- > > drivers/net/dsa/lan9303-core.c | 4 ++-- > > 1 file changed, 2 insertions(+), 2 deletions(-) > > > > diff --git a/drivers/net/dsa/lan9303-core.c b/drivers/net/dsa/lan9303-core.c > > index ee67adeb2cdb..665d69384b62 100644 > > --- a/drivers/net/dsa/lan9303-core.c > > +++ b/drivers/net/dsa/lan9303-core.c > > @@ -1013,8 +1013,8 @@ static void lan9303_get_strings(struct dsa_switch *ds, int port, > > return; > > > > for (u = 0; u < ARRAY_SIZE(lan9303_mib); u++) { > > - strncpy(data + u * ETH_GSTRING_LEN, lan9303_mib[u].name, > > - ETH_GSTRING_LEN); > > + memcpy(data + u * ETH_GSTRING_LEN, lan9303_mib[u].name, > > + ETH_GSTRING_LEN); > > This won't work because lan9303_mib entries aren't ETH_GSTRING_LEN-long > strings; they're string pointers: > > static const struct lan9303_mib_desc lan9303_mib[] = { > { .offset = LAN9303_MAC_RX_BRDCST_CNT_0, .name = "RxBroad", }, > > So this really does need a strcpy-family function. > > And, I think the vnic_gstrings_stats and ipoib_gstrings_stats examples > are actually buggy -- they're copying junk into userspace... > > I am reminded of this patch, which correctly uses strscpy_pad(): > https://lore.kernel.org/lkml/20230718-net-dsa-strncpy-v1-1-e84664747713@google.com/ > > I think you want to do the same here, and use strscpy_pad(). And perhaps > send some fixes for the other memcpy() users? Meh, I think it's not worth fixing the memcpy() users of this. This buggy pattern is very common, it seems: $ git grep 'data.*ETH_GSTRING_LEN' | grep memcpy | wc -l 47
From: Kees Cook <keescook@chromium.org> Date: Wed, 4 Oct 2023 22:02:00 -0700 > On Wed, Oct 04, 2023 at 08:07:55PM -0700, Kees Cook wrote: >> On Thu, Oct 05, 2023 at 12:30:18AM +0000, Justin Stitt wrote: >>> `strncpy` is deprecated for use on NUL-terminated destination strings >>> [1] and as such we should prefer more robust and less ambiguous >>> interfaces. >>> >>> Let's opt for memcpy as we are copying strings into slices of length >>> `ETH_GSTRING_LEN` within the `data` buffer. Other similar get_strings() >>> implementations [2] [3] use memcpy(). >>> >>> Link: https://www.kernel.org/doc/html/latest/process/deprecated.html#strncpy-on-nul-terminated-strings [1] >>> Link: https://elixir.bootlin.com/linux/v6.3/source/drivers/infiniband/ulp/opa_vnic/opa_vnic_ethtool.c#L167 [2] >>> Link: https://elixir.bootlin.com/linux/v6.3/source/drivers/infiniband/ulp/ipoib/ipoib_ethtool.c#L137 [3] >>> Link: https://github.com/KSPP/linux/issues/90 >>> Cc: linux-hardening@vger.kernel.org >>> Signed-off-by: Justin Stitt <justinstitt@google.com> >>> --- >>> Note: build-tested only. >>> --- >>> drivers/net/dsa/lan9303-core.c | 4 ++-- >>> 1 file changed, 2 insertions(+), 2 deletions(-) >>> >>> diff --git a/drivers/net/dsa/lan9303-core.c b/drivers/net/dsa/lan9303-core.c >>> index ee67adeb2cdb..665d69384b62 100644 >>> --- a/drivers/net/dsa/lan9303-core.c >>> +++ b/drivers/net/dsa/lan9303-core.c >>> @@ -1013,8 +1013,8 @@ static void lan9303_get_strings(struct dsa_switch *ds, int port, >>> return; >>> >>> for (u = 0; u < ARRAY_SIZE(lan9303_mib); u++) { >>> - strncpy(data + u * ETH_GSTRING_LEN, lan9303_mib[u].name, >>> - ETH_GSTRING_LEN); >>> + memcpy(data + u * ETH_GSTRING_LEN, lan9303_mib[u].name, >>> + ETH_GSTRING_LEN); >> >> This won't work because lan9303_mib entries aren't ETH_GSTRING_LEN-long >> strings; they're string pointers: >> >> static const struct lan9303_mib_desc lan9303_mib[] = { >> { .offset = LAN9303_MAC_RX_BRDCST_CNT_0, .name = "RxBroad", }, >> >> So this really does need a strcpy-family function. >> >> And, I think the vnic_gstrings_stats and ipoib_gstrings_stats examples >> are actually buggy -- they're copying junk into userspace... >> >> I am reminded of this patch, which correctly uses strscpy_pad(): >> https://lore.kernel.org/lkml/20230718-net-dsa-strncpy-v1-1-e84664747713@google.com/ >> >> I think you want to do the same here, and use strscpy_pad(). And perhaps >> send some fixes for the other memcpy() users? > > Meh, I think it's not worth fixing the memcpy() users of this. This > buggy pattern is very common, it seems: > > $ git grep 'data.*ETH_GSTRING_LEN' | grep memcpy | wc -l > 47 We have ethtool_sprintf() precisely for the sake of filling the Ethtool statistics names. BTW this weird pattern "let's make the array of our stats names of fixed width (ETH_GSTRING_LEN), so that we could use memcpy() instead of strcpy()" was pretty common some time ago, no idea why, as it wastes memory for tons of \0 padding and provokes issues like the one you noticed here. > Thanks, Olek
On Thu, Oct 5, 2023 at 7:54 AM Alexander Lobakin <aleksander.lobakin@intel.com> wrote: > > From: Kees Cook <keescook@chromium.org> > Date: Wed, 4 Oct 2023 22:02:00 -0700 > > > On Wed, Oct 04, 2023 at 08:07:55PM -0700, Kees Cook wrote: > >> On Thu, Oct 05, 2023 at 12:30:18AM +0000, Justin Stitt wrote: > >>> `strncpy` is deprecated for use on NUL-terminated destination strings > >>> [1] and as such we should prefer more robust and less ambiguous > >>> interfaces. > >>> > >>> Let's opt for memcpy as we are copying strings into slices of length > >>> `ETH_GSTRING_LEN` within the `data` buffer. Other similar get_strings() > >>> implementations [2] [3] use memcpy(). > >>> > >>> Link: https://www.kernel.org/doc/html/latest/process/deprecated.html#strncpy-on-nul-terminated-strings [1] > >>> Link: https://elixir.bootlin.com/linux/v6.3/source/drivers/infiniband/ulp/opa_vnic/opa_vnic_ethtool.c#L167 [2] > >>> Link: https://elixir.bootlin.com/linux/v6.3/source/drivers/infiniband/ulp/ipoib/ipoib_ethtool.c#L137 [3] > >>> Link: https://github.com/KSPP/linux/issues/90 > >>> Cc: linux-hardening@vger.kernel.org > >>> Signed-off-by: Justin Stitt <justinstitt@google.com> > >>> --- > >>> Note: build-tested only. > >>> --- > >>> drivers/net/dsa/lan9303-core.c | 4 ++-- > >>> 1 file changed, 2 insertions(+), 2 deletions(-) > >>> > >>> diff --git a/drivers/net/dsa/lan9303-core.c b/drivers/net/dsa/lan9303-core.c > >>> index ee67adeb2cdb..665d69384b62 100644 > >>> --- a/drivers/net/dsa/lan9303-core.c > >>> +++ b/drivers/net/dsa/lan9303-core.c > >>> @@ -1013,8 +1013,8 @@ static void lan9303_get_strings(struct dsa_switch *ds, int port, > >>> return; > >>> > >>> for (u = 0; u < ARRAY_SIZE(lan9303_mib); u++) { > >>> - strncpy(data + u * ETH_GSTRING_LEN, lan9303_mib[u].name, > >>> - ETH_GSTRING_LEN); > >>> + memcpy(data + u * ETH_GSTRING_LEN, lan9303_mib[u].name, > >>> + ETH_GSTRING_LEN); > >> > >> This won't work because lan9303_mib entries aren't ETH_GSTRING_LEN-long > >> strings; they're string pointers: > >> > >> static const struct lan9303_mib_desc lan9303_mib[] = { > >> { .offset = LAN9303_MAC_RX_BRDCST_CNT_0, .name = "RxBroad", }, > >> > >> So this really does need a strcpy-family function. > >> > >> And, I think the vnic_gstrings_stats and ipoib_gstrings_stats examples > >> are actually buggy -- they're copying junk into userspace... > >> > >> I am reminded of this patch, which correctly uses strscpy_pad(): > >> https://lore.kernel.org/lkml/20230718-net-dsa-strncpy-v1-1-e84664747713@google.com/ > >> > >> I think you want to do the same here, and use strscpy_pad(). And perhaps > >> send some fixes for the other memcpy() users? > > > > Meh, I think it's not worth fixing the memcpy() users of this. This > > buggy pattern is very common, it seems: > > > > $ git grep 'data.*ETH_GSTRING_LEN' | grep memcpy | wc -l > > 47 > > We have ethtool_sprintf() precisely for the sake of filling the Ethtool > statistics names. > > BTW this weird pattern "let's make the array of our stats names of fixed > width (ETH_GSTRING_LEN), so that we could use memcpy() instead of > strcpy()" was pretty common some time ago, no idea why, as it wastes > memory for tons of \0 padding and provokes issues like the one you > noticed here. Just sent a v2 using ethtool_sprintf(). I'd appreciate some feedback on if I used it correctly. > > > > > Thanks, > Olek [v2]: https://lore.kernel.org/r/20231005-strncpy-drivers-net-dsa-lan9303-core-c-v2-1-feb452a532db@google.com Thanks Justin
diff --git a/drivers/net/dsa/lan9303-core.c b/drivers/net/dsa/lan9303-core.c index ee67adeb2cdb..665d69384b62 100644 --- a/drivers/net/dsa/lan9303-core.c +++ b/drivers/net/dsa/lan9303-core.c @@ -1013,8 +1013,8 @@ static void lan9303_get_strings(struct dsa_switch *ds, int port, return; for (u = 0; u < ARRAY_SIZE(lan9303_mib); u++) { - strncpy(data + u * ETH_GSTRING_LEN, lan9303_mib[u].name, - ETH_GSTRING_LEN); + memcpy(data + u * ETH_GSTRING_LEN, lan9303_mib[u].name, + ETH_GSTRING_LEN); } }