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

Kees Cook
2024-04-17T17:35:49Z

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