Message ID | 20230913-strncpy-drivers-edac-edac_mc_sysfs-c-v1-1-d232891b05b0@google.com |
---|---|
State | New |
Headers |
Return-Path: <linux-kernel-owner@vger.kernel.org> Delivered-To: ouuuleilei@gmail.com Received: by 2002:a59:9ecd:0:b0:3f2:4152:657d with SMTP id t13csp922146vqx; Wed, 13 Sep 2023 01:01:39 -0700 (PDT) X-Google-Smtp-Source: AGHT+IF65Eai71X4ciLbJxoEP5+/eg15d17a3pqt4S7jUIwibrhQhlqveJZtq+t0XbOqfrnxh4Ph X-Received: by 2002:a05:6a00:21c9:b0:68f:b8ca:af2 with SMTP id t9-20020a056a0021c900b0068fb8ca0af2mr2220259pfj.13.1694592099412; Wed, 13 Sep 2023 01:01:39 -0700 (PDT) ARC-Seal: i=1; a=rsa-sha256; t=1694592099; cv=none; d=google.com; s=arc-20160816; b=ZNxClcuKkiS1F3t98VcwBHIER0ssUg1GLS6lvwXtWWu3i9ctkZLvHTmDN7eO0Z0GDJ e6bdCsemmsSCXJM+Ng8sk1QLbHu5AsvFW4ilvLAcom6vLWMHuYMy9x1Lc3IkRsA1Xmw/ JCviYLIVihtm281i8zopPvmAj56csUhKiiTk/6L1ZWkfws0TMQxpIB/BN3y8TmWRlJGt rI/g0llAQ5Rpmqt9xVG8ke68yolOr4yHYShcxbTB84SU1/y00F4LdfIjUXPGyDq3ni9P HwrJlGuNE9mIlpkjD9I6km96uIt0FXioDI6vGFju3/3w0xTeNbJJsEXcMR3wOpulgV30 eAjQ== 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=42nX+Yobaw2ArjAJMtixTW6UpiTmF6+i2Dxfn02Z0cM=; fh=B4RvtFSSsrTpdoSJeZXqdybqOBGrEMUy+eLoDD6Y0PU=; b=UZqMTU+FqW9FBP6ee/EytWvCTy/kYZ8Oe1AEjzpdpuCArHA7LE3gFe/O9v/Bx91sgX Xi7wc4LhbLZN58hBe80RTSyuWorSTvO9tonbWJ+C+6j2W50eHx/i5rUqtWG/2slPrc2H 5/vUsgmuBdEU3xdP8jx0x2MzEKN4ne8uYWlaA0iPu63/0N6pqJDftCNYbcqGZEfP934i K1cDbgXTei01jnhOC9cArmaUr+FDak0ese7kKae3CPZ5MVJ2BqUdYuGjfHOKmoyx4CHE DIAS9WmS7CNH9Lwk2015VD3LH7eYPGDTykIPtvuEuNFZ6XWsirdFCUabMWSICvryBavc 0rgQ== ARC-Authentication-Results: i=1; mx.google.com; dkim=pass header.i=@google.com header.s=20230601 header.b=dDCdEz1f; spf=pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::3:2 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 agentk.vger.email (agentk.vger.email. [2620:137:e000::3:2]) by mx.google.com with ESMTPS id x14-20020a63170e000000b0056b1f8adaeasi5060579pgl.203.2023.09.13.01.01.39 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Wed, 13 Sep 2023 01:01:39 -0700 (PDT) Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::3:2 as permitted sender) client-ip=2620:137:e000::3:2; Authentication-Results: mx.google.com; dkim=pass header.i=@google.com header.s=20230601 header.b=dDCdEz1f; spf=pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::3:2 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 agentk.vger.email (Postfix) with ESMTP id 6A9CB81A9B5E; Tue, 12 Sep 2023 18:26:32 -0700 (PDT) X-Virus-Status: Clean X-Virus-Scanned: clamav-milter 0.103.10 at agentk.vger.email Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S236634AbjIMB0a (ORCPT <rfc822;pwkd43@gmail.com> + 36 others); Tue, 12 Sep 2023 21:26:30 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:34248 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S234860AbjIMB03 (ORCPT <rfc822;linux-kernel@vger.kernel.org>); Tue, 12 Sep 2023 21:26:29 -0400 Received: from mail-yw1-x114a.google.com (mail-yw1-x114a.google.com [IPv6:2607:f8b0:4864:20::114a]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id 1CD6C10FE for <linux-kernel@vger.kernel.org>; Tue, 12 Sep 2023 18:26:25 -0700 (PDT) Received: by mail-yw1-x114a.google.com with SMTP id 00721157ae682-59224c40275so69553547b3.3 for <linux-kernel@vger.kernel.org>; Tue, 12 Sep 2023 18:26:25 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=20230601; t=1694568384; x=1695173184; darn=vger.kernel.org; h=cc:to:from:subject:message-id:mime-version:date:from:to:cc:subject :date:message-id:reply-to; bh=42nX+Yobaw2ArjAJMtixTW6UpiTmF6+i2Dxfn02Z0cM=; b=dDCdEz1fpnSHxepx83WCRH6TxueF2PrFTF8xE0PqV8lD6CcSnXFvBRYsERpkl0g+y5 ngWQ206tiRUPlkWT6ZwYSbOmXdgOuvzWugM8Xry8WnxI0OKyrfNBbY3BGucBKp4cG9Dc vhGa9/3ftdtsBH9IitvEk9II2x7gErFUE7ZKtqPU0Hx/sNCLKy8o/dmKYqrAPJXLDkZO iVEy/2RIkNQjSCpBBioGYNKsch5/23HLA8uWPcwJxnP/XJ5G6Ki+hAjyrSPmEE8JnEKZ qYzfF6ZtOh/sfllSFOSchfsW7gevP10aoUfxI/6rXENV6Pjl/EbQR8oNml9s9xtCnW+X TiNg== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1694568384; x=1695173184; h=cc:to:from:subject:message-id:mime-version:date:x-gm-message-state :from:to:cc:subject:date:message-id:reply-to; bh=42nX+Yobaw2ArjAJMtixTW6UpiTmF6+i2Dxfn02Z0cM=; b=EAIktox4Mr4IvuLWapvtyLomFnWHxns80URT/k1FLtGcOW5o1UuqJ9v2NchTP1C7ax hwNDHupl2HDpM4jGFmAXiPTQ7BHPkhDeI7LWGaHZ3xV/q0cuQSNTpoOenZsbWYasC9QR 3Q5g1d+fc946E/MaI4Aqcz6M6jADglDa2EADIzOds0zXpHPgwZiOuO4677mLnRxkH8ZO nIT4/LPpZomyHSYsI12NrR3opgz2Pp5sDC3UVZXZmLbV/UPpwC9pbrFD5Of+BPwD5v8j LIVW/A5atCGsYf4D1dCyXWUOLkrdWNHv8idkxZKc/zDIG62/6zrtJtXaLR7TuQrKyluS 91gw== X-Gm-Message-State: AOJu0YyzX2davIJnZxQ4ZZgRNr6mfYbXMUekZATApArG90jdaRaOtlAW hbhYuMYzDPhDMkbmu4lKuGEI6LdRj22jAuCSUw== X-Received: from jstitt-linux1.c.googlers.com ([fda3:e722:ac3:cc00:2b:ff92:c0a8:23b5]) (user=justinstitt job=sendgmr) by 2002:a05:6902:a06:b0:d80:bea:cbb4 with SMTP id cb6-20020a0569020a0600b00d800beacbb4mr23981ybb.5.1694568384316; Tue, 12 Sep 2023 18:26:24 -0700 (PDT) Date: Wed, 13 Sep 2023 01:26:23 +0000 Mime-Version: 1.0 X-B4-Tracking: v=1; b=H4sIAL4PAWUC/x3NywqDMBCF4VeRWTuQixT0VUqRNJm0s2iUGZGK+ O6GbA58m/OfoCRMClN3gtDOykupsH0H8RvKh5BTNTjjvBmtR92kxPXAJLyTKFIKsc38i7MemhW rH3Z8G2PdEDzUq1Uo879lnq/rugHkG0Y5dgAAAA== X-Developer-Key: i=justinstitt@google.com; a=ed25519; pk=tC3hNkJQTpNX/gLKxTNQKDmiQl6QjBNCGKJINqAdJsE= X-Developer-Signature: v=1; a=ed25519-sha256; t=1694568383; l=1656; i=justinstitt@google.com; s=20230717; h=from:subject:message-id; bh=a+gdXE2iWidU2yJZiH3fOKoz8Q6mDOLlN7Uv1qN4FVM=; b=Mv8MVJCcN9KBnoTF4HQ0BVotYpkN2Wjo4juaPRuNp8uPo5pEqbChN82bq4BeMkgADnBRqtVFK +R9SEb26aU+CM1gp5LAYEhYqCmLFurfQbnjWO9fMyFQtOFccAepTpld X-Mailer: b4 0.12.3 Message-ID: <20230913-strncpy-drivers-edac-edac_mc_sysfs-c-v1-1-d232891b05b0@google.com> Subject: [PATCH] EDAC/mc_sysfs: refactor deprecated strncpy From: Justin Stitt <justinstitt@google.com> To: Borislav Petkov <bp@alien8.de>, Tony Luck <tony.luck@intel.com>, James Morse <james.morse@arm.com>, Mauro Carvalho Chehab <mchehab@kernel.org>, Robert Richter <rric@kernel.org> Cc: linux-edac@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" 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 (agentk.vger.email [0.0.0.0]); Tue, 12 Sep 2023 18:26:32 -0700 (PDT) X-Spam-Status: No, score=-8.4 required=5.0 tests=DKIMWL_WL_MED,DKIM_SIGNED, DKIM_VALID,DKIM_VALID_AU,HEADER_FROM_DIFFERENT_DOMAINS, MAILING_LIST_MULTI,SPF_HELO_NONE,SPF_PASS,USER_IN_DEF_DKIM_WL autolearn=unavailable autolearn_force=no version=3.4.6 X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on agentk.vger.email X-getmail-retrieved-from-mailbox: INBOX X-GMAIL-THRID: 1776890599821781766 X-GMAIL-MSGID: 1776908605621875684 |
Series |
EDAC/mc_sysfs: refactor deprecated strncpy
|
|
Commit Message
Justin Stitt
Sept. 13, 2023, 1:26 a.m. UTC
`strncpy` is deprecated for use on NUL-terminated destination strings [1].
We should prefer more robust and less ambiguous string interfaces.
A suitable replacement is `strscpy_pad` [2] due to the fact that it guarantees
NUL-termination on the destination buffer whilst maintaining the
NUL-padding behavior that `strncpy` provides. This may not be strictly
necessary but as I couldn't understand what this code does I wanted to
ensure that the functionality is the same.
Link: https://www.kernel.org/doc/html/latest/process/deprecated.html#strncpy-on-nul-terminated-strings [1]
Link: https://manpages.debian.org/testing/linux-manual-4.8/strscpy.9.en.html [2]
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/edac/edac_mc_sysfs.c | 3 +--
1 file changed, 1 insertion(+), 2 deletions(-)
---
base-commit: 2dde18cd1d8fac735875f2e4987f11817cc0bc2c
change-id: 20230913-strncpy-drivers-edac-edac_mc_sysfs-c-e619b00124a3
Best regards,
--
Justin Stitt <justinstitt@google.com>
Comments
> `strncpy` is deprecated for use on NUL-terminated destination strings [1]. > > We should prefer more robust and less ambiguous string interfaces. > > A suitable replacement is `strscpy_pad` [2] due to the fact that it guarantees > NUL-termination on the destination buffer whilst maintaining the > NUL-padding behavior that `strncpy` provides. This may not be strictly > necessary but as I couldn't understand what this code does I wanted to > ensure that the functionality is the same. > > Link: https://www.kernel.org/doc/html/latest/process/deprecated.html#strncpy-on-nul-terminated-strings [1] > Link: https://manpages.debian.org/testing/linux-manual-4.8/strscpy.9.en.html [2] > 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/edac/edac_mc_sysfs.c | 3 +-- > 1 file changed, 1 insertion(+), 2 deletions(-) > > diff --git a/drivers/edac/edac_mc_sysfs.c b/drivers/edac/edac_mc_sysfs.c > index 15f63452a9be..b303309a63cf 100644 > --- a/drivers/edac/edac_mc_sysfs.c > +++ b/drivers/edac/edac_mc_sysfs.c > @@ -229,8 +229,7 @@ static ssize_t channel_dimm_label_store(struct device *dev, > if (copy_count == 0 || copy_count >= sizeof(rank->dimm->label)) > return -EINVAL; > > - strncpy(rank->dimm->label, data, copy_count); > - rank->dimm->label[copy_count] = '\0'; > + strscpy_pad(rank->dimm->label, data, copy_count); That doc page says the problem with strncpy() is that it doesn't guarantee to NUL terminate the target string. But this code is aware of that limitation and zaps a '\0' at the end to be sure. So this code doesn't suffer from the potential problems. If it is going to be fixed, then some further analysis of the original code would be wise. Just replacing with strscpy_pad() means the code probably still suffers from the "needless performance penalty" also mentioned in the deprecation document. -Tony
On Wed, Sep 13, 2023 at 8:13 AM Luck, Tony <tony.luck@intel.com> wrote: > > > `strncpy` is deprecated for use on NUL-terminated destination strings [1]. > > > > We should prefer more robust and less ambiguous string interfaces. > > > > A suitable replacement is `strscpy_pad` [2] due to the fact that it guarantees > > NUL-termination on the destination buffer whilst maintaining the > > NUL-padding behavior that `strncpy` provides. This may not be strictly > > necessary but as I couldn't understand what this code does I wanted to > > ensure that the functionality is the same. > > > > Link: https://www.kernel.org/doc/html/latest/process/deprecated.html#strncpy-on-nul-terminated-strings [1] > > Link: https://manpages.debian.org/testing/linux-manual-4.8/strscpy.9.en.html [2] > > 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/edac/edac_mc_sysfs.c | 3 +-- > > 1 file changed, 1 insertion(+), 2 deletions(-) > > > > diff --git a/drivers/edac/edac_mc_sysfs.c b/drivers/edac/edac_mc_sysfs.c > > index 15f63452a9be..b303309a63cf 100644 > > --- a/drivers/edac/edac_mc_sysfs.c > > +++ b/drivers/edac/edac_mc_sysfs.c > > @@ -229,8 +229,7 @@ static ssize_t channel_dimm_label_store(struct device *dev, > > if (copy_count == 0 || copy_count >= sizeof(rank->dimm->label)) > > return -EINVAL; > > > > - strncpy(rank->dimm->label, data, copy_count); > > - rank->dimm->label[copy_count] = '\0'; > > + strscpy_pad(rank->dimm->label, data, copy_count); > > That doc page says the problem with strncpy() is that it doesn't guarantee to > NUL terminate the target string. But this code is aware of that limitation and > zaps a '\0' at the end to be sure. > > So this code doesn't suffer from the potential problems. Right, the original code did not have an existing bug due to the reason you mentioned. However, I'm pretty keen on eliminating uses of this interface treewide as there is always a more robust and less ambiguous option. > > If it is going to be fixed, then some further analysis of the original code > would be wise. Just replacing with strscpy_pad() means the code probably > still suffers from the "needless performance penalty" also mentioned in > the deprecation document. Got it, sending a v2 that prefers `strscpy` to `strscpy_pad` resolving the performance issue. > > -Tony > Thanks for the timely review! Justin
diff --git a/drivers/edac/edac_mc_sysfs.c b/drivers/edac/edac_mc_sysfs.c index 15f63452a9be..b303309a63cf 100644 --- a/drivers/edac/edac_mc_sysfs.c +++ b/drivers/edac/edac_mc_sysfs.c @@ -229,8 +229,7 @@ static ssize_t channel_dimm_label_store(struct device *dev, if (copy_count == 0 || copy_count >= sizeof(rank->dimm->label)) return -EINVAL; - strncpy(rank->dimm->label, data, copy_count); - rank->dimm->label[copy_count] = '\0'; + strscpy_pad(rank->dimm->label, data, copy_count); return count; }