Re: [PATCH 0/5] scsi: Avoid possible run-time warning with long manufacturer strings

From: Kees Cook <keescook@chromium.org>
To: "Martin K . Petersen" <martin.petersen@oracle.com>
Cc: Charles Bertsch <cbertsch@cox.net>,
	Justin Stitt <justinstitt@google.com>,
	Bart Van Assche <bvanassche@acm.org>,
	Andy Shevchenko <andy@kernel.org>,
	Sathya Prakash <sathya.prakash@broadcom.com>,
	Sreekanth Reddy <sreekanth.reddy@broadcom.com>,
	Suganath Prabu Subramani <suganath-prabu.subramani@broadcom.com>,
	"James E.J. Bottomley" <jejb@linux.ibm.com>,
	Kashyap Desai <kashyap.desai@broadcom.com>,
	Sumit Saxena <sumit.saxena@broadcom.com>,
	Nilesh Javali <njavali@marvell.com>,
	Andrew Morton <akpm@linux-foundation.org>,
	Himanshu Madhani <himanshu.madhani@oracle.com>,
	linux-kernel@vger.kernel.org, linux-hardening@vger.kernel.org,
	MPT-FusionLinux.pdl@broadcom.com, linux-scsi@vger.kernel.org,
	mpi3mr-linuxdrv.pdl@broadcom.com,
	GR-QLogic-Storage-Upstream@marvell.com
Subject: Re: [PATCH 0/5] scsi: Avoid possible run-time warning with long manufacturer strings
Date: Wed, 17 Apr 2024 10:35:47 -0700	[thread overview]
Message-ID: <202404171035.BDFF28D@keescook> (raw)
In-Reply-To: <20240410021833.work.750-kees@kernel.org>

On Tue, Apr 09, 2024 at 07:31:49PM -0700, Kees Cook wrote:
> Hi,
> 
> Another code pattern using the gloriously ambiguous strncpy() function was
> turning maybe not-NUL-terminated character arrays into NUL-terminated
> strings. In these cases, when strncpy() is replaced with strscpy()
> it creates a situation where if the non-terminated string takes up the
> entire character array (i.e. it is not terminated) run-time warnings
> from CONFIG_FORTIFY_SOURCE will trip, since strscpy() was expecting to
> find a NUL-terminated source but checking for the NUL would walk off
> the end of the source buffer.
> 
> In doing an instrumented build of the kernel to find these cases, it
> seems it was almost entirely a code pattern used in the SCSI subsystem,
> so the creation of the new helper, memtostr(), can land via the SCSI
> tree. And, as it turns out, inappropriate conversions have been happening
> for several years now, some of which even moved through strlcpy() first
> (and were never noticed either).
> 
> This series fixes all 4 of the instances I could find in the SCSI
> subsystem.

Friendly ping. Can the SCSI tree pick this up, or should I take it
through the hardening tree?

Thanks!

-Kees

> 
> Thanks,
> 
> -Kees
> 
> Kees Cook (5):
>   string.h: Introduce memtostr() and memtostr_pad()
>   scsi: mptfusion: Avoid possible run-time warning with long
>     manufacturer strings
>   scsi: mpt3sas: Avoid possible run-time warning with long manufacturer
>     strings
>   scsi: mpi3mr: Avoid possible run-time warning with long manufacturer
>     strings
>   scsi: qla2xxx: Avoid possible run-time warning with long model_num
> 
>  drivers/message/fusion/mptsas.c          | 14 +++----
>  drivers/scsi/mpi3mr/mpi3mr_transport.c   | 14 +++----
>  drivers/scsi/mpt3sas/mpt3sas_base.c      |  2 +-
>  drivers/scsi/mpt3sas/mpt3sas_transport.c | 14 +++----
>  drivers/scsi/qla2xxx/qla_mr.c            |  6 +--
>  include/linux/string.h                   | 49 ++++++++++++++++++++++++
>  lib/strscpy_kunit.c                      | 26 +++++++++++++
>  7 files changed, 93 insertions(+), 32 deletions(-)
> 
> -- 
> 2.34.1
> 

-- 
Kees Cook