linux-kernel - Re: [PATCH bpf-next v1 1/2] bpftool: Add -Wformat-signedness flag to detect format errors

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: Windows password security audit tool. GUI, reports in PDF.

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

Message-ID: <821170d2c5d570ca7961e7f91169110dcce5449c@linux.dev>
Date: Tue, 11 Mar 2025 02:41:07 +0000
From: "Jiayuan Chen" <jiayuan.chen@...ux.dev>
To: "Quentin Monnet" <qmo@...nel.org>, bpf@...r.kernel.org
Cc: daniel@...earbox.net, linux-kernel@...r.kernel.org, ast@...nel.org,
 davem@...emloft.net, kuba@...nel.org, hawk@...nel.org,
 john.fastabend@...il.com, andrii@...nel.org, martin.lau@...ux.dev,
 eddyz87@...il.com, song@...nel.org, yonghong.song@...ux.dev,
 kpsingh@...nel.org, sdf@...ichev.me, haoluo@...gle.com, jolsa@...nel.org,
 mrpre@....com, "Arnaldo Carvalho de Melo" <acme@...nel.org>
Subject: Re: [PATCH bpf-next v1 1/2] bpftool: Add -Wformat-signedness flag to
 detect format errors

March 10, 2025 at 23:49, "Quentin Monnet" <qmo@...nel.org> wrote:

> 
> 2025-03-10 22:20 UTC+0800 ~ Jiayuan Chen <jiayuan.chen@...ux.dev>
> 
> > 
> > This commit adds the -Wformat-signedness compiler flag to detect and
> > 
> >  prevent printf format errors, where signed or unsigned types are
> > 
> >  mismatched with format specifiers. This helps to catch potential issues at
> > 
> >  compile-time, ensuring that our code is more robust and reliable. With
> > 
> >  this flag, the compiler will now warn about incorrect format strings, such
> > 
> >  as using %d with unsigned types or %u with signed types.
> > 
> >  
> > 
> >  Signed-off-by: Jiayuan Chen <jiayuan.chen@...ux.dev>
> > 
> 
> Acked-by: Quentin Monnet <qmo@...nel.org>
> 
> Thanks for that. Have you looked into enabling the flag along with the
> 
> other EXTRA_WARNINGS in tools/scripts/Makefile.include? It would be
> 
> ideal to have it there, but I suppose it raises too many warnings across
> 
> tools/? (I didn't try myself.) No objection to taking it in bpftool only.
> 

Thanks for reminding me of these.

I tried adding it to Makefile.include, and indeed there were a lot of warnings,
but this part is not part of bpftool. I'll take a look later and try to enable this
options to fix these warnings as well. For now, I'll only fix the warnings related
to bpftool and these changes can also be easily synced to the bpftool on github.

> > ---
> > 
> >  tools/bpf/bpftool/Makefile | 2 +-
> > 
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> > 
> >  
> > 
> >  diff --git a/tools/bpf/bpftool/Makefile b/tools/bpf/bpftool/Makefile
> > 
> >  index dd9f3ec84201..d9f3eb51a48f 100644
> > 
> >  --- a/tools/bpf/bpftool/Makefile
> > 
> >  +++ b/tools/bpf/bpftool/Makefile
> > 
> >  @@ -71,7 +71,7 @@ prefix ?= /usr/local
> > 
> >  bash_compdir ?= /usr/share/bash-completion/completions
> > 
> >  
> > 
> >  CFLAGS += -O2
> > 
> >  -CFLAGS += -W -Wall -Wextra -Wno-unused-parameter -Wno-missing-field-initializers
> > 
> >  +CFLAGS += -W -Wall -Wextra -Wformat-signedness -Wno-unused-parameter -Wno-missing-field-initializers
> > 
> 
> Nit: This line is becoming long enough that I'd consider moving each
> 
> flag to its own line, for better reading:
> 
>  CFLAGS += -W
> 
>  CFLAGS += -Wall
> 
>  CFLAGS += -Wextra
> 
>  CFLAGS += -Wformat-signedness
> 
>  ...
OK，I will do this.