Re: [PATCH v2 14/14] powerpc: Implement UACCESS validation on PPC32

[Christophe Leroy]

Le 22/06/2023 à 13:56, Peter Zijlstra a écrit :
> On Thu, Jun 22, 2023 at 12:54:36PM +0200, Christophe Leroy wrote:
> 
>> diff --git a/tools/objtool/check.c b/tools/objtool/check.c
>> index f850ab892ad5..8ac5711a055f 100644
>> --- a/tools/objtool/check.c
>> +++ b/tools/objtool/check.c
>> @@ -218,6 +218,7 @@ static bool __dead_end_function(struct objtool_file *file, struct symbol *func,
>>   		"kthread_exit",
>>   		"kunit_try_catch_throw",
>>   		"lbug_with_loc",
>> +		"longjmp",
>>   		"machine_real_restart",
>>   		"make_task_dead",
>>   		"mpt_halt_firmware",
>> @@ -230,7 +231,9 @@ static bool __dead_end_function(struct objtool_file *file, struct symbol *func,
>>   		"sev_es_terminate",
>>   		"snp_abort",
>>   		"start_kernel",
>> +		"start_secondary_resume",
>>   		"stop_this_cpu",
>> +		"unrecoverable_exception",
>>   		"usercopy_abort",
>>   		"x86_64_start_kernel",
>>   		"x86_64_start_reservations",
> 
> Someone went and changed all that in tip/objtool/core :-)
> 
> But perhaps, like the uaccess_safe_builtins[] array below, should we
> start marking sections so we can remember where stuff comes from later?

Or, now that it is a H file, maybe each arch could have its own H file 
for arch specific functions ? Then we'd get:

diff --git a/tools/objtool/arch/powerpc/include/arch/noreturns.h 
b/tools/objtool/arch/powerpc/include/arch/noreturns.h
new file mode 100644
index 000000000000..664f17d39026
--- /dev/null
+++ b/tools/objtool/arch/powerpc/include/arch/noreturns.h
@@ -0,0 +1,11 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+
+/*
+ * This is a (sorted!) list of all known __noreturn functions in 
arch/powerpc.
+ * It's needed for objtool to properly reverse-engineer the control 
flow graph.
+ *
+ * Yes, this is unfortunate.  A better solution is in the works.
+ */
+NORETURN(longjmp)
+NORETURN(start_secondary_resume)
+NORETURN(unrecoverable_exception)
diff --git a/tools/objtool/noreturns.h b/tools/objtool/noreturns.h
index 1514e84d5cc4..f725ed37532d 100644
--- a/tools/objtool/noreturns.h
+++ b/tools/objtool/noreturns.h
@@ -1,5 +1,7 @@
  /* SPDX-License-Identifier: GPL-2.0 */

+#include <arch/noreturns.h>
+
  /*
   * This is a (sorted!) list of all known __noreturn functions in the 
kernel.
   * It's needed for objtool to properly reverse-engineer the control 
flow graph.


> 
>> @@ -1335,6 +1338,8 @@ static const char *uaccess_safe_builtin[] = {
>>   	"rep_stos_alternative",
>>   	"rep_movs_alternative",
>>   	"__copy_user_nocache",
>> +	"__copy_tofrom_user",
>> +	"__arch_clear_user",
>>   	NULL
>>   };
> 
> Do we want to rename the 'misc' sectino to 'x86' and start a 'ppc32'
> section there?
> 

Sure.

Then that would look like:

diff --git a/tools/objtool/check.c b/tools/objtool/check.c
index 2b61f8180bea..2d564d0e2ae1 100644
--- a/tools/objtool/check.c
+++ b/tools/objtool/check.c
@@ -1259,13 +1259,15 @@ static const char *uaccess_safe_builtin[] = {
  	"stackleak_track_stack",
  	/* misc */
  	"csum_partial_copy_generic",
+	"ftrace_likely_update", /* CONFIG_TRACE_BRANCH_PROFILING */
+	/* misc x86 */
  	"copy_mc_fragile",
  	"copy_mc_fragile_handle_tail",
  	"copy_mc_enhanced_fast_string",
-	"ftrace_likely_update", /* CONFIG_TRACE_BRANCH_PROFILING */
  	"rep_stos_alternative",
  	"rep_movs_alternative",
  	"__copy_user_nocache",
+	/* misc powerpc */
  	"__copy_tofrom_user",
  	"__arch_clear_user",
  	NULL