[PATCH 0/2] objtool: Add option to fail build on vmlinux warnings

[PATCH 0/2] objtool: Add option to fail build on vmlinux warnings

[Brendan Jackman]

This adds an option to objtool to exit with an error when it enounters
warnings.

Then, it adds a config to enable that flag on vmlinux. This enables you
to fail the build e.g. when noinstr is violated.

Signed-off-by: Brendan Jackman <jackmanb@google.com>
---
Brendan Jackman (2):
      objtool: Add --fail-on-warn
      kbuild: Add option to fail build on vmlinux objtool issues

 lib/Kconfig.debug                       | 11 +++++++++++
 scripts/Makefile.vmlinux_o              |  1 +
 tools/objtool/builtin-check.c           |  6 ++++++
 tools/objtool/check.c                   |  7 ++-----
 tools/objtool/include/objtool/builtin.h |  1 +
 5 files changed, 21 insertions(+), 5 deletions(-)
---
base-commit: f932fb9b40749d1c9a539d89bb3e288c077aafe5
change-id: 20241213-objtool-strict-cb9a0a75139e

Best regards,
-- 
Brendan Jackman <jackmanb@google.com>

[PATCH 1/2] objtool: Add --fail-on-warn

[Brendan Jackman]

At present objtool only prints to the terminal when observing "fatal
warnings". This option lets you have it produce an error instead.

My use case for this is noinstr validation; so far I've never seen any
false warnings here, but it quite often detects real bugs. I'd like my
build to fail when I have those bugs.

Signed-off-by: Brendan Jackman <jackmanb@google.com>
---
 tools/objtool/builtin-check.c           | 6 ++++++
 tools/objtool/check.c                   | 7 ++-----
 tools/objtool/include/objtool/builtin.h | 1 +
 3 files changed, 9 insertions(+), 5 deletions(-)

diff --git a/tools/objtool/builtin-check.c b/tools/objtool/builtin-check.c
index 387d56a7f5fb8da8435d0a3f5c05eeee66932c9b..dd70cbb98929b7f558c27766bda46ad276c0750d 100644
--- a/tools/objtool/builtin-check.c
+++ b/tools/objtool/builtin-check.c
@@ -94,6 +94,12 @@ static const struct option check_options[] = {
 	OPT_BOOLEAN(0, "sec-address", &opts.sec_address, "print section addresses in warnings"),
 	OPT_BOOLEAN(0, "stats", &opts.stats, "print statistics"),
 	OPT_BOOLEAN('v', "verbose", &opts.verbose, "verbose warnings"),
+	/*
+	 *  For now, don't fail the kernel build on fatal warnings by default.
+	 *  These errors are still fairly common due to the growing matrix of
+	 *  supported toolchains and their recent pace of change.
+	 */
+	OPT_BOOLEAN(0, "fail-on-warn", &opts.fail_on_warn, "fail on fatal warnings"),
 
 	OPT_END(),
 };
diff --git a/tools/objtool/check.c b/tools/objtool/check.c
index 4ce176ad411fb12a10101bbedbb6180275941b4b..266896b46e92c5c4a3244aa73deb3a355e6d8f8d 100644
--- a/tools/objtool/check.c
+++ b/tools/objtool/check.c
@@ -4941,10 +4941,7 @@ int check(struct objtool_file *file)
 	}
 
 out:
-	/*
-	 *  For now, don't fail the kernel build on fatal warnings.  These
-	 *  errors are still fairly common due to the growing matrix of
-	 *  supported toolchains and their recent pace of change.
-	 */
+	if (opts.fail_on_warn && warnings)
+		return 1;
 	return 0;
 }
diff --git a/tools/objtool/include/objtool/builtin.h b/tools/objtool/include/objtool/builtin.h
index fcca6662c8b4b5e0048e54fada8694cc2e6ebc34..f9af81ad9f600044280085cd1a743609ce054a21 100644
--- a/tools/objtool/include/objtool/builtin.h
+++ b/tools/objtool/include/objtool/builtin.h
@@ -38,6 +38,7 @@ struct opts {
 	bool sec_address;
 	bool stats;
 	bool verbose;
+	bool fail_on_warn;
 };
 
 extern struct opts opts;

