Message ID | 20240228-strncpy-drivers-scsi-mpi3mr-mpi3mr_fw-c-v2-0-dacebd3fcfa0@google.com |
---|---|
Headers |
Return-Path: <linux-kernel+bounces-85815-ouuuleilei=gmail.com@vger.kernel.org> Delivered-To: ouuuleilei@gmail.com Received: by 2002:a05:6358:a1a:b0:17b:cd04:e0c6 with SMTP id 26csp243268rwa; Wed, 28 Feb 2024 15:00:22 -0800 (PST) X-Forwarded-Encrypted: i=3; AJvYcCUVfqj3Fc45p1bLCiSMb0uNgnO6bGmMetXlUxAPcjugsyVqaQNJFxvT0akOxMGpmogsmnbwmdppQDnOFivXZ2WCdrXDhA== X-Google-Smtp-Source: AGHT+IFBGKwnc87282QciYn3YNieqjZfsIOhBqDHp1tHocKxb8BljPPUoSFBTTsBGZc0si8Rp5Vg X-Received: by 2002:a05:6a20:ac44:b0:1a0:78b3:af33 with SMTP id dh4-20020a056a20ac4400b001a078b3af33mr727449pzb.37.1709161222377; Wed, 28 Feb 2024 15:00:22 -0800 (PST) ARC-Seal: i=2; a=rsa-sha256; t=1709161222; cv=pass; d=google.com; s=arc-20160816; b=LujEuyY9rGkGDI0X0P3dnRytTDgUR216XUkhJk8Jwn2j/V7I8j60tJvtOhhJM0d3Qu aS9JcKAxQY4xQ9tnynhWb+c/Ul4l02nNnWObSVb0YVM6jqVcyanwCoRIb5LIEPEyUha7 6Ba9t/atYy0kFr/xs9T5LkYpOFFgsuPD0rz1fRYKZrx8pLmABT0r5JOElgE7RXRl4C7Q VOVlVyE9OuIKqcicWw0ElQeIBjvkrS7A7o3OumcYdE1vDE7GXUIa8kUNavyyDaSBQ/2M F5+xSOSEd+mtLKgtUardA0yMyHbBUKJdEHS45ytNB/chVCtpuqlwl8ohIDdejWFURMtc SoWQ== ARC-Message-Signature: i=2; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=arc-20160816; h=cc:to:from:subject:message-id:mime-version:list-unsubscribe :list-subscribe:list-id:precedence:date:dkim-signature; bh=bwcBf6VTN7kSbKJ8Vs5Gx4XAVHKYHpXJ1cRKxEXuKj0=; fh=tLKXv2VTeYOcu4clBzGZR2d+PKFs2fQZY2FfiH2bhkc=; b=KMRYKvzyw29KteH8+9/w4CEYR2mwPdbp8qhh5qLrpKPhS0Wd1VocfiH76gMfNKkdEg Dwq65d5JU8CizOsheEpS9NEItC7cY7syTOg8c6qcx7c8KZrEy05+0uHAeDfp3msD10gi 6Rg4/a5jj+zkYvyup5CqwL0bZnaiHapntZbfbtq8fu3EIe68y6WidQVfqN7lGVrrqSY0 tE+QeM2sQzKOgrtiBp4v1OH6M7oNtBP9jK3JjXe1SmbHUyFvymqdx4H+3xwBbhdOzIml 7nw+Cu+DF0XQXFUtCC+lsK0mujNMqozKKymtgrI6xqaz7DJ9TpzMz6v1jj/cAdXrnHwM nsLA==; dara=google.com ARC-Authentication-Results: i=2; mx.google.com; dkim=pass header.i=@google.com header.s=20230601 header.b=iraPFeRH; arc=pass (i=1 spf=pass spfdomain=flex--justinstitt.bounces.google.com dkim=pass dkdomain=google.com dmarc=pass fromdomain=google.com); spf=pass (google.com: domain of linux-kernel+bounces-85815-ouuuleilei=gmail.com@vger.kernel.org designates 2604:1380:45e3:2400::1 as permitted sender) smtp.mailfrom="linux-kernel+bounces-85815-ouuuleilei=gmail.com@vger.kernel.org"; dmarc=pass (p=REJECT sp=REJECT dis=NONE) header.from=google.com Received: from sv.mirrors.kernel.org (sv.mirrors.kernel.org. [2604:1380:45e3:2400::1]) by mx.google.com with ESMTPS id k21-20020a635615000000b005cd7b938543si48705pgb.185.2024.02.28.15.00.22 for <ouuuleilei@gmail.com> (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Wed, 28 Feb 2024 15:00:22 -0800 (PST) Received-SPF: pass (google.com: domain of linux-kernel+bounces-85815-ouuuleilei=gmail.com@vger.kernel.org designates 2604:1380:45e3:2400::1 as permitted sender) client-ip=2604:1380:45e3:2400::1; Authentication-Results: mx.google.com; dkim=pass header.i=@google.com header.s=20230601 header.b=iraPFeRH; arc=pass (i=1 spf=pass spfdomain=flex--justinstitt.bounces.google.com dkim=pass dkdomain=google.com dmarc=pass fromdomain=google.com); spf=pass (google.com: domain of linux-kernel+bounces-85815-ouuuleilei=gmail.com@vger.kernel.org designates 2604:1380:45e3:2400::1 as permitted sender) smtp.mailfrom="linux-kernel+bounces-85815-ouuuleilei=gmail.com@vger.kernel.org"; dmarc=pass (p=REJECT sp=REJECT dis=NONE) header.from=google.com Received: from smtp.subspace.kernel.org (wormhole.subspace.kernel.org [52.25.139.140]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by sv.mirrors.kernel.org (Postfix) with ESMTPS id 2FEB828B99E for <ouuuleilei@gmail.com>; Wed, 28 Feb 2024 23:00:22 +0000 (UTC) Received: from localhost.localdomain (localhost.localdomain [127.0.0.1]) by smtp.subspace.kernel.org (Postfix) with ESMTP id 6A36686251; Wed, 28 Feb 2024 22:59:16 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=google.com header.i=@google.com header.b="iraPFeRH" Received: from mail-yw1-f201.google.com (mail-yw1-f201.google.com [209.85.128.201]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id D8D3672901 for <linux-kernel@vger.kernel.org>; Wed, 28 Feb 2024 22:59:10 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=209.85.128.201 ARC-Seal: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1709161152; cv=none; b=HL9dwpqRg8TltjLnoErKZEUIS17zvPUD5h+LHMkjjE0B3TkV8+ohmlZ/OTRLd3FFrcaSE23DLEwaqQB6ww0jcaP74I1RVbg59IPt4uzT43oST89kcCEfllAgt5oDJRw+S7R2ZXfe4m34MOT7lOE4MP7sWKoNk3Y7NpjC1I2i+10= ARC-Message-Signature: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1709161152; c=relaxed/simple; bh=Fzxw3B7E/9cimGnCcB1YYMSbhwnEqsP1/yKx00gcrAU=; h=Date:Mime-Version:Message-ID:Subject:From:To:Cc:Content-Type; b=kkxsAhoOhJyoJdsH0vynHw3jejv2177lP4Ev2ax4vOsW/V2kt29sxXfOZOMEH5TEqgXl1bEP78s3PSOKJGcCT2DMO7AHyWBVyFr9sK2cgV8H/IgBZ3Q64/32AapHTGxAb/vlaN7HWPOIG6paSNilNI7QhdH0StLlMJfd916hQAU= ARC-Authentication-Results: i=1; smtp.subspace.kernel.org; dmarc=pass (p=reject dis=none) header.from=google.com; spf=pass smtp.mailfrom=flex--justinstitt.bounces.google.com; dkim=pass (2048-bit key) header.d=google.com header.i=@google.com header.b=iraPFeRH; arc=none smtp.client-ip=209.85.128.201 Authentication-Results: smtp.subspace.kernel.org; dmarc=pass (p=reject dis=none) header.from=google.com Authentication-Results: smtp.subspace.kernel.org; spf=pass smtp.mailfrom=flex--justinstitt.bounces.google.com Received: by mail-yw1-f201.google.com with SMTP id 00721157ae682-6093f75fc81so5456957b3.1 for <linux-kernel@vger.kernel.org>; Wed, 28 Feb 2024 14:59:10 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=20230601; t=1709161150; x=1709765950; darn=vger.kernel.org; h=cc:to:from:subject:message-id:mime-version:date:from:to:cc:subject :date:message-id:reply-to; bh=bwcBf6VTN7kSbKJ8Vs5Gx4XAVHKYHpXJ1cRKxEXuKj0=; b=iraPFeRHZCaDwmnufgGQMKC41vAlkljMD5aEZMbtocoBv0Oqc0xIVZjm4gowenrOfq UzXVcdXnqwL/MncaNbLZTbeklu5dDLf80smf8YI6udNkK/LG7Uxdoh7ynA7lJBlidBnu IhVn8pBl4pl4yhu363XqrKONLUcoJi1/NeH76ss8vUTbg4W/BZow+iWHVb0BbpNsEqix Ulk2PixO5GLdSdPSRWM++ew1HeQijNxc9sATkfKxMPhQRuUNIHFu26+6kYOIhE+Etswk vk4NdG0Fb9eNmywN2J9sDHQAqvnu7oMJ6ws/18WI1+A1mj5VwVK0pxIh+aR8M6jgaLVT aXew== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1709161150; x=1709765950; h=cc:to:from:subject:message-id:mime-version:date:x-gm-message-state :from:to:cc:subject:date:message-id:reply-to; bh=bwcBf6VTN7kSbKJ8Vs5Gx4XAVHKYHpXJ1cRKxEXuKj0=; b=pLXInKVXJS2Jf5u+nl2XZ8wu8Hs9OGGoLR258CvvZ2lo7c/U9ifWilQVVpPPsqkbVN bmhcP067m34bRJ0PIDyLkynMaOVt0HBfN8hk55uCybS6B6Ba79gphXQVWxwD7zBJ68Ma HEbSdJntgx6Qhe9YAK9QdIAG4WYTQqFKW7jW/+iuphQh4YGo+8TFR6lVbLxYmnonjspS +4u921RMLUJ4E/s5ljJHNqGfEDwT4LrQG2unSXC1Cpwt78v4d6ujIcw3ZZWYFNUOsGhJ CF8ovDFgxwmVQ/fwI67/qXKWPn5tjiCboB1SDmQF4AamtCmuY7MQYpF+pBbUmpOG2ucK DuOQ== X-Forwarded-Encrypted: i=1; AJvYcCUH55A+4l9gdHr1KyVh2axGVQIhRGgPtWKbSlq0NsUPUXo7KaetEZTu/csR+pNJGIeCh1I4bbk84UkMFwMJa9LW3qa00uU67DjsO1/y X-Gm-Message-State: AOJu0Yz5GE1OH9X5Z+OoxptbF0dV8rs+Cf7HRMV9xmrQ8HBCrzpV9ko2 9jqiW91TVhZfaMb33siWGwZkTsoFucSHap8zswNCQE1wrCEsK+Okiu9P0SB2P4X14hGrUEqavpT cii+C622w4C0KVG1HABQtYA== X-Received: from jstitt-linux1.c.googlers.com ([fda3:e722:ac3:cc00:2b:ff92:c0a8:23b5]) (user=justinstitt job=sendgmr) by 2002:a05:690c:3744:b0:608:ce23:638c with SMTP id fw4-20020a05690c374400b00608ce23638cmr117431ywb.4.1709161150059; Wed, 28 Feb 2024 14:59:10 -0800 (PST) Date: Wed, 28 Feb 2024 22:59:00 +0000 Precedence: bulk X-Mailing-List: linux-kernel@vger.kernel.org List-Id: <linux-kernel.vger.kernel.org> List-Subscribe: <mailto:linux-kernel+subscribe@vger.kernel.org> List-Unsubscribe: <mailto:linux-kernel+unsubscribe@vger.kernel.org> Mime-Version: 1.0 X-B4-Tracking: v=1; b=H4sIALS632UC/43NTQrCQAyG4avIrI3Mj2J15T2kiM1M24DtlKSMl tK7OxbduwpvFt8zKwlMQdR5MysOiYRin8NuNwrbe98EIJ9bWW332loLMnKPwwSeKQUWEBSCbiD X8ffc6icgmMo47Q+m8ogqrw0canqt0rXM3ZKMkacVTubz/RnubyMZ0HBC74rC1vqo9aWJsXmEH cZOlcuyvAHoIOgv3wAAAA== X-Developer-Key: i=justinstitt@google.com; a=ed25519; pk=tC3hNkJQTpNX/gLKxTNQKDmiQl6QjBNCGKJINqAdJsE= X-Developer-Signature: v=1; a=ed25519-sha256; t=1709161149; l=1822; i=justinstitt@google.com; s=20230717; h=from:subject:message-id; bh=Fzxw3B7E/9cimGnCcB1YYMSbhwnEqsP1/yKx00gcrAU=; b=+xX1p9F5acPGhLQnOC0ZvGYgBGCYCRPUuIWu0ALKmc/Irx/7euLxtpBb0vFctrt3pFBFzOk26 qMxJqYSZlYPB+nRKJdxH1TPJ7iPps8Q9EjX7l5U6axtxrVXXb6PK2aN X-Mailer: b4 0.12.3 Message-ID: <20240228-strncpy-drivers-scsi-mpi3mr-mpi3mr_fw-c-v2-0-dacebd3fcfa0@google.com> Subject: [PATCH v2 0/7] scsi: replace deprecated strncpy From: Justin Stitt <justinstitt@google.com> To: Sathya Prakash Veerichetty <sathya.prakash@broadcom.com>, Kashyap Desai <kashyap.desai@broadcom.com>, Sumit Saxena <sumit.saxena@broadcom.com>, Sreekanth Reddy <sreekanth.reddy@broadcom.com>, "James E.J. Bottomley" <jejb@linux.ibm.com>, "Martin K. Petersen" <martin.petersen@oracle.com>, Suganath Prabu Subramani <suganath-prabu.subramani@broadcom.com>, Ariel Elior <aelior@marvell.com>, Manish Chopra <manishc@marvell.com>, "David S. Miller" <davem@davemloft.net>, Eric Dumazet <edumazet@google.com>, Jakub Kicinski <kuba@kernel.org>, Paolo Abeni <pabeni@redhat.com>, Saurav Kashyap <skashyap@marvell.com>, Javed Hasan <jhasan@marvell.com>, GR-QLogic-Storage-Upstream@marvell.com, Nilesh Javali <njavali@marvell.com>, Manish Rangankar <mrangankar@marvell.com>, Don Brace <don.brace@microchip.com> Cc: mpi3mr-linuxdrv.pdl@broadcom.com, linux-scsi@vger.kernel.org, linux-hardening@vger.kernel.org, linux-kernel@vger.kernel.org, Kees Cook <keescook@chromium.org>, MPT-FusionLinux.pdl@broadcom.com, netdev@vger.kernel.org, storagedev@microchip.com, Justin Stitt <justinstitt@google.com> Content-Type: text/plain; charset="utf-8" X-getmail-retrieved-from-mailbox: INBOX X-GMAIL-THRID: 1792185437748542095 X-GMAIL-MSGID: 1792185437748542095 |
Series | scsi: replace deprecated strncpy | |
Message
Justin Stitt
Feb. 28, 2024, 10:59 p.m. UTC
This series contains multiple replacements of strncpy throughout the scsi subsystem. strncpy() is deprecated for use on NUL-terminated destination strings [1] and as such we should prefer more robust and less ambiguous string interfaces. The details of each replacement will be in their respective patch. --- Changes in v2: - for (1/7): change strscpy to simple const char* assignments - Link to v1: https://lore.kernel.org/r/20240223-strncpy-drivers-scsi-mpi3mr-mpi3mr_fw-c-v1-0-9cd3882f0700@google.com --- Justin Stitt (7): scsi: mpi3mr: replace deprecated strncpy with assignments scsi: mpt3sas: replace deprecated strncpy with strscpy scsi: qedf: replace deprecated strncpy with strscpy scsi: qla4xxx: replace deprecated strncpy with strscpy scsi: devinfo: replace strncpy and manual pad scsi: smartpqi: replace deprecated strncpy with strscpy scsi: wd33c93: replace deprecated strncpy with strscpy drivers/net/ethernet/qlogic/qed/qed_main.c | 2 +- drivers/scsi/mpi3mr/mpi3mr_fw.c | 10 +++++----- drivers/scsi/mpt3sas/mpt3sas_base.c | 2 +- drivers/scsi/mpt3sas/mpt3sas_transport.c | 18 +++++++++--------- drivers/scsi/qedf/qedf_main.c | 2 +- drivers/scsi/qla4xxx/ql4_mbx.c | 17 ++++++++++++----- drivers/scsi/qla4xxx/ql4_os.c | 14 +++++++------- drivers/scsi/scsi_devinfo.c | 18 ++++++++++-------- drivers/scsi/smartpqi/smartpqi_init.c | 5 ++--- drivers/scsi/wd33c93.c | 4 +--- 10 files changed, 49 insertions(+), 43 deletions(-) --- base-commit: 39133352cbed6626956d38ed72012f49b0421e7b change-id: 20240222-strncpy-drivers-scsi-mpi3mr-mpi3mr_fw-c-1b130d51bdcc Best regards, -- Justin Stitt <justinstitt@google.com>
Comments
On Wed, Feb 28, 2024 at 10:59:03PM +0000, Justin Stitt wrote: > We expect slowpath_params.name to be NUL-terminated based on its future > usage with other string APIs: > > | static int qed_slowpath_start(struct qed_dev *cdev, > | struct qed_slowpath_params *params) > ... > | strscpy(drv_version.name, params->name, > | MCP_DRV_VER_STR_SIZE - 4); > > Moreover, NUL-padding is not necessary as the only use for this slowpath > name parameter is to copy into the drv_version.name field. > > Also, let's prefer using strscpy(src, dest, sizeof(src)) in two > instances (one of which is outside of the scsi system but it is trivial > and related to this patch). > > We can see the drv_version.name size here: > | struct qed_mcp_drv_version { > | u32 version; > | u8 name[MCP_DRV_VER_STR_SIZE - 4]; > | }; > > Signed-off-by: Justin Stitt <justinstitt@google.com> Reviewed-by: Kees Cook <keescook@chromium.org>
On Wed, Feb 28, 2024 at 10:59:04PM +0000, Justin Stitt wrote: > Replace 3 instances of strncpy in ql4_mbx.c > > No bugs exist in the current implementation as some care was taken to > ensure the write length was decreased by one to leave some space for a > NUL-byte. However, instead of using strncpy(dest, src, LEN-1) we can opt > for strscpy(dest, src, sizeof(dest)) which will result in > NUL-termination as well. It should be noted that the entire chap_table > is zero-allocated so the NUL-padding provided by strncpy is not needed. > > While here, I noticed that MIN_CHAP_SECRET_LEN was not used anywhere. > Since strscpy gives us the number of bytes copied into the destination > buffer (or an -E2BIG) we can check both for an error during copying and > also for a non-length compliant secret. Add a new jump label so we can > properly clean up our chap_table should we have to abort due to bad > secret. > > The third instance in this file involves some more peculiar handling of > strings: > | uint32_t mbox_cmd[MBOX_REG_COUNT]; > | ... > | memset(&mbox_cmd, 0, sizeof(mbox_cmd)); > | ... > | mbox_cmd[0] = MBOX_CMD_SET_PARAM; > | if (param == SET_DRVR_VERSION) { > | mbox_cmd[1] = SET_DRVR_VERSION; > | strncpy((char *)&mbox_cmd[2], QLA4XXX_DRIVER_VERSION, > | MAX_DRVR_VER_LEN - 1); > > mbox_cmd has a size of 8: > | #define MBOX_REG_COUNT 8 > ... and its type width is 4 bytes. Hence, we have 32 bytes to work with > here. The first 4 bytes are used as a flag for the MBOX_CMD_SET_PARAM. > The next 4 bytes are used for SET_DRVR_VERSION. We now have 32-8=24 > bytes remaining -- which thankfully is what MAX_DRVR_VER_LEN is equal to > | #define MAX_DRVR_VER_LEN 24 > > ... and the thing we're copying into this pseudo-string buffer is > | #define QLA4XXX_DRIVER_VERSION "5.04.00-k6" > > ... which is great because its less than 24 bytes (therefore we aren't > truncating the source). > > All to say, there's no bug in the existing implementation (yay!) but we > can clean the code up a bit by using strscpy(). > > In ql4_os.c, there aren't any strncpy() uses to replace but there are > some existing strscpy() calls that could be made more idiomatic. Where > possible, use strscpy(dest, src, sizeof(dest)). Note that > chap_rec->password has a size of ISCSI_CHAP_AUTH_SECRET_MAX_LEN > | #define ISCSI_CHAP_AUTH_SECRET_MAX_LEN 256 > ... while the current strscpy usage uses QL4_CHAP_MAX_SECRET_LEN > | #define QL4_CHAP_MAX_SECRET_LEN 100 > ... however since chap_table->secret was set and bounded properly in its > string assignment its probably safe here to switch over to sizeof(). > > | struct iscsi_chap_rec { > ... > | char username[ISCSI_CHAP_AUTH_NAME_MAX_LEN]; > | uint8_t password[ISCSI_CHAP_AUTH_SECRET_MAX_LEN]; > ... > | }; > > | strscpy(chap_rec->password, chap_table->secret, > | QL4_CHAP_MAX_SECRET_LEN); > > Signed-off-by: Justin Stitt <justinstitt@google.com> > --- > drivers/scsi/qla4xxx/ql4_mbx.c | 17 ++++++++++++----- > drivers/scsi/qla4xxx/ql4_os.c | 14 +++++++------- > 2 files changed, 19 insertions(+), 12 deletions(-) > > diff --git a/drivers/scsi/qla4xxx/ql4_mbx.c b/drivers/scsi/qla4xxx/ql4_mbx.c > index 249f1d7021d4..75125d2021f5 100644 > --- a/drivers/scsi/qla4xxx/ql4_mbx.c > +++ b/drivers/scsi/qla4xxx/ql4_mbx.c > @@ -1641,6 +1641,7 @@ int qla4xxx_set_chap(struct scsi_qla_host *ha, char *username, char *password, > struct ql4_chap_table *chap_table; > uint32_t chap_size = 0; > dma_addr_t chap_dma; > + ssize_t secret_len; > > chap_table = dma_pool_zalloc(ha->chap_dma_pool, GFP_KERNEL, &chap_dma); > if (chap_table == NULL) { > @@ -1652,9 +1653,13 @@ int qla4xxx_set_chap(struct scsi_qla_host *ha, char *username, char *password, > chap_table->flags |= BIT_6; /* peer */ > else > chap_table->flags |= BIT_7; /* local */ > - chap_table->secret_len = strlen(password); > - strncpy(chap_table->secret, password, MAX_CHAP_SECRET_LEN - 1); > - strncpy(chap_table->name, username, MAX_CHAP_NAME_LEN - 1); > + > + secret_len = strscpy(chap_table->secret, password, > + sizeof(chap_table->secret)); > + if (secret_len < MIN_CHAP_SECRET_LEN) > + goto cleanup_chap_table; > + chap_table->secret_len = (uint8_t)secret_len; > + strscpy(chap_table->name, username, sizeof(chap_table->name)); > chap_table->cookie = cpu_to_le16(CHAP_VALID_COOKIE); > > if (is_qla40XX(ha)) { > @@ -1679,6 +1684,8 @@ int qla4xxx_set_chap(struct scsi_qla_host *ha, char *username, char *password, > memcpy((struct ql4_chap_table *)ha->chap_list + idx, > chap_table, sizeof(struct ql4_chap_table)); > } > + > +cleanup_chap_table: > dma_pool_free(ha->chap_dma_pool, chap_table, chap_dma); > if (rval != QLA_SUCCESS) > ret = -EINVAL; > @@ -2281,8 +2288,8 @@ int qla4_8xxx_set_param(struct scsi_qla_host *ha, int param) > mbox_cmd[0] = MBOX_CMD_SET_PARAM; > if (param == SET_DRVR_VERSION) { > mbox_cmd[1] = SET_DRVR_VERSION; > - strncpy((char *)&mbox_cmd[2], QLA4XXX_DRIVER_VERSION, > - MAX_DRVR_VER_LEN - 1); > + strscpy((char *)&mbox_cmd[2], QLA4XXX_DRIVER_VERSION, > + MAX_DRVR_VER_LEN); > } else { > ql4_printk(KERN_ERR, ha, "%s: invalid parameter 0x%x\n", > __func__, param); > diff --git a/drivers/scsi/qla4xxx/ql4_os.c b/drivers/scsi/qla4xxx/ql4_os.c > index 675332e49a7b..17cccd14765f 100644 > --- a/drivers/scsi/qla4xxx/ql4_os.c > +++ b/drivers/scsi/qla4xxx/ql4_os.c > @@ -799,10 +799,10 @@ static int qla4xxx_get_chap_list(struct Scsi_Host *shost, uint16_t chap_tbl_idx, > > chap_rec->chap_tbl_idx = i; > strscpy(chap_rec->username, chap_table->name, > - ISCSI_CHAP_AUTH_NAME_MAX_LEN); > - strscpy(chap_rec->password, chap_table->secret, > - QL4_CHAP_MAX_SECRET_LEN); > - chap_rec->password_length = chap_table->secret_len; > + sizeof(chap_rec->username)); > + chap_rec->password_length = strscpy(chap_rec->password, > + chap_table->secret, > + sizeof(chap_rec->password)); This hunk took me a bit to convince myself it's safe, but yes, I agree. It all boils down to sizeof(chap_table->secret) being less than sizeof(chap_rec->password), so there's no behavioral change here. Reviewed-by: Kees Cook <keescook@chromium.org>
On Wed, Feb 28, 2024 at 10:59:00PM +0000, Justin Stitt wrote: > This series contains multiple replacements of strncpy throughout the > scsi subsystem. > > strncpy() is deprecated for use on NUL-terminated destination strings > [1] and as such we should prefer more robust and less ambiguous string > interfaces. The details of each replacement will be in their respective > patch. > > --- > Changes in v2: > - for (1/7): change strscpy to simple const char* assignments > - Link to v1: https://lore.kernel.org/r/20240223-strncpy-drivers-scsi-mpi3mr-mpi3mr_fw-c-v1-0-9cd3882f0700@google.com I think you lost my tags for the later patches. I re-reviewed a few and then remembered I'd already reviewed these and they were unchanged. :) Please run: b4 trailers -u 20240223-strncpy-drivers-scsi-mpi3mr-mpi3mr_fw-c-v1-0-9cd3882f0700@google.com :) -Kees