1. Patchew
  2. linux
  3. [PATCH 0/9] enabled -Wformat-truncation for clang
  4. View patch

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

Arnd Bergmann posted 9 patches 1 year, 10 months ago
[PATCH 5/9] surface3_power: avoid format string truncation warning
Posted by Arnd Bergmann 1 year, 10 months ago 
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
Posted by Uwe Kleine-König 1 year, 10 months ago 
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/ |
Re: [PATCH 5/9] surface3_power: avoid format string truncation warning
Posted by Nathan Chancellor 1 year, 10 months ago 
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 5/9] surface3_power: avoid format string truncation warning
Posted by Andy Shevchenko 1 year, 10 months ago 
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

Patchew 2.1-devel-b67e4c2

© 2016 - 2026 Red Hat, Inc.