From patchwork Thu Oct 26 01:47:32 2023 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Justin Stitt X-Patchwork-Id: 158351 Return-Path: Delivered-To: ouuuleilei@gmail.com Received: by 2002:a59:d641:0:b0:403:3b70:6f57 with SMTP id cy1csp362966vqb; Wed, 25 Oct 2023 18:48:01 -0700 (PDT) X-Google-Smtp-Source: AGHT+IFQLjtD3Hr4+5/76OC+5RfE7al75Rh3pPDEDoOdrveORfpFAsk2yZeljaWpFfUHIbro6c5m X-Received: by 2002:a05:6902:188f:b0:d9a:c946:4187 with SMTP id cj15-20020a056902188f00b00d9ac9464187mr19662134ybb.22.1698284881179; Wed, 25 Oct 2023 18:48:01 -0700 (PDT) ARC-Seal: i=1; a=rsa-sha256; t=1698284881; cv=none; d=google.com; s=arc-20160816; b=OrXTKd/pH1EJPNRMsMgZq+R+j/8wXK7FCUqXgbhuw85UsBENH12caJXWWYmUU8gw4M vX6iH9FP3dvEiZTNxLdS+gK1EAJbfd3IUYZQW0pHpywvETqSv1xM0can5ij5tl0I2Rsb tPNYK9dWCT7yP726AybkjlPsc+deq7iIkalggHvJZD+coju5JDcKRftRsRLZg5A+ALba CI2QNeAg17FcimG2R6tzxkq4EbtbhxncJISirdJC7Ba/zfgrnonXzsRlCStKSuiJLCdV 1fyeCgOvL238Hurn7ALCeJ1k9vh9iVQxNtdnRJGjtim7ryt6/a1qGSrq1Nu6Aqqow2Ch 0a3A== 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=Js4XprMdTI6hSlDAn2ukBGBbLII8ZoL1rfyHAKO62Tw=; fh=VEQsqYnGSi5U5n8vQlJCmRoejLYl+Epe+h7zhNcInzw=; b=cIgsPeWaYZksEcOktcof7T5PTWTI5HN/r1dAUBK6Jnp7n0j8NXXetc2HHNqnmMNmUe 9vkgUzKP8gnltopF5EgJkZ0VF8SQ8Cm471A18UULc8KbQdhUMov4mjBLQOifz8MQ5p+g zJ87dYP4sIB8Fi+7fRQ8P8kKJMDC7rgn2OIXvxrO3ymkzG/V3AwWaBxeBGnE+7FzLHkq x7vHkZUqzODhchpfaSFz8AGzgO36Wagg5/qX8QPMbOmpQjMYmf9OyfFhl9Yd/EVvs61B u2RiN3LH/NRVCJzuVuUGcM7+h/7zkNnBepfHlAW/iuBisr0kpJoKNkf0kUyi1fZH99v4 Uvhg== ARC-Authentication-Results: i=1; mx.google.com; dkim=pass header.i=@google.com header.s=20230601 header.b=gMbUAE6V; spf=pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::3:8 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 fry.vger.email (fry.vger.email. [2620:137:e000::3:8]) by mx.google.com with ESMTPS id s5-20020a257705000000b00d8164e89b14si13281526ybc.303.2023.10.25.18.48.00 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Wed, 25 Oct 2023 18:48:01 -0700 (PDT) Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::3:8 as permitted sender) client-ip=2620:137:e000::3:8; Authentication-Results: mx.google.com; dkim=pass header.i=@google.com header.s=20230601 header.b=gMbUAE6V; spf=pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::3:8 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 fry.vger.email (Postfix) with ESMTP id 8F5FA81FD9EE; Wed, 25 Oct 2023 18:47:58 -0700 (PDT) X-Virus-Status: Clean X-Virus-Scanned: clamav-milter 0.103.10 at fry.vger.email Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S229748AbjJZBrk (ORCPT + 25 others); Wed, 25 Oct 2023 21:47:40 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:40902 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S229932AbjJZBrh (ORCPT ); Wed, 25 Oct 2023 21:47:37 -0400 Received: from mail-yb1-xb4a.google.com (mail-yb1-xb4a.google.com [IPv6:2607:f8b0:4864:20::b4a]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id 87A27CE for ; Wed, 25 Oct 2023 18:47:34 -0700 (PDT) Received: by mail-yb1-xb4a.google.com with SMTP id 3f1490d57ef6-d81e9981ff4so310538276.3 for ; Wed, 25 Oct 2023 18:47:34 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=20230601; t=1698284854; x=1698889654; darn=vger.kernel.org; h=cc:to:from:subject:message-id:mime-version:date:from:to:cc:subject :date:message-id:reply-to; bh=Js4XprMdTI6hSlDAn2ukBGBbLII8ZoL1rfyHAKO62Tw=; b=gMbUAE6VJ9LRevEU2cEMY9Ai+3kRndvKKTV7f9LaPUUaeoLQ+Fei03yU1jF4uGxN9m F7bHh0R32hd81V4x5X+OuvqqPhZ2LF3f6ehaCeXGYaxa/J28zYwgtW2IxTE28l7P59CC JJUINJT+5pMm9vtlst6uay5/xog+9ovwNObg7+BW6u9dnD8KOYzspK0BiUvgD3k3oWog ueW7RwJYUI28ExiGVHqXqXEyTlP5+n/v8QU0zWKrxj1/wMUkURRfe/ek+Kr4DdqN77fu zMePyouhQluf50aQKODKpVwp8M96MQin7OYq95JV7CTbln2xouuZ0ad6pJXaaJpRloDY cRxg== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1698284854; x=1698889654; h=cc:to:from:subject:message-id:mime-version:date:x-gm-message-state :from:to:cc:subject:date:message-id:reply-to; bh=Js4XprMdTI6hSlDAn2ukBGBbLII8ZoL1rfyHAKO62Tw=; b=gWtFLt231r2lei1LkcaV0wUvxRdRbuOEZCEhcPBFxUUx0YuTrHVgOlvvxKwRMZevDY GxxeneervvYbSR1muwqvLaPlLO/71xW0eYB+Om8aed8W6JL+eoMk0z3EwQkTuyG+clo/ JH0uUsQlza1Ck2YFJrMsWNixFbEYUMyyXdBK5n6irYCjfmpEuRAybhqEWqTtXON9lUSy L9rCfw81qDz1dCH6K1sckzZA+a5McIUoAHHDw7vBh0LvgpIs4uhzZqryUR5vw2jsBvXw bf8Exjxv56ipI2ruETKr5LTUMrKLYF+RoNky2m1yjrIT2FuWgFtBtRPABKPEUF9YCzkS CaMA== X-Gm-Message-State: AOJu0YwfZhFmLLLxQcBtwJ1OOLBSbGZdP3VVHe5AawqU933HxmrcIq/4 BfRKy/vii8Urhi1N/VF7IpGRxt+21pg+Is4Tig== X-Received: from jstitt-linux1.c.googlers.com ([fda3:e722:ac3:cc00:2b:ff92:c0a8:23b5]) (user=justinstitt job=sendgmr) by 2002:a25:243:0:b0:d9a:5e8f:1562 with SMTP id 64-20020a250243000000b00d9a5e8f1562mr316938ybc.6.1698284853737; Wed, 25 Oct 2023 18:47:33 -0700 (PDT) Date: Thu, 26 Oct 2023 01:47:32 +0000 Mime-Version: 1.0 X-B4-Tracking: v=1; b=H4sIADTFOWUC/x3MQQqEMAwAwK9Izga0ysr6FfHQpnHNpZZERBH/b tnjXOYGYxU2GKsblA8x2VJBW1dAq08/RonF4BrXtY37oO2aKF8YVQ5WQyMTXLN5JOwpDCH2/A3 DAmXIyouc/32an+cFmqrP/20AAAA= X-Developer-Key: i=justinstitt@google.com; a=ed25519; pk=tC3hNkJQTpNX/gLKxTNQKDmiQl6QjBNCGKJINqAdJsE= X-Developer-Signature: v=1; a=ed25519-sha256; t=1698284852; l=5190; i=justinstitt@google.com; s=20230717; h=from:subject:message-id; bh=AKejiU2rH/fybS/397SBwIybgBTb+KL0HploXuRamMk=; b=tA3DghjZOpNWy8m2pqqzyD5dpHovEGZXzhUoYwF0HmXrNd6rndmettjevOZlgbejRYFU2U60l m/ukhhYipVaAbeMBjkysS1X1e+HWqFD14bCU1Jhm3unSKXjRn5hhdpT X-Mailer: b4 0.12.3 Message-ID: <20231026-strncpy-drivers-scsi-hpsa-c-v1-1-75519d7a191b@google.com> Subject: [PATCH] scsi: hpsa: replace deprecated strncpy with strscpy/kmemdup_nul From: Justin Stitt To: Don Brace , "James E.J. Bottomley" , "Martin K. Petersen" Cc: storagedev@microchip.com, linux-scsi@vger.kernel.org, linux-kernel@vger.kernel.org, linux-hardening@vger.kernel.org, Kees Cook , Justin Stitt 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 fry.vger.email Precedence: bulk List-ID: X-Mailing-List: linux-kernel@vger.kernel.org X-Greylist: Sender passed SPF test, not delayed by milter-greylist-4.6.4 (fry.vger.email [0.0.0.0]); Wed, 25 Oct 2023 18:47:58 -0700 (PDT) X-getmail-retrieved-from-mailbox: INBOX X-GMAIL-THRID: 1780780767573382347 X-GMAIL-MSGID: 1780780767573382347 strncpy() is deprecated for use on NUL-terminated destination strings [1] and as such we should prefer more robust and less ambiguous string interfaces. This whole process of 1) determining smaller length so we don't overread the buffer and 2) manually NUL-terminating our buffer so we can use in string APIs is handled implicitly by strscpy(). Therefore, a suitable replacement is `strscpy` [2] due to the fact that it guarantees NUL-termination on the destination buffer without unnecessarily NUL-padding. For the last two strncpy() use cases in init_driver_version(), we can actually drop this function entirely. Firstly, we are kmalloc()'ing driver_version. Then, we are calling init_driver_version() which memset's it to 0 followed by a strncpy(). This pattern of 1) allocating memory for a string, 2) setting all bytes to NUL, 3) copy bytes from another string + ensure NUL-padded destination is just an open-coded kmemdup_nul(). The last case involves swapping kmalloc_array() for kcalloc() to give us a zero-filled two-element array for both old_driver_version and driver_version without needing the memset from init_driver_version(). Now this code is easier to read and less fragile (no more ... - 1's) or min length checks and now we have guaranteed NUL-termination everywhere! Although perhaps there should be a macro for: 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 Cc: Kees Cook Signed-off-by: Justin Stitt --- Note: build-tested only. Found with: $ rg "strncpy\(" --- drivers/scsi/hpsa.c | 29 +++++++++++------------------ 1 file changed, 11 insertions(+), 18 deletions(-) --- base-commit: d88520ad73b79e71e3ddf08de335b8520ae41c5c change-id: 20231026-strncpy-drivers-scsi-hpsa-c-4cb7bd4e9b7f Best regards, -- Justin Stitt diff --git a/drivers/scsi/hpsa.c b/drivers/scsi/hpsa.c index af18d20f3079..3376d4614fe5 100644 --- a/drivers/scsi/hpsa.c +++ b/drivers/scsi/hpsa.c @@ -452,16 +452,15 @@ static ssize_t host_store_hp_ssd_smart_path_status(struct device *dev, struct device_attribute *attr, const char *buf, size_t count) { - int status, len; + int status; struct ctlr_info *h; struct Scsi_Host *shost = class_to_shost(dev); char tmpbuf[10]; if (!capable(CAP_SYS_ADMIN) || !capable(CAP_SYS_RAWIO)) return -EACCES; - len = count > sizeof(tmpbuf) - 1 ? sizeof(tmpbuf) - 1 : count; - strncpy(tmpbuf, buf, len); - tmpbuf[len] = '\0'; + strscpy(tmpbuf, buf, count); + if (sscanf(tmpbuf, "%d", &status) != 1) return -EINVAL; h = shost_to_hba(shost); @@ -476,16 +475,15 @@ static ssize_t host_store_raid_offload_debug(struct device *dev, struct device_attribute *attr, const char *buf, size_t count) { - int debug_level, len; + int debug_level; struct ctlr_info *h; struct Scsi_Host *shost = class_to_shost(dev); char tmpbuf[10]; if (!capable(CAP_SYS_ADMIN) || !capable(CAP_SYS_RAWIO)) return -EACCES; - len = count > sizeof(tmpbuf) - 1 ? sizeof(tmpbuf) - 1 : count; - strncpy(tmpbuf, buf, len); - tmpbuf[len] = '\0'; + strscpy(tmpbuf, buf, count); + if (sscanf(tmpbuf, "%d", &debug_level) != 1) return -EINVAL; if (debug_level < 0) @@ -7234,24 +7232,19 @@ static int hpsa_controller_hard_reset(struct pci_dev *pdev, return 0; } -static void init_driver_version(char *driver_version, int len) -{ - memset(driver_version, 0, len); - strncpy(driver_version, HPSA " " HPSA_DRIVER_VERSION, len - 1); -} - static int write_driver_ver_to_cfgtable(struct CfgTable __iomem *cfgtable) { char *driver_version; int i, size = sizeof(cfgtable->driver_version); - driver_version = kmalloc(size, GFP_KERNEL); + driver_version = kmemdup_nul(HPSA " " HPSA_DRIVER_VERSION, size, + GFP_KERNEL); if (!driver_version) return -ENOMEM; - init_driver_version(driver_version, size); for (i = 0; i < size; i++) writeb(driver_version[i], &cfgtable->driver_version[i]); + kfree(driver_version); return 0; } @@ -7271,7 +7264,7 @@ static int controller_reset_failed(struct CfgTable __iomem *cfgtable) char *driver_ver, *old_driver_ver; int rc, size = sizeof(cfgtable->driver_version); - old_driver_ver = kmalloc_array(2, size, GFP_KERNEL); + old_driver_ver = kcalloc(2, size, GFP_KERNEL); if (!old_driver_ver) return -ENOMEM; driver_ver = old_driver_ver + size; @@ -7279,7 +7272,7 @@ static int controller_reset_failed(struct CfgTable __iomem *cfgtable) /* After a reset, the 32 bytes of "driver version" in the cfgtable * should have been changed, otherwise we know the reset failed. */ - init_driver_version(old_driver_ver, size); + strscpy(old_driver_ver, HPSA " " HPSA_DRIVER_VERSION, size); read_driver_ver_from_cfgtable(cfgtable, driver_ver); rc = !memcmp(driver_ver, old_driver_ver, size); kfree(old_driver_ver);