[PATCH ethtool] netlink: fix print_string when the value is NULL

[PATCH ethtool] netlink: fix print_string when the value is NULL

[Michel Lind]

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

The previous fix in commit b70c92866102 ("netlink: fix missing headers
in text output") handles the case when value is NULL by still using
`fprintf` but passing no value.

This fails if `-Werror=format-security` is passed to gcc, as is the
default in distros like Fedora.

```
json_print.c: In function 'print_string':
json_print.c:147:25: error: format not a string literal and no format arguments [-Werror=format-security]
  147 |                         fprintf(stdout, fmt);
      |
```

Use `fprintf(stdout, "%s", fmt)` instead, using the format string as the
value, since in this case we know it is just a string without format
chracters.

Reviewed-by: Jakub Kicinski <kuba@kernel.org>
Signed-off-by: Michel Lind <michel@michel-slm.name>
---
 json_print.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/json_print.c b/json_print.c
index e07c651..75e6cd9 100644
--- a/json_print.c
+++ b/json_print.c
@@ -144,7 +144,7 @@ void print_string(enum output_type type,
 		if (value)
 			fprintf(stdout, fmt, value);
 		else
-			fprintf(stdout, fmt);
+			fprintf(stdout, "%s", fmt);
 	}
 }
 
-- 
2.50.1


-- 
 _o) Michel Lind
_( ) identities: https://keyoxide.org/5dce2e7e9c3b1cffd335c1d78b229d2f7ccc04f2
     README:     https://fedoraproject.org/wiki/User:Salimma#README

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

Re: [PATCH ethtool] netlink: fix print_string when the value is NULL

[Michal Kubecek]

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

On Thu, Jul 24, 2025 at 07:48:11PM GMT, Michel Lind wrote:
> The previous fix in commit b70c92866102 ("netlink: fix missing headers
> in text output") handles the case when value is NULL by still using
> `fprintf` but passing no value.
> 
> This fails if `-Werror=format-security` is passed to gcc, as is the
> default in distros like Fedora.
> 
> ```
> json_print.c: In function 'print_string':
> json_print.c:147:25: error: format not a string literal and no format arguments [-Werror=format-security]
>   147 |                         fprintf(stdout, fmt);
>       |
> ```
> 
> Use `fprintf(stdout, "%s", fmt)` instead, using the format string as the
> value, since in this case we know it is just a string without format
> chracters.
> 
> Reviewed-by: Jakub Kicinski <kuba@kernel.org>
> Signed-off-by: Michel Lind <michel@michel-slm.name>

Applied, thank you.

It's a bit surprising that I didn't hit this problem as I always test
building with "-Wall -Wextra -Werror". I suppose this option is not
contained in -Wall or -Wextra.

Michal

> ---
>  json_print.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/json_print.c b/json_print.c
> index e07c651..75e6cd9 100644
> --- a/json_print.c
> +++ b/json_print.c
> @@ -144,7 +144,7 @@ void print_string(enum output_type type,
>  		if (value)
>  			fprintf(stdout, fmt, value);
>  		else
> -			fprintf(stdout, fmt);
> +			fprintf(stdout, "%s", fmt);
>  	}
>  }
>  
> -- 
> 2.50.1
> 
> 
> -- 
>  _o) Michel Lind
> _( ) identities: https://keyoxide.org/5dce2e7e9c3b1cffd335c1d78b229d2f7ccc04f2
>      README:     https://fedoraproject.org/wiki/User:Salimma#README



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

Re: [PATCH ethtool] netlink: fix print_string when the value is NULL

[patchwork-bot+netdevbpf]

Hello:

This patch was applied to ethtool/ethtool.git (master)
by Michal Kubecek <mkubecek@suse.cz>:

