[PATCH 0/9] enabled -Wformat-truncation for clang

[PATCH 0/9] enabled -Wformat-truncation for clang

[Arnd Bergmann]

From: Arnd Bergmann <arnd@arndb.de>

With randconfig build testing, I found only eight files that produce
warnings with clang when -Wformat-truncation is enabled. This means
we can just turn it on by default rather than only enabling it for
"make W=1".

Unfortunately, gcc produces a lot more warnings when the option
is enabled, so it's not yet possible to turn it on both both
compilers.

I hope that the patches can get picked up by platform maintainers
directly, so the final patch can go in later on.

     Arnd

Arnd Bergmann (9):
  fbdev: shmobile: fix snprintf truncation
  enetc: avoid truncating error message
  qed: avoid truncating work queue length
  mlx5: avoid truncating error message
  surface3_power: avoid format string truncation warning
  Input: IMS: fix printf string overflow
  scsi: mylex: fix sysfs buffer lengths
  ALSA: aoa: avoid false-positive format truncation warning
  kbuild: enable -Wformat-truncation on clang

 drivers/input/misc/ims-pcu.c                  |  4 ++--
 drivers/net/ethernet/freescale/enetc/enetc.c  |  2 +-
 .../ethernet/mellanox/mlx5/core/esw/bridge.c  |  2 +-
 drivers/net/ethernet/qlogic/qed/qed_main.c    |  9 ++++---
 drivers/platform/surface/surface3_power.c     |  2 +-
 drivers/scsi/myrb.c                           | 20 ++++++++--------
 drivers/scsi/myrs.c                           | 24 +++++++++----------
 drivers/video/fbdev/sh_mobile_lcdcfb.c        |  2 +-
 scripts/Makefile.extrawarn                    |  2 ++
 sound/aoa/soundbus/i2sbus/core.c              |  2 +-
 10 files changed, 35 insertions(+), 34 deletions(-)

-- 
2.39.2

Cc: Dmitry Torokhov <dmitry.torokhov@gmail.com>
Cc: Claudiu Manoil <claudiu.manoil@nxp.com>
Cc: Vladimir Oltean <vladimir.oltean@nxp.com>
Cc: Jakub Kicinski <kuba@kernel.org>
Cc: Saeed Mahameed <saeedm@nvidia.com>
Cc: Leon Romanovsky <leon@kernel.org>
Cc: Ariel Elior <aelior@marvell.com>
Cc: Manish Chopra <manishc@marvell.com>
Cc: Hans de Goede <hdegoede@redhat.com>
Cc: "Ilpo Järvinen" <ilpo.jarvinen@linux.intel.com>
Cc: Maximilian Luz <luzmaximilian@gmail.com>
Cc: Hannes Reinecke <hare@kernel.org>
Cc: "Martin K. Petersen" <martin.petersen@oracle.com>
Cc: Helge Deller <deller@gmx.de>
Cc: Masahiro Yamada <masahiroy@kernel.org>
Cc: Nathan Chancellor <nathan@kernel.org>
Cc: Nicolas Schier <nicolas@fjasle.eu>
Cc: Johannes Berg <johannes@sipsolutions.net>
Cc: Jaroslav Kysela <perex@perex.cz>
Cc: Takashi Iwai <tiwai@suse.com>
Cc: Nick Desaulniers <ndesaulniers@google.com>
Cc: Bill Wendling <morbo@google.com>
Cc: Justin Stitt <justinstitt@google.com>
Cc: linux-input@vger.kernel.org
Cc: linux-kernel@vger.kernel.org
Cc: netdev@vger.kernel.org
Cc: linux-rdma@vger.kernel.org
Cc: platform-driver-x86@vger.kernel.org
Cc: linux-scsi@vger.kernel.org
Cc: linux-fbdev@vger.kernel.org
Cc: dri-devel@lists.freedesktop.org
Cc: linux-kbuild@vger.kernel.org
Cc: linuxppc-dev@lists.ozlabs.org
Cc: alsa-devel@alsa-project.org
Cc: linux-sound@vger.kernel.org
Cc: llvm@lists.linux.dev

[PATCH 5/9] surface3_power: avoid format string truncation warning

[Arnd Bergmann]

From: Arnd Bergmann <arnd@arndb.de>

clang warns about printing a pair of escaped strings into a buffer that is
too short:

drivers/platform/surface/surface3_power.c:248:3: error: 'snprintf' will always be truncated; specified size is 10, but format string expands to at least 12 [-Werror,-Wformat-truncation-non-kprintf]
  248 |                 snprintf(bix->serial, ARRAY_SIZE(bix->serial), "%3pE%6pE", buf + 7, buf);
      |                 ^

