[PATCH 00/11] address remaining stringop-truncation warnings

[PATCH 00/11] address remaining stringop-truncation warnings

[Arnd Bergmann]

From: Arnd Bergmann <arnd@arndb.de>

We are close to being able to turn on -Wstringop-truncation
unconditionally instead of only at the 'make W=1' level, these ten
warnings are all that I saw in randconfig testing across compiler versions
on arm, arm64 and x86.

The final patch is only there for reference at the moment, I hope
we can merge the other ones through the subsystem trees first,
as there are no dependencies between them.

     Arnd

Arnd Bergmann (11):
  staging: vc04_services: changen strncpy() to strscpy_pad()
  scsi: devinfo: rework scsi_strcpy_devinfo()
  staging: replace weird strncpy() with memcpy()
  orangefs: convert strncpy() to strscpy()
  test_hexdump: avoid string truncation warning
  acpi: avoid warning for truncated string copy
  block/partitions/ldm: convert strncpy() to strscpy()
  blktrace: convert strncpy() to strscpy_pad()
  staging: rtl8723bs: convert strncpy to strscpy
  staging: greybus: change strncpy() to strscpy()
  kbuild: enable -Wstringop-truncation globally

 block/partitions/ldm.c                        |  6 ++--
 drivers/acpi/acpica/tbfind.c                  | 19 +++++------
 drivers/scsi/scsi_devinfo.c                   | 30 +++++++++++------
 drivers/staging/greybus/fw-management.c       |  4 +--
 .../staging/rtl8723bs/os_dep/ioctl_cfg80211.c |  5 ++-
 drivers/staging/rts5208/rtsx_scsi.c           |  2 +-
 .../vc04_services/vchiq-mmal/mmal-vchiq.c     |  4 +--
 fs/orangefs/dcache.c                          |  4 +--
 fs/orangefs/namei.c                           | 33 +++++++++----------
 fs/orangefs/super.c                           | 16 ++++-----
 kernel/trace/blktrace.c                       |  3 +-
 lib/test_hexdump.c                            |  2 +-
 scripts/Makefile.extrawarn                    |  1 -
 13 files changed, 64 insertions(+), 65 deletions(-)

-- 
2.39.2

Cc: "Richard Russon
Cc: Jens Axboe <axboe@kernel.dk>
Cc: Robert Moore <robert.moore@intel.com>
Cc: "Rafael J. Wysocki" <rafael.j.wysocki@intel.com>
Cc: Len Brown <lenb@kernel.org>
Cc: "James E.J. Bottomley" <jejb@linux.ibm.com>
Cc: "Martin K. Petersen" <martin.petersen@oracle.com>
Cc: Viresh Kumar <vireshk@kernel.org>
Cc: Johan Hovold <johan@kernel.org>
Cc: Alex Elder <elder@kernel.org>
Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Cc: Florian Fainelli <florian.fainelli@broadcom.com>
Cc: Broadcom internal kernel review list <bcm-kernel-feedback-list@broadcom.com>
Cc: Mike Marshall <hubcap@omnibond.com>
Cc: Martin Brandenburg <martin@omnibond.com>
Cc: Steven Rostedt <rostedt@goodmis.org>
Cc: Masami Hiramatsu <mhiramat@kernel.org>
Cc: Mathieu Desnoyers <mathieu.desnoyers@efficios.com>
Cc: Andrew Morton <akpm@linux-foundation.org>
Cc: Masahiro Yamada <masahiroy@kernel.org>
Cc: Nathan Chancellor <nathan@kernel.org>
Cc: Nicolas Schier <nicolas@fjasle.eu>
Cc: Arnd Bergmann <arnd@arndb.de>
Cc: Kees Cook <keescook@chromium.org>
Cc: Alexey Starikovskiy <astarikovskiy@suse.de>
Cc: linux-ntfs-dev@lists.sourceforge.net
Cc: linux-block@vger.kernel.org
Cc: linux-kernel@vger.kernel.org
Cc: linux-acpi@vger.kernel.org
Cc: acpica-devel@lists.linux.dev
Cc: linux-scsi@vger.kernel.org
Cc: greybus-dev@lists.linaro.org
Cc: linux-staging@lists.linux.dev
Cc: linux-rpi-kernel@lists.infradead.org
Cc: linux-arm-kernel@lists.infradead.org
Cc: devel@lists.orangefs.org
Cc: linux-trace-kernel@vger.kernel.org
Cc: linux-kbuild@vger.kernel.org

