Re: [PATCH] sparc: vdso: Disable UBSAN instrumentation

2024-02-23 Thread Sam Ravnborg
Hi Kees, On Fri, Feb 23, 2024 at 03:32:37PM -0800, Kees Cook wrote: > On Fri, Feb 23, 2024 at 07:26:46PM +0100, Sam Ravnborg wrote: > > Hi Kees, > > > > On Fri, Feb 23, 2024 at 08:59:45AM -0800, Kees Cook wrote: > > > The UBSAN instrumentation cannot work in the vDSO since it is executing > > >

Re: [PATCH 6/7] scsi: smartpqi: replace deprecated strncpy with strscpy

2024-02-23 Thread Kees Cook
On Fri, Feb 23, 2024 at 10:23:11PM +, Justin Stitt wrote: > buffer->driver_version is sized 32: > | struct bmic_host_wellness_driver_version { > | ... > | chardriver_version[32]; > ... the source string "Linux " + DRIVER_VERISON is sized at 16. There's > really no bug

Re: [PATCH 5/7] scsi: devinfo: replace strncpy and manual pad

2024-02-23 Thread Kees Cook
On Fri, Feb 23, 2024 at 10:23:10PM +, Justin Stitt wrote: > Depending on the state of @compatible, we are going to do different > things with our @to buffer. > > When @compatible is true we want a NUL-term'd and NUL-padded destination > buffer. Conversely, if @compatible is false we just want

Re: [PATCH 4/7] scsi: qla4xxx: replace deprecated strncpy with strscpy

2024-02-23 Thread Kees Cook
On Fri, Feb 23, 2024 at 10:23:09PM +, 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

Re: [PATCH 3/7] scsi: qedf: replace deprecated strncpy with strscpy

2024-02-23 Thread Kees Cook
On Fri, Feb 23, 2024 at 10:23:08PM +, 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

Re: [PATCH 2/7] scsi: mpt3sas: replace deprecated strncpy with strscpy

2024-02-23 Thread Kees Cook
On Fri, Feb 23, 2024 at 10:23:07PM +, Justin Stitt wrote: > The replacement in mpt3sas_base.c is a trivial one because desc is > already zero-initialized meaning there is no functional change here. > > For mpt3sas_transport.c, we know edev is zero-initialized as well while > manufacture_reply

Re: [PATCH 1/7] scsi: mpi3mr: replace deprecated strncpy with strscpy

2024-02-23 Thread Kees Cook
On Sat, Feb 24, 2024 at 10:58:07AM +1100, Finn Thain wrote: > > On Fri, 23 Feb 2024, Justin Stitt wrote: > > > Really, there's no bug with the current code. > > If (hypothetically) you needed to reduce stack size, just copy the char > pointer instead of copying the chars onto the stack. > >

Re: [PATCH 7/7] scsi: wd33c93: replace deprecated strncpy with strscpy

2024-02-23 Thread Kees Cook
On Sat, Feb 24, 2024 at 10:44:12AM +1100, Finn Thain wrote: > > 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

Re: [PATCH 1/7] scsi: mpi3mr: replace deprecated strncpy with strscpy

2024-02-23 Thread Finn Thain
On Fri, 23 Feb 2024, Justin Stitt wrote: > Really, there's no bug with the current code. If (hypothetically) you needed to reduce stack size, just copy the char pointer instead of copying the chars onto the stack. If (hypothetically) strncpy() was banned altogether (rather than merely

Re: [PATCH] scsi: lpfc: replace deprecated strncpy with strscpy

2024-02-23 Thread Kees Cook
On Fri, Feb 23, 2024 at 12:02:22PM -0800, Justin Stitt wrote: > Hi, > > On Wed, Feb 21, 2024 at 6:38 PM Kees Cook wrote: > > > > > > > > On February 21, 2024 4:41:52 PM PST, Justin Stitt > > wrote: > > >strncpy() is deprecated for use on NUL-terminated destination strings > > >[1] and as such

Re: [PATCH 7/7] scsi: wd33c93: replace deprecated strncpy with strscpy

2024-02-23 Thread Finn Thain
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

Re: [PATCH] sparc: vdso: Disable UBSAN instrumentation

2024-02-23 Thread Kees Cook
On Fri, Feb 23, 2024 at 07:26:46PM +0100, Sam Ravnborg wrote: > Hi Kees, > > On Fri, Feb 23, 2024 at 08:59:45AM -0800, Kees Cook wrote: > > The UBSAN instrumentation cannot work in the vDSO since it is executing > > in userspace, so disable it in the Makefile. Fixes the build failures > > such

[PATCH 7/7] scsi: wd33c93: replace deprecated strncpy with strscpy

2024-02-23 Thread Justin Stitt
@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

[PATCH 6/7] scsi: smartpqi: replace deprecated strncpy with strscpy

2024-02-23 Thread Justin Stitt
buffer->driver_version is sized 32: | struct bmic_host_wellness_driver_version { | ... | chardriver_version[32]; ... the source string "Linux " + DRIVER_VERISON is sized at 16. There's really no bug in the existing code since the buffers are sized appropriately with

[PATCH 5/7] scsi: devinfo: replace strncpy and manual pad

2024-02-23 Thread Justin Stitt
Depending on the state of @compatible, we are going to do different things with our @to buffer. When @compatible is true we want a NUL-term'd and NUL-padded destination buffer. Conversely, if @compatible is false we just want a space-padded destination buffer (no NUL-term required). As per: /**

[PATCH 4/7] scsi: qla4xxx: replace deprecated strncpy with strscpy

2024-02-23 Thread Justin Stitt
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))