Change the format string two print two less bytes so it always fits. The string
is still truncated, so there is no change in behavior, but the compiler no
longer warns about it.

Fixes: 85f7582cd484 ("platform/surface: Move Surface 3 Power OpRegion driver to platform/surface")
Signed-off-by: Arnd Bergmann <arnd@arndb.de>
---
Not entirely sure about this one, as I've never used escaped strings, and
don't know if gcc is correct to warn here, or if the kernel defines it
differently from the standard.
---
 drivers/platform/surface/surface3_power.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/platform/surface/surface3_power.c b/drivers/platform/surface/surface3_power.c
index 4c0f92562a79..72f904761fde 100644
--- a/drivers/platform/surface/surface3_power.c
+++ b/drivers/platform/surface/surface3_power.c
@@ -245,7 +245,7 @@ static int mshw0011_bix(struct mshw0011_data *cdata, struct bix *bix)
 		dev_err(&client->dev, "Error reading serial no: %d\n", ret);
 		return ret;
 	} else {
-		snprintf(bix->serial, ARRAY_SIZE(bix->serial), "%3pE%6pE", buf + 7, buf);
+		snprintf(bix->serial, ARRAY_SIZE(bix->serial), "%3pE%4pE", buf + 7, buf);
 	}
 
 	/* get cycle count */
-- 
2.39.2

Re: [PATCH 5/9] surface3_power: avoid format string truncation warning

[Nathan Chancellor]

On Tue, Mar 26, 2024 at 11:38:04PM +0100, Arnd Bergmann wrote:
> From: Arnd Bergmann <arnd@arndb.de>
> 
> clang warns about printing a pair of escaped strings into a buffer that is
> too short:
> 
> drivers/platform/surface/surface3_power.c:248:3: error: 'snprintf' will always be truncated; specified size is 10, but format string expands to at least 12 [-Werror,-Wformat-truncation-non-kprintf]
>   248 |                 snprintf(bix->serial, ARRAY_SIZE(bix->serial), "%3pE%6pE", buf + 7, buf);
>       |                 ^
> 
> Change the format string two print two less bytes so it always fits. The string
> is still truncated, so there is no change in behavior, but the compiler no
> longer warns about it.
> 
> Fixes: 85f7582cd484 ("platform/surface: Move Surface 3 Power OpRegion driver to platform/surface")
> Signed-off-by: Arnd Bergmann <arnd@arndb.de>
> ---
> Not entirely sure about this one, as I've never used escaped strings, and
> don't know if gcc is correct to warn here, or if the kernel defines it
> differently from the standard.

As far as I understand it, this is a false positive because clang does
not understand the kernel's %p extensions. GCC does not warn for
overflow or truncation when %p is involved but the clang developers
chose to intentionally deviate from GCC in that aspect while sticking it
under a separate diagnostic that we could disable. I sent a patch that
did so some time ago but I guess Masahiro never applied it...

https://lore.kernel.org/20231002-disable-wformat-truncation-overflow-non-kprintf-v1-1-35179205c8d9@kernel.org/

Consider dropping the changes that fix non-kprintf warnings and
including that patch as part of this series.

> ---
>  drivers/platform/surface/surface3_power.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/platform/surface/surface3_power.c b/drivers/platform/surface/surface3_power.c
> index 4c0f92562a79..72f904761fde 100644
> --- a/drivers/platform/surface/surface3_power.c
> +++ b/drivers/platform/surface/surface3_power.c
> @@ -245,7 +245,7 @@ static int mshw0011_bix(struct mshw0011_data *cdata, struct bix *bix)
>  		dev_err(&client->dev, "Error reading serial no: %d\n", ret);
>  		return ret;
>  	} else {
> -		snprintf(bix->serial, ARRAY_SIZE(bix->serial), "%3pE%6pE", buf + 7, buf);
> +		snprintf(bix->serial, ARRAY_SIZE(bix->serial), "%3pE%4pE", buf + 7, buf);
>  	}
>  
>  	/* get cycle count */
> -- 
> 2.39.2
> 

Re: [PATCH 0/9] enabled -Wformat-truncation for clang

[Jakub Kicinski]

On Tue, 26 Mar 2024 23:37:59 +0100 Arnd Bergmann wrote:
> I hope that the patches can get picked up by platform maintainers
> directly, so the final patch can go in later on.

platform == subsystem? :)

Re: [PATCH 5/9] surface3_power: avoid format string truncation warning

[Andy Shevchenko]