[PATCH 06/11] acpi: avoid warning for truncated string copy

[Arnd Bergmann]

From: Arnd Bergmann <arnd@arndb.de>

gcc -Wstringop-truncation warns about copying a string that results in a
missing nul termination:

drivers/acpi/acpica/tbfind.c: In function 'acpi_tb_find_table':
drivers/acpi/acpica/tbfind.c:60:9: error: 'strncpy' specified bound 6 equals destination size [-Werror=stringop-truncation]
   60 |         strncpy(header.oem_id, oem_id, ACPI_OEM_ID_SIZE);
      |         ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
drivers/acpi/acpica/tbfind.c:61:9: error: 'strncpy' specified bound 8 equals destination size [-Werror=stringop-truncation]
   61 |         strncpy(header.oem_table_id, oem_table_id, ACPI_OEM_TABLE_ID_SIZE);
      |         ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

This one is intentional, so rewrite the code in a way that avoids the
warning. Since there is already an extra strlen() and an overflow check,
the actual size to be copied is already known here.

Fixes: 47c08729bf1c ("ACPICA: Fix for LoadTable operator, input strings")
Signed-off-by: Arnd Bergmann <arnd@arndb.de>
---
 drivers/acpi/acpica/tbfind.c | 19 +++++++++----------
 1 file changed, 9 insertions(+), 10 deletions(-)

