ClangBuiltLinux / linux

Linux kernel source tree
Other
241 stars 14 forks source link

[LLVM-19] modpost warnings #2051

Open cgzones opened 1 month ago

cgzones commented 1 month ago

When building v6.10.9 with LLVM 19 and LTO and CFI and UBSAN enabled the following warnings are shown:

  ...
  MODPOST Module.symvers
WARNING: modpost: vmlinux: section mismatch in reference: bsp_pm_check_init.bsp_pm_callback_nb+0x0 (section: .data) -> bsp_pm_callback (section: .text)
The .data:bsp_pm_check_init.bsp_pm_callback_nb references
the .text:bsp_pm_callback
WARNING: modpost: modpost: Found 1 writable function pointer(s).
nathanchance commented 1 month ago

Was this against the upstream kernel source? I cannot find any part of the string writable function pointer in scripts/mod/modpost.c and none of the values in sectioncheck indicate that .data to .text is a section violation...

cgzones commented 1 month ago

It's from an out-of-tree patch ("add writable function pointer detection"): https://github.com/anthraxx/linux-hardened/commit/16be296eae85c65c38b03035066a9b03240edfc0

But the initial warning

WARNING: modpost: vmlinux: section mismatch in reference: bsp_pm_check_init.bsp_pm_callback_nb+0x0 (section: .data) -> bsp_pm_callback (section: .text)

is from upstream. (And persists after disabling CONFIG_UBSAN_SIGNED_WRAP.)

nathanchance commented 1 month ago

I think the initial warning might be a result of that patch as well, since it adds the DATA_TO_TEXT mismatch, which is what the initial warning is (something in .data referring to a function in .text).

@kees is this something we would want in upstream? However, I am not really sure how this would not show up with GCC as well, given that bsp_pm_callback_nb is in the global scope with a function pointer to bsp_pm_callback(). Perhaps the _nb variable in pm_notifier should be marked as __ro_after_init or something?

cgzones commented 1 month ago

Thanks, I am currently testing the following patch:

Subject: [PATCH] PM: mark generated data from pm_notifier() __ro_after_init

All callers of pm_notifier() are init functions makrd as __init.
Thus avoid .data to .text references in these structs in particular the
callback function pointer.

TODO: add something like `BUG_ON(kernel_set_to_readonly);` to
pm_notifier() to avoid non __init callers.

Closes: https://github.com/ClangBuiltLinux/linux/issues/2051
Suggested-by: Nathan Chancellor <nathan@kernel.org>
Signed-off-by: Christian Göttsche <cgzones@googlemail.com>
---
 include/linux/suspend.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/include/linux/suspend.h b/include/linux/suspend.h
index da6ebca3ff77..81ccb44d8850 100644
--- a/include/linux/suspend.h
+++ b/include/linux/suspend.h
@@ -443,7 +443,7 @@ extern void pm_report_hw_sleep_time(u64 t);
 extern void pm_report_max_hw_sleep(u64 t);

 #define pm_notifier(fn, pri) {                         \
-       static struct notifier_block fn##_nb =                  \
+       static struct notifier_block fn##_nb __ro_after_init =  \
                { .notifier_call = fn, .priority = pri };       \
        register_pm_notifier(&fn##_nb);                 \
 }
-- 
2.45.2

Is there a trick to avoid pm_notifier() being called from a function not marked __init()?

nathanchance commented 1 month ago

I am not sure. If pm_notifier() can be called from a module, then perhaps that would not work… it would be nice to make that const if it is not modified at any point but it would require marking a number of parameters as const too, which might not be tolerable (but I have not checked).