On Wed, Mar 27, 2024 at 1:05 AM Nathan Chancellor <nathan@kernel.org> wrote:
> On Tue, Mar 26, 2024 at 11:38:04PM +0100, Arnd Bergmann wrote:
> > From: Arnd Bergmann <arnd@arndb.de>
> >
> > clang warns about printing a pair of escaped strings into a buffer that is
> > too short:
> >
> > drivers/platform/surface/surface3_power.c:248:3: error: 'snprintf' will always be truncated; specified size is 10, but format string expands to at least 12 [-Werror,-Wformat-truncation-non-kprintf]
> >   248 |                 snprintf(bix->serial, ARRAY_SIZE(bix->serial), "%3pE%6pE", buf + 7, buf);
> >       |                 ^
> >
> > Change the format string two print two less bytes so it always fits. The string
> > is still truncated, so there is no change in behavior, but the compiler no
> > longer warns about it.

> > Fixes: 85f7582cd484 ("platform/surface: Move Surface 3 Power OpRegion driver to platform/surface")

Hmm... Is it really a move patch (which by title should not have
changed the contents)?
(I haven't looked into it, though.)

> > Signed-off-by: Arnd Bergmann <arnd@arndb.de>
> > ---
> > Not entirely sure about this one, as I've never used escaped strings, and
> > don't know if gcc is correct to warn here, or if the kernel defines it
> > differently from the standard.
>
> As far as I understand it, this is a false positive because clang does
> not understand the kernel's %p extensions.

Yes, %pE here is special. Btw, what has already been discussed a long
time is to have a validation plugin in the kernel to check those %p
extensions, but IIUC nobody committed to it.

That said, the patch is most likely incorrect.

> GCC does not warn for
> overflow or truncation when %p is involved but the clang developers
> chose to intentionally deviate from GCC in that aspect while sticking it
> under a separate diagnostic that we could disable. I sent a patch that
> did so some time ago but I guess Masahiro never applied it...
>
> https://lore.kernel.org/20231002-disable-wformat-truncation-overflow-non-kprintf-v1-1-35179205c8d9@kernel.org/
>
> Consider dropping the changes that fix non-kprintf warnings and
> including that patch as part of this series.

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH 0/9] enabled -Wformat-truncation for clang

[patchwork-bot+netdevbpf]

Hello:

This series was applied to netdev/net-next.git (main)
by Jakub Kicinski <kuba@kernel.org>:

On Tue, 26 Mar 2024 23:37:59 +0100 you wrote:
> From: Arnd Bergmann <arnd@arndb.de>
> 
> With randconfig build testing, I found only eight files that produce
> warnings with clang when -Wformat-truncation is enabled. This means
> we can just turn it on by default rather than only enabling it for
> "make W=1".
> 
> [...]

Here is the summary with links:
  - [2/9] enetc: avoid truncating error message
    https://git.kernel.org/netdev/net-next/c/9046d581ed58
  - [3/9] qed: avoid truncating work queue length
    https://git.kernel.org/netdev/net-next/c/954fd908f177
  - [4/9] mlx5: avoid truncating error message
    https://git.kernel.org/netdev/net-next/c/b324a960354b

You are awesome, thank you!
-- 
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html

Re: [PATCH 5/9] surface3_power: avoid format string truncation warning

[Uwe Kleine-König]

[-- Attachment #1: Type: text/plain, Size: 475 bytes --]

Hello,

just a nitpick:

On Tue, Mar 26, 2024 at 11:38:04PM +0100, Arnd Bergmann wrote:
> Change the format string two print two less bytes so it always fits. The string

s/two/to/

> is still truncated, so there is no change in behavior, but the compiler no
> longer warns about it.

Best regards
Uwe

-- 
Pengutronix e.K.                           | Uwe Kleine-König            |
Industrial Linux Solutions                 | https://www.pengutronix.de/ |

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

Re: (subset) [PATCH 0/9] enabled -Wformat-truncation for clang

[Martin K. Petersen]

On Tue, 26 Mar 2024 23:37:59 +0100, Arnd Bergmann wrote:

> With randconfig build testing, I found only eight files that produce
> warnings with clang when -Wformat-truncation is enabled. This means
> we can just turn it on by default rather than only enabling it for
> "make W=1".
> 
> Unfortunately, gcc produces a lot more warnings when the option
> is enabled, so it's not yet possible to turn it on both both
> compilers.
> 
> [...]

Applied to 6.9/scsi-fixes, thanks!

[7/9] scsi: mylex: fix sysfs buffer lengths
      https://git.kernel.org/mkp/scsi/c/1197c5b2099f

-- 
Martin K. Petersen	Oracle Linux Engineering