[PATCH 3/7] scsi: qedf: replace deprecated strncpy with strscpy

2024-02-23 Thread Justin Stitt
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, |

[PATCH 2/7] scsi: mpt3sas: replace deprecated strncpy with strscpy

2024-02-23 Thread Justin Stitt
The replacement in mpt3sas_base.c is a trivial one because desc is already zero-initialized meaning there is no functional change here. For mpt3sas_transport.c, we know edev is zero-initialized as well while manufacture_reply comes from dma_alloc_coherent(). No functional change here either. For

[PATCH 1/7] scsi: mpi3mr: replace deprecated strncpy with strscpy

2024-02-23 Thread Justin Stitt
Really, there's no bug with the current code. Let's just ditch strncpy() all together. Since strscpy() will not NUL-pad the destination buffer let's NUL-initialize @personality; just like the others. Link:

[PATCH 0/7] scsi: replace deprecated strncpy

2024-02-23 Thread Justin Stitt
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

Re: [PATCH] scsi: lpfc: replace deprecated strncpy with strscpy

2024-02-23 Thread Justin Stitt
Hi, On Wed, Feb 21, 2024 at 6:38 PM Kees Cook wrote: > > > > On February 21, 2024 4:41:52 PM PST, Justin Stitt > wrote: > >strncpy() is deprecated for use on NUL-terminated destination strings > >[1] and as such we should prefer more robust and less ambiguous string > >interfaces. > > > >We

Re: [PATCH] init/Kconfig: Lower GCC version check for -Warray-bounds

2024-02-23 Thread Paul Moore
On Fri, Feb 23, 2024 at 12:08 PM Kees Cook wrote: > > We continue to see false positives from -Warray-bounds even in GCC 10, > which is getting reported in a few places[1] still: > > security/security.c:811:2: warning: ‘memcpy’ offset 32 is out of the bounds > [0, 0] [-Warray-bounds] > > Lower

Re: [PATCH] sparc: vdso: Disable UBSAN instrumentation

2024-02-23 Thread Sam Ravnborg
Hi Kees, On Fri, Feb 23, 2024 at 08:59:45AM -0800, Kees Cook wrote: > The UBSAN instrumentation cannot work in the vDSO since it is executing > in userspace, so disable it in the Makefile. Fixes the build failures > such as: > > arch/sparc/vdso/vclock_gettime.c:217: undefined reference to >

Re: [PATCH v2] iio: pressure: dlhl60d: Initialize empty DLH bytes

2024-02-23 Thread Kees Cook
On Fri, Feb 23, 2024 at 07:47:36PM +0200, Andy Shevchenko wrote: > On Fri, Feb 23, 2024 at 09:29:39AM -0800, Kees Cook wrote: > > 3 bytes were being read but 4 were being written. Explicitly initialize > > the unused bytes to 0 and refactor the loop to use direct array > > indexing, which appears

Re: [PATCH v2] iio: pressure: dlhl60d: Initialize empty DLH bytes

2024-02-23 Thread Andy Shevchenko
On Fri, Feb 23, 2024 at 07:47:36PM +0200, Andy Shevchenko wrote: > On Fri, Feb 23, 2024 at 09:29:39AM -0800, Kees Cook wrote: > > 3 bytes were being read but 4 were being written. Explicitly initialize > > the unused bytes to 0 and refactor the loop to use direct array > > indexing, which appears

Re: [PATCH v2] iio: pressure: dlhl60d: Initialize empty DLH bytes

2024-02-23 Thread Andy Shevchenko
On Fri, Feb 23, 2024 at 09:29:39AM -0800, Kees Cook wrote: > 3 bytes were being read but 4 were being written. Explicitly initialize > the unused bytes to 0 and refactor the loop to use direct array > indexing, which appears to silence a Clang false positive warning[1]. ... >

Re: [PATCH] [RFC] iio: pressure: dlhl60d: Check mask_width for IRQs

2024-02-23 Thread Jonathan Cameron
On Fri, 23 Feb 2024 09:14:53 -0800 Kees Cook wrote: > On Fri, Feb 23, 2024 at 05:09:18PM +, Jonathan Cameron wrote: > > On Thu, 22 Feb 2024 14:23:39 -0800 > > Kees Cook wrote: > > > > > Clang tripped over a FORTIFY warning in this code, and while it seems it > > > may be a false positive

Re: [PATCH v1 1/1] kernel.h: Move lib/cmdline.c prototypes to string.h

