Message ID | 20240410021833.work.750-kees@kernel.org |
---|---|
Headers | show |
Series | scsi: Avoid possible run-time warning with long manufacturer strings | expand |
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 >
Hi Kees! >> 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? It's on my list of series to review. Have a couple of fires going right now.
On Wed, Apr 17, 2024 at 08:35:15PM -0400, Martin K. Petersen wrote: > > Hi Kees! > > >> 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? > > It's on my list of series to review. Have a couple of fires going right > now. Okay, thanks! I just wanted to make sure it got picked up for v6.9 since it fixes a real-world issue. :)
Kees, > This series fixes all 4 of the instances I could find in the SCSI > subsystem. Looks OK to me. Minor nit: I do find it a bit odd to think of a string as "memory". Maybe that's just because I am so used to always having to distinguish between fixed length strings and NUL-terminated strings in the storage protocols and hardware programming interfaces. But both types are definitely referred to as "strings" colloquially and not so much "memory". Reviewed-by: Martin K. Petersen <martin.petersen@oracle.com>