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] [thread-next>] [day] [month] [year] [list]

Message-ID: <654713e8-f4ce-4742-8165-f73838ea8d16@kernel.org>
Date: Mon, 10 Mar 2025 15:49:05 +0000
From: Quentin Monnet <qmo@...nel.org>
To: Jiayuan Chen <jiayuan.chen@...ux.dev>, 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

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.


> ---
>  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
	...

>  CFLAGS += $(filter-out -Wswitch-enum -Wnested-externs,$(EXTRA_WARNINGS))
>  CFLAGS += -DPACKAGE='"bpftool"' -D__EXPORTED_HEADERS__ \
>  	-I$(or $(OUTPUT),.) \