From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from flow6-smtp.messagingengine.com (flow6-smtp.messagingengine.com [103.168.172.141]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id F0A8A84FB3; Fri, 23 Feb 2024 23:43:37 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=103.168.172.141 ARC-Seal:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1708731820; cv=none; b=eN6czBg4aQAf924MvLxhCGB+V6Um1Ep8iBxHlVvITQO+NqLFC8IqXAL//h/P4pCgH0RXRlLpRAaLVOnL4hmAoI994hzT9+AAcdqyTWdrr+4rXhGflpP4w3Jz4qUw89AAFny7Y5R+3dLu5/z4vRGHJOw+Etveb8QiKXoNRf0LMo8= ARC-Message-Signature:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1708731820; c=relaxed/simple; bh=uc2tZGR4P0x+PoLZlQj6rY29hlVYa/H5ELy0Qw70NOE=; h=Date:From:To:cc:Subject:In-Reply-To:Message-ID:References: MIME-Version:Content-Type; b=FL0Bkiv+knp5Q+rGDHTUvgWgwt0JWZe/c+BHhgj3BQUWwtfi53Sbys8T7nPns90lmgE6abHnNbIhEq0xzGttQCqeotUuurGmqKmcxiJGi/+d9OoN00zLzwZCWLmAbn6apcdFx5OuwshnWGcXY9OUunWpTFhIEoydDvPI3Bp9lU4= ARC-Authentication-Results:i=1; smtp.subspace.kernel.org; dmarc=none (p=none dis=none) header.from=linux-m68k.org; spf=none smtp.mailfrom=linux-m68k.org; dkim=pass (2048-bit key) header.d=messagingengine.com header.i=@messagingengine.com header.b=E3zsd4U6; arc=none smtp.client-ip=103.168.172.141 Authentication-Results: smtp.subspace.kernel.org; dmarc=none (p=none dis=none) header.from=linux-m68k.org Authentication-Results: smtp.subspace.kernel.org; spf=none smtp.mailfrom=linux-m68k.org Authentication-Results: smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=messagingengine.com header.i=@messagingengine.com header.b="E3zsd4U6" Received: from compute1.internal (compute1.nyi.internal [10.202.2.41]) by mailflow.nyi.internal (Postfix) with ESMTP id B67DF2000CD; Fri, 23 Feb 2024 18:43:36 -0500 (EST) Received: from mailfrontend1 ([10.202.2.162]) by compute1.internal (MEProxy); Fri, 23 Feb 2024 18:43:36 -0500 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d= messagingengine.com; h=cc:cc:content-type:content-type:date:date :feedback-id:feedback-id:from:from:in-reply-to:in-reply-to :message-id:mime-version:references:reply-to:subject:subject:to :to:x-me-proxy:x-me-proxy:x-me-sender:x-me-sender:x-sasl-enc; s= fm1; t=1708731816; x=1708739016; bh=liG+eRquBSeLX1xRKI6KGXUP1iEl r+oOxWU/4UKc9+A=; b=E3zsd4U6ySZdHjZ1ZcyNYUztkCoxVsEl2WezxqhTe6yh Mi6CiE828lh7V8bkEmLJbvkTZ+uiCzXg5I3iA5WEsdoVoIpH5g2if3uSg6FhAP8I ybmYhl+E/FAmIuvVF64qkGKZpUbtwGUcQcFf0IMOL8tDNyNFGZ3Di+IzxNxLEaxE O52NsXuaNXXIG0Ttm5NyvKHmkdfsto145HbmEzTXpD1CHTbsTpH412oEZra6Rv6G C22vgUIn3tcXtPIAlsGfeBdjBNlRYi7VeoDfbA5ihapHwd2mRHPrMdZYyFKsLmG8 JWPY4WIOFcy0ZtjlDRpDoE4RQcjw3WCpLwvHSlDjxw== X-ME-Sender: X-ME-Received: X-ME-Proxy-Cause: gggruggvucftvghtrhhoucdtuddrgedvledrfeejgddufecutefuodetggdotefrodftvf curfhrohhfihhlvgemucfhrghsthforghilhdpqfgfvfdpuffrtefokffrpgfnqfghnecu uegrihhlohhuthemuceftddtnecusecvtfgvtghiphhivghnthhsucdlqddutddtmdenuc fjughrpeffhffvvefujgfkfhggtgesthdtredttddtvdenucfhrhhomhephfhinhhnucfv hhgrihhnuceofhhthhgrihhnsehlihhnuhigqdhmieekkhdrohhrgheqnecuggftrfgrth htvghrnhepleeuheelheekgfeuvedtveetjeekhfffkeeffffftdfgjeevkeegfedvueeh ueelnecuvehluhhsthgvrhfuihiivgeptdenucfrrghrrghmpehmrghilhhfrhhomhepfh hthhgrihhnsehlihhnuhigqdhmieekkhdrohhrgh X-ME-Proxy: Feedback-ID: i58a146ae:Fastmail Received: by mail.messagingengine.com (Postfix) with ESMTPA; Fri, 23 Feb 2024 18:43:32 -0500 (EST) Date: Sat, 24 Feb 2024 10:44:12 +1100 (AEDT) From: Finn Thain To: Justin Stitt cc: Sathya Prakash Veerichetty , Kashyap Desai , Sumit Saxena , Sreekanth Reddy , "James E.J. Bottomley" , "Martin K. Petersen" , Suganath Prabu Subramani , Ariel Elior , Manish Chopra , "David S. Miller" , Eric Dumazet , Jakub Kicinski , Paolo Abeni , Saurav Kashyap , Javed Hasan , GR-QLogic-Storage-Upstream@marvell.com, Nilesh Javali , Manish Rangankar , Don Brace , mpi3mr-linuxdrv.pdl@broadcom.com, linux-scsi@vger.kernel.org, linux-hardening@vger.kernel.org, linux-kernel@vger.kernel.org, Kees Cook , MPT-FusionLinux.pdl@broadcom.com, netdev@vger.kernel.org, storagedev@microchip.com Subject: Re: [PATCH 7/7] scsi: wd33c93: replace deprecated strncpy with strscpy In-Reply-To: <20240223-strncpy-drivers-scsi-mpi3mr-mpi3mr_fw-c-v1-7-9cd3882f0700@google.com> Message-ID: <4a52a2ae-8abf-30e2-5c2a-d57280cb6028@linux-m68k.org> References: <20240223-strncpy-drivers-scsi-mpi3mr-mpi3mr_fw-c-v1-0-9cd3882f0700@google.com> <20240223-strncpy-drivers-scsi-mpi3mr-mpi3mr_fw-c-v1-7-9cd3882f0700@google.com> Precedence: bulk X-Mailing-List: netdev@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 Content-Type: text/plain; charset=US-ASCII On Fri, 23 Feb 2024, Justin Stitt wrote: > @p1 is assigned to @setup_buffer and then we manually assign a NUL-byte > at the first index. This renders the following strlen() call useless. > Moreover, we don't need to reassign p1 to setup_buffer for any reason -- > neither do we need to manually set a NUL-byte at the end. strscpy() > resolves all this code making it easier to read. > > Even considering the path where @str is falsey, the manual NUL-byte > assignment is useless And yet your patch would only remove one of those assignments... > as setup_buffer is declared with static storage > duration in the top-level scope which should NUL-initialize the whole > buffer. > So, in order to review this patch, to try to avoid regressions, I would have to check your assumption that setup_buffer cannot change after being statically initialized. (The author of this code apparently was not willing to make that assumption.) It seems that patch review would require exhaustively searching for functions using the buffer, and examining the call graphs involving those functions. Is it really worth the effort? > Signed-off-by: Justin Stitt > --- > drivers/scsi/wd33c93.c | 4 +--- > 1 file changed, 1 insertion(+), 3 deletions(-) > > diff --git a/drivers/scsi/wd33c93.c b/drivers/scsi/wd33c93.c > index e4fafc77bd20..a44b60c9004a 100644 > --- a/drivers/scsi/wd33c93.c > +++ b/drivers/scsi/wd33c93.c > @@ -1721,9 +1721,7 @@ wd33c93_setup(char *str) > p1 = setup_buffer; > *p1 = '\0'; > if (str) > - strncpy(p1, str, SETUP_BUFFER_SIZE - strlen(setup_buffer)); > - setup_buffer[SETUP_BUFFER_SIZE - 1] = '\0'; > - p1 = setup_buffer; > + strscpy(p1, str, SETUP_BUFFER_SIZE); > i = 0; > while (*p1 && (i < MAX_SETUP_ARGS)) { > p2 = strchr(p1, ','); > >