Re: [RFC PATCH 2/7] static_call: deal with unexported keys without cluttering up the API

On Tue, 9 Nov 2021 at 19:49, Peter Zijlstra [off-list ref] wrote:

On Tue, Nov 09, 2021 at 05:45:44PM +0100, Ard Biesheuvel wrote:

quoted

diff --git a/include/linux/static_call_types.h b/include/linux/static_call_types.h
index 5a00b8b2cf9f..0bb36294cce7 100644
--- a/include/linux/static_call_types.h
+++ b/include/linux/static_call_types.h

@@ -32,15 +32,20 @@
 struct static_call_site {
      s32 addr;
      s32 key;
+     s32 tramp;
 };

I can't say I'm thrilled at growing this thing, but the cleanup is nice.
Perhaps we can increase alignment on struct static_call_key and instead
frob it in .key still?

This is already a place-relative field, and one points into the data
section and the other into text. So I don't see how we can squeeze
enough bits out of it to make this fit.

quoted

 #define DECLARE_STATIC_CALL(name, func)                                      \
-     extern struct static_call_key STATIC_CALL_KEY(name);            \
+     extern __weak struct static_call_key STATIC_CALL_KEY(name);     \
      extern typeof(func) STATIC_CALL_TRAMP(name);

I'm a little bit confused on how this actually works. What does a __weak
extern data symbol do?

A __weak function definition would create a module local instance of the
function barring a strong override.

But what does a __weak extern do?

It is simply a reference that is permitted to remain undefined.

That is why (in another patch) I do sth like

extern __weak foo;

if (&foo)
  ... use foo
else
   ... use sth else

quoted

diff --git a/kernel/static_call.c b/kernel/static_call.c
index 43ba0b1e0edb..360cc3cd0fbf 100644
--- a/kernel/static_call.c
+++ b/kernel/static_call.c

@@ -366,18 +366,18 @@ static int static_call_add_module(struct module *mod)
               * means modules are allowed to call static_call_update() on
               * it.
               *
-              * Otherwise, the key isn't exported, and 'addr' points to the
+              * Otherwise, the key isn't exported, and 'tramp' points to the
               * trampoline so we need to lookup the key.
               *
               * We go through this dance to prevent crazy modules from
               * abusing sensitive static calls.
               */
-             if (!kernel_text_address(addr))
+             if (addr)
                      continue;

This seems to imply that the __weak extern symbol gets resolved to 0 at
module link time.

Yes, if the referenced symbol is not exported, it just uses 0x0 for its value.

quoted

-             key = tramp_key_lookup(addr);
+             key = tramp_key_lookup((unsigned long)offset_to_ptr(&site->tramp));
              if (!key) {
-                     pr_warn("Failed to fixup __raw_static_call() usage at: %ps\n",
+                     pr_warn("Failed to fixup static_call() usage at: %ps\n",
                              static_call_addr(site));
                      return -EINVAL;

quoted

              }

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel