Re: [GIT PULL] objtool changes for v5.19

[Linus Torvalds <torvalds@linux-foundation.org>]

[-- Attachment #1: Type: text/plain, Size: 1060 bytes --]

On Mon, May 23, 2022 at 11:02 PM Ingo Molnar <mingo@kernel.org> wrote:
>
> Note that with your latest tree you'll get 3 new conflicts:

No problem, but the conflicts did make something clear: the objtool
code should just get rid of the "--uaccess" flag that is now
unconditional when CONFIG_X86_SMAP has been removed.

I didn't actually do that, and instead just did the mindless merge
conflict resolution, but it might be a good idea.

See commit dbae0a934f09 ("x86/cpu: Remove CONFIG_X86_SMAP and
'nosmap'") for when it was removed.

Josh? The patch *might* be something like the attached, but this is
(a) untested and (b) that 'opts.noinstr' part of the patch is a bit
dodgy (ie I made the previous 'if' unconditional, but then changed the
follow 'else if ()'  to just 'if ()' instead of deleting it, which is
what "uaccess is always set" would technically have done.

Again: I did *not* do this as part of the merge, the attached patch is
just a suggestion of something that I think should now be done after
the merge.

Hmm?

                        Linus

[-- Attachment #2: patch.diff --]
[-- Type: text/x-patch, Size: 4744 bytes --]

 scripts/Makefile.build                  |  1 -
 scripts/link-vmlinux.sh                 |  2 --
 tools/objtool/arch/x86/special.c        |  8 ++------
 tools/objtool/builtin-check.c           |  4 +---
 tools/objtool/check.c                   | 30 +++++++++++++-----------------
 tools/objtool/include/objtool/builtin.h |  1 -
 6 files changed, 16 insertions(+), 30 deletions(-)

diff --git a/scripts/Makefile.build b/scripts/Makefile.build
index f89d3fcff39f..4d96dea9ff31 100644
--- a/scripts/Makefile.build
+++ b/scripts/Makefile.build
@@ -236,7 +236,6 @@ objtool_args =								\
 	$(if $(CONFIG_SLS), --sls)					\
 	$(if $(CONFIG_STACK_VALIDATION), --stackval)			\
 	$(if $(CONFIG_HAVE_STATIC_CALL_INLINE), --static-call)		\
-	--uaccess							\
 	$(if $(linked-object), --link)					\
 	$(if $(part-of-module), --module)				\
 	$(if $(CONFIG_GCOV_KERNEL), --no-unreachable)
diff --git a/scripts/link-vmlinux.sh b/scripts/link-vmlinux.sh
index d7f26f02f142..add98ce9e1e5 100755
--- a/scripts/link-vmlinux.sh
+++ b/scripts/link-vmlinux.sh
@@ -152,8 +152,6 @@ objtool_link()
 		if is_enabled CONFIG_HAVE_STATIC_CALL_INLINE; then
 			objtoolopt="${objtoolopt} --static-call"
 		fi
-
-		objtoolopt="${objtoolopt} --uaccess"
 	fi
 
 	if is_enabled CONFIG_NOINSTR_VALIDATION; then
diff --git a/tools/objtool/arch/x86/special.c b/tools/objtool/arch/x86/special.c
index 7c97b7391279..b070edb5fd8f 100644
--- a/tools/objtool/arch/x86/special.c
+++ b/tools/objtool/arch/x86/special.c
@@ -12,18 +12,14 @@ void arch_handle_alternative(unsigned short feature, struct special_alt *alt)
 	switch (feature) {
 	case X86_FEATURE_SMAP:
 		/*
-		 * If UACCESS validation is enabled; force that alternative;
-		 * otherwise force it the other way.
+		 * Force the uaccess alternative.
 		 *
 		 * What we want to avoid is having both the original and the
 		 * alternative code flow at the same time, in that case we can
 		 * find paths that see the STAC but take the NOP instead of
 		 * CLAC and the other way around.
 		 */
-		if (opts.uaccess)
-			alt->skip_orig = true;
-		else
-			alt->skip_alt = true;
+		alt->skip_orig = true;
 		break;
 	case X86_FEATURE_POPCNT:
 		/*
diff --git a/tools/objtool/builtin-check.c b/tools/objtool/builtin-check.c
index f4c3a5091737..ae28905dc17a 100644
--- a/tools/objtool/builtin-check.c
+++ b/tools/objtool/builtin-check.c
@@ -71,7 +71,6 @@ const struct option check_options[] = {
 	OPT_BOOLEAN('l', "sls", &opts.sls, "validate straight-line-speculation mitigations"),
 	OPT_BOOLEAN('s', "stackval", &opts.stackval, "validate frame pointer rules"),
 	OPT_BOOLEAN('t', "static-call", &opts.static_call, "annotate static calls"),
-	OPT_BOOLEAN('u', "uaccess", &opts.uaccess, "validate uaccess rules for SMAP"),
 	OPT_CALLBACK_OPTARG(0, "dump", NULL, NULL, "orc", "dump metadata", parse_dump),
 
 	OPT_GROUP("Options:"),
@@ -125,8 +124,7 @@ static bool opts_valid(void)
 	    opts.retpoline		||
 	    opts.sls			||
 	    opts.stackval		||
-	    opts.static_call		||
-	    opts.uaccess) {
+	    opts.static_call) {
 		if (opts.dump_orc) {
 			ERROR("--dump can't be combined with other options");
 			return false;
diff --git a/tools/objtool/check.c b/tools/objtool/check.c
index 190b2f6e360a..86f01f86cd13 100644
--- a/tools/objtool/check.c
+++ b/tools/objtool/check.c
@@ -1030,9 +1030,6 @@ static void add_uaccess_safe(struct objtool_file *file)
 	struct symbol *func;
 	const char **name;
 
-	if (!opts.uaccess)
-		return;
-
 	for (name = uaccess_safe_builtin; *name; name++) {
 		func = find_symbol_by_name(file->elf, *name);
 		if (!func)
@@ -3919,25 +3916,24 @@ int check(struct objtool_file *file)
 		warnings += ret;
 	}
 
-	if (opts.stackval || opts.orc || opts.uaccess) {
-		ret = validate_functions(file);
-		if (ret < 0)
-			goto out;
-		warnings += ret;
+	ret = validate_functions(file);
+	if (ret < 0)
+		goto out;
+	warnings += ret;
 
-		ret = validate_unwind_hints(file, NULL);
+	ret = validate_unwind_hints(file, NULL);
+	if (ret < 0)
+		goto out;
+	warnings += ret;
+
+	if (!warnings) {
+		ret = validate_reachable_instructions(file);
 		if (ret < 0)
 			goto out;
 		warnings += ret;
+	}
 
-		if (!warnings) {
-			ret = validate_reachable_instructions(file);
-			if (ret < 0)
-				goto out;
-			warnings += ret;
-		}
-
-	} else if (opts.noinstr) {
+	if (opts.noinstr) {
 		ret = validate_noinstr_sections(file);
 		if (ret < 0)
 			goto out;
diff --git a/tools/objtool/include/objtool/builtin.h b/tools/objtool/include/objtool/builtin.h
index 280ea18b7f2b..8054f3dc7712 100644
--- a/tools/objtool/include/objtool/builtin.h
+++ b/tools/objtool/include/objtool/builtin.h
@@ -22,7 +22,6 @@ struct opts {
 	bool sls;
 	bool stackval;
 	bool static_call;
-	bool uaccess;
 
 	/* options: */
 	bool backtrace;