-- 
2.47.1.613.gc27f4b7a9f-goog

Re: [PATCH 1/2] objtool: Add --fail-on-warn

[Josh Poimboeuf]

On Fri, Dec 13, 2024 at 11:31:30AM +0000, Brendan Jackman wrote:
> At present objtool only prints to the terminal when observing "fatal
> warnings". This option lets you have it produce an error instead.
> 
> My use case for this is noinstr validation; so far I've never seen any
> false warnings here, but it quite often detects real bugs. I'd like my
> build to fail when I have those bugs.
> 
> Signed-off-by: Brendan Jackman <jackmanb@google.com>
> ---
>  tools/objtool/builtin-check.c           | 6 ++++++
>  tools/objtool/check.c                   | 7 ++-----
>  tools/objtool/include/objtool/builtin.h | 1 +
>  3 files changed, 9 insertions(+), 5 deletions(-)
> 
> diff --git a/tools/objtool/builtin-check.c b/tools/objtool/builtin-check.c
> index 387d56a7f5fb8da8435d0a3f5c05eeee66932c9b..dd70cbb98929b7f558c27766bda46ad276c0750d 100644
> --- a/tools/objtool/builtin-check.c
> +++ b/tools/objtool/builtin-check.c
> @@ -94,6 +94,12 @@ static const struct option check_options[] = {
>  	OPT_BOOLEAN(0, "sec-address", &opts.sec_address, "print section addresses in warnings"),
>  	OPT_BOOLEAN(0, "stats", &opts.stats, "print statistics"),
>  	OPT_BOOLEAN('v', "verbose", &opts.verbose, "verbose warnings"),
> +	/*
> +	 *  For now, don't fail the kernel build on fatal warnings by default.
> +	 *  These errors are still fairly common due to the growing matrix of
> +	 *  supported toolchains and their recent pace of change.
> +	 */
> +	OPT_BOOLEAN(0, "fail-on-warn", &opts.fail_on_warn, "fail on fatal warnings"),

How about "--Werror" to mirror the compiler -Werror option.

-- 
Josh

Re: [PATCH 1/2] objtool: Add --fail-on-warn

[Brendan Jackman]

On Sat, 14 Dec 2024 at 01:36, Josh Poimboeuf <jpoimboe@kernel.org> wrote:
> How about "--Werror" to mirror the compiler -Werror option.

Sure, --Werror sounds good to me.

[PATCH 2/2] kbuild: Add option to fail build on vmlinux objtool issues

[Brendan Jackman]

NOINSTR_VALIDATION is pretty helpful for detecting bugs, I would like
my build to fail when those bugs arise.

If we wanted to we could enable this for individual warnings, it seems
unlikely there's a use-case for that though. So for now I've just added
a global setting for vmlinux.

Signed-off-by: Brendan Jackman <jackmanb@google.com>
---
 lib/Kconfig.debug          | 11 +++++++++++
 scripts/Makefile.vmlinux_o |  1 +
 2 files changed, 12 insertions(+)

diff --git a/lib/Kconfig.debug b/lib/Kconfig.debug
index f3d72370587936fa373129cc9b246f15dac907be..b1f0f8c83b050d4112428e0d8dece059ebf8dcd2 100644
--- a/lib/Kconfig.debug
+++ b/lib/Kconfig.debug
@@ -563,6 +563,17 @@ config NOINSTR_VALIDATION
 	select OBJTOOL
 	default y
 
+config VMLINUX_OBJTOOL_STRICT
+	bool "Strict objtool on vmlinux"
+	default n
+	# Conditions when we run objtool on vmlinux
+	depends on NOINSTR_VALIDATION || LTO_CLANG || X86_KERNEL_IBT
+	help
+	  Fail the build when objtool produces warnings on vmlinux.
+
+	  By default, objtool just prints warnings to the terminal without
+	  causing a build failure. This config changes that for vmlinux.
+
 config VMLINUX_MAP
 	bool "Generate vmlinux.map file when linking"
 	depends on EXPERT
diff --git a/scripts/Makefile.vmlinux_o b/scripts/Makefile.vmlinux_o
index 0b6e2ebf60dc1bb69d9651d5b7858ccd296e92dd..97b6b262d482e0bac1a4d74f9a2e7b1867b6ee00 100644
--- a/scripts/Makefile.vmlinux_o
+++ b/scripts/Makefile.vmlinux_o
@@ -39,6 +39,7 @@ vmlinux-objtool-args-$(delay-objtool)			+= $(objtool-args-y)
 vmlinux-objtool-args-$(CONFIG_GCOV_KERNEL)		+= --no-unreachable
 vmlinux-objtool-args-$(CONFIG_NOINSTR_VALIDATION)	+= --noinstr \
 							   $(if $(or $(CONFIG_MITIGATION_UNRET_ENTRY),$(CONFIG_MITIGATION_SRSO)), --unret)
+vmlinux-objtool-args-$(CONFIG_VMLINUX_OBJTOOL_STRICT)	+= --fail-on-warn
 
 objtool-args = $(vmlinux-objtool-args-y) --link
 

-- 
2.47.1.613.gc27f4b7a9f-goog

Re: [PATCH 2/2] kbuild: Add option to fail build on vmlinux objtool issues

[Josh Poimboeuf]

On Fri, Dec 13, 2024 at 11:31:31AM +0000, Brendan Jackman wrote:
> NOINSTR_VALIDATION is pretty helpful for detecting bugs, I would like
> my build to fail when those bugs arise.
> 
> If we wanted to we could enable this for individual warnings, it seems
> unlikely there's a use-case for that though. So for now I've just added
> a global setting for vmlinux.
> 
> Signed-off-by: Brendan Jackman <jackmanb@google.com>

Note that *any* objtool warning has a good change of being a major bug
in the kernel or compiler which could result in crashing the kernel or
breaking the livepatch consistency model.  So the option shouldn't be
restricted to noinstr builds only.  In which case it should be called
CONFIG_OBJTOOL_WERROR, analagous to CONFIG_WERROR.

We definitely need this, though it will likely break a lot of robot
builds, so it wouldn't be a good idea to merge it until after the
holidays.  Though once the patches are ready I could throw it on a git
branch to see how bad the robot breakage is.

-- 
Josh

Re: [PATCH 2/2] kbuild: Add option to fail build on vmlinux objtool issues

[Brendan Jackman]

On Sat, 14 Dec 2024 at 01:52, Josh Poimboeuf <jpoimboe@kernel.org> wrote:
> Note that *any* objtool warning has a good change of being a major bug
> in the kernel or compiler which could result in crashing the kernel or
> breaking the livepatch consistency model.  So the option shouldn't be
> restricted to noinstr builds only.  In which case it should be called
> CONFIG_OBJTOOL_WERROR, analagous to CONFIG_WERROR.

Sure, sounds good too.

Just to make sure I'm on the same page - are you saying I should add
the flag to $(objtool-args-y) instead of $(vmlinux-objtool-args-y)?

Re: [PATCH 2/2] kbuild: Add option to fail build on vmlinux objtool issues

[Josh Poimboeuf]

On Mon, Dec 16, 2024 at 11:00:58AM +0100, Brendan Jackman wrote:
> On Sat, 14 Dec 2024 at 01:52, Josh Poimboeuf <jpoimboe@kernel.org> wrote:
> > Note that *any* objtool warning has a good change of being a major bug
> > in the kernel or compiler which could result in crashing the kernel or
> > breaking the livepatch consistency model.  So the option shouldn't be
> > restricted to noinstr builds only.  In which case it should be called
> > CONFIG_OBJTOOL_WERROR, analagous to CONFIG_WERROR.
> 
> Sure, sounds good too.
> 
> Just to make sure I'm on the same page - are you saying I should add
> the flag to $(objtool-args-y) instead of $(vmlinux-objtool-args-y)?

Yeah, that should do it.

-- 
Josh