netdev - Re: [PATCH ethtool] netlink: fix print

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite for Android: free password hash cracker in your pocket

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <b6unuycjddzrl55q3gwtki2rmm2ituknbmgwpuorgten5xr65w@w4dnhvf6mkoa>
Date: Mon, 11 Aug 2025 22:07:00 +0200
From: Michal Kubecek <mkubecek@...e.cz>
To: Salvatore Bonaccorso <carnil@...ian.org>
Cc: Michel Lind <michel@...hel-slm.name>, netdev@...r.kernel.org, 
	Jakub Kicinski <kuba@...nel.org>
Subject: Re: [PATCH ethtool] netlink: fix print_string when the value is NULL

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@...nel.org>
> > > Signed-off-by: Michel Lind <michel@...hel-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

Download attachment "signature.asc" of type "application/pgp-signature" (489 bytes)