2024-02-23 Thread Kees Cook
On Tue, 03 Oct 2023 16:01:42 +0300, Andy Shevchenko wrote: > The lib/cmdline.c is basically a set of some small string parsers > which are wide used in the kernel. Their prototypes belong to the > string.h rather then kernel.h. Applied to for-next/hardening, thanks! [1/1] kernel.h: Move

[PATCH v2] iio: pressure: dlhl60d: Initialize empty DLH bytes

2024-02-23 Thread Kees Cook
3 bytes were being read but 4 were being written. Explicitly initialize the unused bytes to 0 and refactor the loop to use direct array indexing, which appears to silence a Clang false positive warning[1]. Link: https://github.com/ClangBuiltLinux/linux/issues/2000 [1] Fixes: ac78c6aa4a5d ("iio:

Re: [PATCH] [RFC] iio: pressure: dlhl60d: Check mask_width for IRQs

2024-02-23 Thread Kees Cook
On Fri, Feb 23, 2024 at 05:09:18PM +, Jonathan Cameron wrote: > On Thu, 22 Feb 2024 14:23:39 -0800 > Kees Cook wrote: > > > Clang tripped over a FORTIFY warning in this code, and while it seems it > > may be a false positive in Clang due to loop unwinding, the code in > > question seems to

Re: [PATCH] [RFC] iio: pressure: dlhl60d: Check mask_width for IRQs

2024-02-23 Thread Jonathan Cameron
On Thu, 22 Feb 2024 14:23:39 -0800 Kees Cook wrote: > Clang tripped over a FORTIFY warning in this code, and while it seems it > may be a false positive in Clang due to loop unwinding, the code in > question seems to make a lot of assumptions. Hi Kees, The assumptions are mostly

[PATCH] init/Kconfig: Lower GCC version check for -Warray-bounds

2024-02-23 Thread Kees Cook
We continue to see false positives from -Warray-bounds even in GCC 10, which is getting reported in a few places[1] still: security/security.c:811:2: warning: ‘memcpy’ offset 32 is out of the bounds [0, 0] [-Warray-bounds] Lower the GCC version check from 11 to 10. Reported-by: Lu Yao Closes:

[PATCH] sparc: vdso: Disable UBSAN instrumentation

2024-02-23 Thread Kees Cook
The UBSAN instrumentation cannot work in the vDSO since it is executing in userspace, so disable it in the Makefile. Fixes the build failures such as: arch/sparc/vdso/vclock_gettime.c:217: undefined reference to `__ubsan_handle_shift_out_of_bounds' Signed-off-by: Kees Cook --- Cc: "David S.

Re: [linux-next:master] BUILD REGRESSION e31185ce00a96232308300008db193416ceb9769

2024-02-23 Thread John Paul Adrian Glaubitz
On Fri, 2024-02-23 at 07:46 -0800, Kees Cook wrote: > > arch/sh/boot/compressed/../../../../lib/decompress_unxz.c:350:(.text+0x20b4): > > undefined reference to `__ubsan_handle_out_of_bounds' > > sh4-linux-ld: > > arch/sh/boot/compressed/../../../../lib/xz/xz_dec_lzma2.c:751:(.text+0x904): > >

Re: [linux-next:master] BUILD REGRESSION e31185ce00a96232308300008db193416ceb9769

2024-02-23 Thread Kees Cook
On February 22, 2024 8:29:28 PM PST, kernel test robot wrote: >tree/branch: >https://git.kernel.org/pub/scm/linux/kernel/git/next/linux-next.git master >branch HEAD: e31185ce00a9623230838db193416ceb9769 Add linux-next specific >files for 20240222 > >Error/Warning reports: >

Re: [PATCH v2] checkpatch: add check for snprintf to scnprintf

2024-02-23 Thread Joe Perches
On Fri, 2024-02-23 at 10:38 +, Lee Jones wrote: > On Wed, 21 Feb 2024, Joe Perches wrote: > > > On Wed, 2024-02-21 at 22:11 +, Justin Stitt wrote: > > > I am going to quote Lee Jones who has been doing some snprintf -> > > > scnprintf refactorings: > > > > > > "There is a general

Re: [PATCH v2] checkpatch: add check for snprintf to scnprintf

2024-02-23 Thread Lee Jones
On Thu, 22 Feb 2024, David Laight wrote: > From: Justin Stitt > > Sent: 21 February 2024 22:12 > > > > I am going to quote Lee Jones who has been doing some snprintf -> > > scnprintf refactorings: > > > > "There is a general misunderstanding amongst engineers that > > {v}snprintf() returns the

Re: [PATCH v2] checkpatch: add check for snprintf to scnprintf

2024-02-23 Thread Lee Jones
On Wed, 21 Feb 2024, Joe Perches wrote: > On Wed, 2024-02-21 at 22:11 +, Justin Stitt wrote: > > I am going to quote Lee Jones who has been doing some snprintf -> > > scnprintf refactorings: > > > > "There is a general misunderstanding amongst engineers that > > {v}snprintf() returns the