On Thu, 24 Jul 2025 19:48:11 -0500 you wrote:
> The previous fix in commit b70c92866102 ("netlink: fix missing headers
> in text output") handles the case when value is NULL by still using
> `fprintf` but passing no value.
> 
> This fails if `-Werror=format-security` is passed to gcc, as is the
> default in distros like Fedora.
> 
> [...]

Here is the summary with links:
  - [ethtool] netlink: fix print_string when the value is NULL
    https://git.kernel.org/pub/scm/network/ethtool/ethtool.git/commit/?id=41d6105250c8

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

Re: [PATCH ethtool] netlink: fix print_string when the value is NULL

[Salvatore Bonaccorso]

Hi Michal,

On Fri, Aug 08, 2025 at 01:05:52AM +0200, Michal Kubecek wrote:
> On Thu, Jul 24, 2025 at 07:48:11PM GMT, Michel Lind wrote:
> > The previous fix in commit b70c92866102 ("netlink: fix missing headers
> > in text output") handles the case when value is NULL by still using
> > `fprintf` but passing no value.
> > 
> > This fails if `-Werror=format-security` is passed to gcc, as is the
> > default in distros like Fedora.
> > 
> > ```
> > json_print.c: In function 'print_string':
> > json_print.c:147:25: error: format not a string literal and no format arguments [-Werror=format-security]
> >   147 |                         fprintf(stdout, fmt);
> >       |
> > ```
> > 
> > Use `fprintf(stdout, "%s", fmt)` instead, using the format string as the
> > value, since in this case we know it is just a string without format
> > chracters.
> > 
> > Reviewed-by: Jakub Kicinski <kuba@kernel.org>
> > Signed-off-by: Michel Lind <michel@michel-slm.name>
> 
> Applied, thank you.
> 
> It's a bit surprising that I didn't hit this problem as I always test
> building with "-Wall -Wextra -Werror". I suppose this option is not
> contained in -Wall or -Wextra.
> 
> Michal
> 
> > ---
> >  json_print.c | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> > 
> > diff --git a/json_print.c b/json_print.c
> > index e07c651..75e6cd9 100644
> > --- a/json_print.c
> > +++ b/json_print.c
> > @@ -144,7 +144,7 @@ void print_string(enum output_type type,
> >  		if (value)
> >  			fprintf(stdout, fmt, value);
> >  		else
> > -			fprintf(stdout, fmt);
> > +			fprintf(stdout, "%s", fmt);
> >  	}
> >  }
> >  
> > -- 
> > 2.50.1

As b70c92866102 ("netlink: fix missing headers in text output") was
backported as well for the 6.14.2 version, should that get as well a
new release 6.14.3 with the fix?

Regards,
Salvatore

Re: [PATCH ethtool] netlink: fix print_string when the value is NULL

[Michal Kubecek]

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

Dne Sun, Aug 10, 2025 at 09:14:26AM GMT, Salvatore Bonaccorso napsal:
> Hi Michal,
> 
> On Fri, Aug 08, 2025 at 01:05:52AM +0200, Michal Kubecek wrote:
> > On Thu, Jul 24, 2025 at 07:48:11PM GMT, Michel Lind wrote:
> > > The previous fix in commit b70c92866102 ("netlink: fix missing headers
> > > in text output") handles the case when value is NULL by still using
> > > `fprintf` but passing no value.
> > > 
> > > This fails if `-Werror=format-security` is passed to gcc, as is the
> > > default in distros like Fedora.
> > > 
> > > ```
> > > json_print.c: In function 'print_string':
> > > json_print.c:147:25: error: format not a string literal and no format arguments [-Werror=format-security]
> > >   147 |                         fprintf(stdout, fmt);
> > >       |
> > > ```
> > > 
> > > Use `fprintf(stdout, "%s", fmt)` instead, using the format string as the
> > > value, since in this case we know it is just a string without format
> > > chracters.
> > > 
> > > Reviewed-by: Jakub Kicinski <kuba@kernel.org>
> > > Signed-off-by: Michel Lind <michel@michel-slm.name>
> > 
> > Applied, thank you.
> > 
> > It's a bit surprising that I didn't hit this problem as I always test
> > building with "-Wall -Wextra -Werror". I suppose this option is not
> > contained in -Wall or -Wextra.
> > 
> > Michal
> > 
> > > ---
> > >  json_print.c | 2 +-
> > >  1 file changed, 1 insertion(+), 1 deletion(-)
> > > 
> > > diff --git a/json_print.c b/json_print.c
> > > index e07c651..75e6cd9 100644
> > > --- a/json_print.c
> > > +++ b/json_print.c
> > > @@ -144,7 +144,7 @@ void print_string(enum output_type type,
> > >  		if (value)
> > >  			fprintf(stdout, fmt, value);
> > >  		else
> > > -			fprintf(stdout, fmt);
> > > +			fprintf(stdout, "%s", fmt);
> > >  	}
> > >  }
> > >  
> > > -- 
> > > 2.50.1
> 
> As b70c92866102 ("netlink: fix missing headers in text output") was
> backported as well for the 6.14.2 version, should that get as well a
> new release 6.14.3 with the fix?

I could do that but it didn't seem necessary. If I understand correctly,
this patch does not address any runtime issue (at least not until there
is an actual call of print_string() with null value and fmt containing
a template); and the build issue only happens with a very specific
compiler option which is not only not default but is not included even
in "-Wall -Wextra" (not even in gcc15).

I'm aware that the commit message says that Fedora uses that compiler
option in its package builds but that's something that can be addressed
by a distribution patch. Therefore my plan was to cherry pick the commit
into ethtool-6.14.y branch but not to release 6.14.3 unless something
more serious shows up.

But if I misunderstood the situation and 6.14.3 with this commit would
be really helpful, I can reconsider.

Michal

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

Re: [PATCH ethtool] netlink: fix print_string when the value is NULL

[Salvatore Bonaccorso]

Hi,

On Mon, Aug 11, 2025 at 10:07:00PM +0200, Michal Kubecek wrote:
> Dne Sun, Aug 10, 2025 at 09:14:26AM GMT, Salvatore Bonaccorso napsal:
> > Hi Michal,
> > 
> > On Fri, Aug 08, 2025 at 01:05:52AM +0200, Michal Kubecek wrote:
> > > On Thu, Jul 24, 2025 at 07:48:11PM GMT, Michel Lind wrote:
> > > > The previous fix in commit b70c92866102 ("netlink: fix missing headers
> > > > in text output") handles the case when value is NULL by still using
> > > > `fprintf` but passing no value.
> > > > 
> > > > This fails if `-Werror=format-security` is passed to gcc, as is the
> > > > default in distros like Fedora.
> > > > 
> > > > ```
> > > > json_print.c: In function 'print_string':
> > > > json_print.c:147:25: error: format not a string literal and no format arguments [-Werror=format-security]
> > > >   147 |                         fprintf(stdout, fmt);
> > > >       |
> > > > ```
> > > > 
> > > > Use `fprintf(stdout, "%s", fmt)` instead, using the format string as the
> > > > value, since in this case we know it is just a string without format
> > > > chracters.
> > > > 
> > > > Reviewed-by: Jakub Kicinski <kuba@kernel.org>
> > > > Signed-off-by: Michel Lind <michel@michel-slm.name>
> > > 
> > > Applied, thank you.
> > > 
> > > It's a bit surprising that I didn't hit this problem as I always test
> > > building with "-Wall -Wextra -Werror". I suppose this option is not
> > > contained in -Wall or -Wextra.
> > > 
> > > Michal
> > > 
> > > > ---
> > > >  json_print.c | 2 +-
> > > >  1 file changed, 1 insertion(+), 1 deletion(-)
> > > > 
> > > > diff --git a/json_print.c b/json_print.c
> > > > index e07c651..75e6cd9 100644
> > > > --- a/json_print.c
> > > > +++ b/json_print.c
> > > > @@ -144,7 +144,7 @@ void print_string(enum output_type type,
> > > >  		if (value)
> > > >  			fprintf(stdout, fmt, value);
> > > >  		else
> > > > -			fprintf(stdout, fmt);
> > > > +			fprintf(stdout, "%s", fmt);
> > > >  	}
> > > >  }
> > > >  
> > > > -- 
> > > > 2.50.1
> > 
> > As b70c92866102 ("netlink: fix missing headers in text output") was
> > backported as well for the 6.14.2 version, should that get as well a
> > new release 6.14.3 with the fix?
> 
> I could do that but it didn't seem necessary. If I understand correctly,
> this patch does not address any runtime issue (at least not until there
> is an actual call of print_string() with null value and fmt containing
> a template); and the build issue only happens with a very specific
> compiler option which is not only not default but is not included even
> in "-Wall -Wextra" (not even in gcc15).
> 
> I'm aware that the commit message says that Fedora uses that compiler
> option in its package builds but that's something that can be addressed
> by a distribution patch. Therefore my plan was to cherry pick the commit
> into ethtool-6.14.y branch but not to release 6.14.3 unless something
> more serious shows up.
> 
> But if I misunderstood the situation and 6.14.3 with this commit would
> be really helpful, I can reconsider.

No not urgent, but I hit the same issue when preparing 6.14.2 for
Debian trixie. But I can equally just cherry-pick the commit locally
and then drop it once 6.14.3 is released.

So really no hurry about that.

Regards,
Salvatore

Re: [PATCH ethtool] netlink: fix print_string when the value is NULL

[Michal Kubecek]

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

Dne Mon, Aug 11, 2025 at 10:23:38PM GMT, Salvatore Bonaccorso napsal:
> No not urgent, but I hit the same issue when preparing 6.14.2 for
> Debian trixie. But I can equally just cherry-pick the commit locally
> and then drop it once 6.14.3 is released.
> 
> So really no hurry about that.

OK, I'll release 6.14.3 on Friday when I'm back home and can do it from
my desktop which will be a bit easier.

Michal

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