2023-12-13 13:43:35

by Dimitri John Ledkov

[permalink] [raw]
Subject: [PATCH v2 2/3] objtool: make objtool SLS validation fatal when building with CONFIG_SLS=y

Make objtool SLS validation fatal when building with CONFIG_SLS=y,
currently it is a build.log warning only.

This is a standalone patch, such that if regressions are identified
(with any config or toolchain configuration) it can be reverted until
relevant identified code is fixed up or otherwise
ignored/silecned/marked as safe.

Signed-off-by: Dimitri John Ledkov <[email protected]>
---
tools/objtool/check.c | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/tools/objtool/check.c b/tools/objtool/check.c
index 15df4afae2..9709f037f1 100644
--- a/tools/objtool/check.c
+++ b/tools/objtool/check.c
@@ -4500,7 +4500,9 @@ static int validate_sls(struct objtool_file *file)
}
}

- return warnings;
+ /* SLS is an optional security safety feature, make it fatal
+ * to ensure no new code is introduced that fails SLS */
+ return -warnings;
}

static bool ignore_noreturn_call(struct instruction *insn)
--
2.34.1


2024-01-08 09:19:58

by Ingo Molnar

[permalink] [raw]
Subject: Re: [PATCH v2 2/3] objtool: make objtool SLS validation fatal when building with CONFIG_SLS=y


* Dimitri John Ledkov <[email protected]> wrote:

> Make objtool SLS validation fatal when building with CONFIG_SLS=y,
> currently it is a build.log warning only.
>
> This is a standalone patch, such that if regressions are identified
> (with any config or toolchain configuration) it can be reverted until
> relevant identified code is fixed up or otherwise
> ignored/silecned/marked as safe.
>
> Signed-off-by: Dimitri John Ledkov <[email protected]>
> ---
> tools/objtool/check.c | 4 +++-
> 1 file changed, 3 insertions(+), 1 deletion(-)
>
> diff --git a/tools/objtool/check.c b/tools/objtool/check.c
> index 15df4afae2..9709f037f1 100644
> --- a/tools/objtool/check.c
> +++ b/tools/objtool/check.c
> @@ -4500,7 +4500,9 @@ static int validate_sls(struct objtool_file *file)
> }
> }
>
> - return warnings;
> + /* SLS is an optional security safety feature, make it fatal
> + * to ensure no new code is introduced that fails SLS */
> + return -warnings;

please use the customary (multi-line) comment style:

/*
* Comment .....
* ...... goes here.
*/

specified in Documentation/process/coding-style.rst.

Thanks,

Ingo

2024-01-09 19:32:00

by Josh Poimboeuf

[permalink] [raw]
Subject: Re: [PATCH v2 2/3] objtool: make objtool SLS validation fatal when building with CONFIG_SLS=y

On Wed, Dec 13, 2023 at 01:43:01PM +0000, Dimitri John Ledkov wrote:
> Make objtool SLS validation fatal when building with CONFIG_SLS=y,
> currently it is a build.log warning only.
>
> This is a standalone patch, such that if regressions are identified
> (with any config or toolchain configuration) it can be reverted until
> relevant identified code is fixed up or otherwise
> ignored/silecned/marked as safe.
>
> Signed-off-by: Dimitri John Ledkov <[email protected]>
> ---
> tools/objtool/check.c | 4 +++-
> 1 file changed, 3 insertions(+), 1 deletion(-)
>
> diff --git a/tools/objtool/check.c b/tools/objtool/check.c
> index 15df4afae2..9709f037f1 100644
> --- a/tools/objtool/check.c
> +++ b/tools/objtool/check.c
> @@ -4500,7 +4500,9 @@ static int validate_sls(struct objtool_file *file)
> }
> }
>
> - return warnings;
> + /* SLS is an optional security safety feature, make it fatal
> + * to ensure no new code is introduced that fails SLS */
> + return -warnings;
> }

I'm thinking this patch (and the next one) go too far, yet not far
enough :-)

Too far, because there are still some outstanding randconfig warnings
which need to be fixed.

Not far enough, because there are other warnings which might also have
disastrous effects. For example, even the "unreachable warning" could
mean missing SLS or retpoline mitigations.

So I'm thinking we should try to get as many of the outstanding warnings
fixed as we can, and then flip the CONFIG_WERROR switch for all
non-fatal warnings.

--
Josh