diff --git a/drivers/acpi/acpica/tbfind.c b/drivers/acpi/acpica/tbfind.c
index 1c1b2e284bd9..472ce2a6624b 100644
--- a/drivers/acpi/acpica/tbfind.c
+++ b/drivers/acpi/acpica/tbfind.c
@@ -36,7 +36,7 @@ acpi_tb_find_table(char *signature,
 {
 	acpi_status status = AE_OK;
 	struct acpi_table_header header;
-	u32 i;
+	u32 len, i;
 
 	ACPI_FUNCTION_TRACE(tb_find_table);
 
@@ -46,19 +46,18 @@ acpi_tb_find_table(char *signature,
 		return_ACPI_STATUS(AE_BAD_SIGNATURE);
 	}
 
-	/* Don't allow the OEM strings to be too long */
-
-	if ((strlen(oem_id) > ACPI_OEM_ID_SIZE) ||
-	    (strlen(oem_table_id) > ACPI_OEM_TABLE_ID_SIZE)) {
-		return_ACPI_STATUS(AE_AML_STRING_LIMIT);
-	}
-
 	/* Normalize the input strings */
 
 	memset(&header, 0, sizeof(struct acpi_table_header));
 	ACPI_COPY_NAMESEG(header.signature, signature);
-	strncpy(header.oem_id, oem_id, ACPI_OEM_ID_SIZE);
-	strncpy(header.oem_table_id, oem_table_id, ACPI_OEM_TABLE_ID_SIZE);
+	len = strlen(oem_id);
+	if (len > ACPI_OEM_ID_SIZE)
+		return_ACPI_STATUS(AE_AML_STRING_LIMIT);
+	memcpy(header.oem_id, oem_id, len);
+	len = strlen(oem_table_id);
+	if (len > ACPI_OEM_TABLE_ID_SIZE)
+		return_ACPI_STATUS(AE_AML_STRING_LIMIT);
+	memcpy(header.oem_table_id, oem_table_id, len);
 
 	/* Search for the table */
 
-- 
2.39.2

Re: [PATCH 06/11] acpi: avoid warning for truncated string copy

[Justin Stitt]

Hi,

On Thu, Mar 28, 2024 at 03:04:50PM +0100, Arnd Bergmann wrote:
> From: Arnd Bergmann <arnd@arndb.de>
> 
> gcc -Wstringop-truncation warns about copying a string that results in a
> missing nul termination:
> 
> drivers/acpi/acpica/tbfind.c: In function 'acpi_tb_find_table':
> drivers/acpi/acpica/tbfind.c:60:9: error: 'strncpy' specified bound 6 equals destination size [-Werror=stringop-truncation]
>    60 |         strncpy(header.oem_id, oem_id, ACPI_OEM_ID_SIZE);
>       |         ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> drivers/acpi/acpica/tbfind.c:61:9: error: 'strncpy' specified bound 8 equals destination size [-Werror=stringop-truncation]
>    61 |         strncpy(header.oem_table_id, oem_table_id, ACPI_OEM_TABLE_ID_SIZE);
>       |         ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> 
> This one is intentional, so rewrite the code in a way that avoids the
> warning. Since there is already an extra strlen() and an overflow check,
> the actual size to be copied is already known here.
>
I also tried cleaning these up but Kees informed me that this subsystem
is maintained elsewhere:

https://lore.kernel.org/all/202308241612.DFE4119@keescook/

I am not sure if you can get changes through by the traditional means.

> 
> Fixes: 47c08729bf1c ("ACPICA: Fix for LoadTable operator, input strings")
> Signed-off-by: Arnd Bergmann <arnd@arndb.de>
> ---
>  drivers/acpi/acpica/tbfind.c | 19 +++++++++----------
>  1 file changed, 9 insertions(+), 10 deletions(-)
> 
> diff --git a/drivers/acpi/acpica/tbfind.c b/drivers/acpi/acpica/tbfind.c
> index 1c1b2e284bd9..472ce2a6624b 100644
> --- a/drivers/acpi/acpica/tbfind.c
> +++ b/drivers/acpi/acpica/tbfind.c
> @@ -36,7 +36,7 @@ acpi_tb_find_table(char *signature,
>  {
>  	acpi_status status = AE_OK;
>  	struct acpi_table_header header;
> -	u32 i;
> +	u32 len, i;
>  
>  	ACPI_FUNCTION_TRACE(tb_find_table);
>  
> @@ -46,19 +46,18 @@ acpi_tb_find_table(char *signature,
>  		return_ACPI_STATUS(AE_BAD_SIGNATURE);
>  	}
>  
> -	/* Don't allow the OEM strings to be too long */
> -
> -	if ((strlen(oem_id) > ACPI_OEM_ID_SIZE) ||
> -	    (strlen(oem_table_id) > ACPI_OEM_TABLE_ID_SIZE)) {
> -		return_ACPI_STATUS(AE_AML_STRING_LIMIT);
> -	}
> -
>  	/* Normalize the input strings */
>  
>  	memset(&header, 0, sizeof(struct acpi_table_header));
>  	ACPI_COPY_NAMESEG(header.signature, signature);
> -	strncpy(header.oem_id, oem_id, ACPI_OEM_ID_SIZE);
> -	strncpy(header.oem_table_id, oem_table_id, ACPI_OEM_TABLE_ID_SIZE);
> +	len = strlen(oem_id);
> +	if (len > ACPI_OEM_ID_SIZE)
> +		return_ACPI_STATUS(AE_AML_STRING_LIMIT);
> +	memcpy(header.oem_id, oem_id, len);
> +	len = strlen(oem_table_id);
> +	if (len > ACPI_OEM_TABLE_ID_SIZE)
> +		return_ACPI_STATUS(AE_AML_STRING_LIMIT);
> +	memcpy(header.oem_table_id, oem_table_id, len);
>  
>  	/* Search for the table */
>  
> -- 
> 2.39.2
> 
Thanks
Justin

Re: [PATCH 06/11] acpi: avoid warning for truncated string copy

[Rafael J. Wysocki]

On Thu, Mar 28, 2024 at 3:06 PM Arnd Bergmann <arnd@kernel.org> wrote:
>
> From: Arnd Bergmann <arnd@arndb.de>
>
> gcc -Wstringop-truncation warns about copying a string that results in a
> missing nul termination:
>
> drivers/acpi/acpica/tbfind.c: In function 'acpi_tb_find_table':
> drivers/acpi/acpica/tbfind.c:60:9: error: 'strncpy' specified bound 6 equals destination size [-Werror=stringop-truncation]
>    60 |         strncpy(header.oem_id, oem_id, ACPI_OEM_ID_SIZE);
>       |         ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> drivers/acpi/acpica/tbfind.c:61:9: error: 'strncpy' specified bound 8 equals destination size [-Werror=stringop-truncation]
>    61 |         strncpy(header.oem_table_id, oem_table_id, ACPI_OEM_TABLE_ID_SIZE);
>       |         ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
>
> This one is intentional, so rewrite the code in a way that avoids the
> warning. Since there is already an extra strlen() and an overflow check,
> the actual size to be copied is already known here.
>
> Fixes: 47c08729bf1c ("ACPICA: Fix for LoadTable operator, input strings")
> Signed-off-by: Arnd Bergmann <arnd@arndb.de>

Because ACPICA is an external project supplying code to the Linux
kernel, the way to change the ACPICA code in the kernel is to submit a
pull request to the upstream ACPICA project on GitHub and once that PR
has been merged, submit a Linux patch corresponding to it including
the Link: tag pointing to the PR in question and the git ID of the
corresponding upstream ACPICA commit.

However, note that upstream ACPICA changes are applied to the Linux
kernel source code every time the upstream ACPICA project makes a
release, so it is not necessary to send the corresponding Linux
patches for them unless in the cases when timing matters.

> ---
>  drivers/acpi/acpica/tbfind.c | 19 +++++++++----------
>  1 file changed, 9 insertions(+), 10 deletions(-)
>
> diff --git a/drivers/acpi/acpica/tbfind.c b/drivers/acpi/acpica/tbfind.c
> index 1c1b2e284bd9..472ce2a6624b 100644
> --- a/drivers/acpi/acpica/tbfind.c
> +++ b/drivers/acpi/acpica/tbfind.c
> @@ -36,7 +36,7 @@ acpi_tb_find_table(char *signature,
>  {
>         acpi_status status = AE_OK;
>         struct acpi_table_header header;
> -       u32 i;
> +       u32 len, i;
>
>         ACPI_FUNCTION_TRACE(tb_find_table);
>
> @@ -46,19 +46,18 @@ acpi_tb_find_table(char *signature,
>                 return_ACPI_STATUS(AE_BAD_SIGNATURE);
>         }
>
> -       /* Don't allow the OEM strings to be too long */
> -
> -       if ((strlen(oem_id) > ACPI_OEM_ID_SIZE) ||
> -           (strlen(oem_table_id) > ACPI_OEM_TABLE_ID_SIZE)) {
> -               return_ACPI_STATUS(AE_AML_STRING_LIMIT);
> -       }
> -
>         /* Normalize the input strings */
>
>         memset(&header, 0, sizeof(struct acpi_table_header));
>         ACPI_COPY_NAMESEG(header.signature, signature);
> -       strncpy(header.oem_id, oem_id, ACPI_OEM_ID_SIZE);
> -       strncpy(header.oem_table_id, oem_table_id, ACPI_OEM_TABLE_ID_SIZE);
> +       len = strlen(oem_id);
> +       if (len > ACPI_OEM_ID_SIZE)
> +               return_ACPI_STATUS(AE_AML_STRING_LIMIT);
> +       memcpy(header.oem_id, oem_id, len);
> +       len = strlen(oem_table_id);
> +       if (len > ACPI_OEM_TABLE_ID_SIZE)
> +               return_ACPI_STATUS(AE_AML_STRING_LIMIT);
> +       memcpy(header.oem_table_id, oem_table_id, len);
>
>         /* Search for the table */
>
> --
> 2.39.2
>
>