Thread (23 messages) 23 messages, 5 authors, 2017-12-29

Re: [PATCH v6 8/8] x86/kernel: jump_table: use relative references

From: Ard Biesheuvel <hidden>
Date: 2017-12-28 16:26:11
Also in: linux-arm-kernel, linux-mips, lkml, sparclinux

On 28 December 2017 at 16:19, Steven Rostedt [off-list ref] wrote:
On Wed, 27 Dec 2017 08:50:33 +0000
Ard Biesheuvel [off-list ref] wrote:
quoted
 static inline jump_label_t jump_entry_code(const struct jump_entry *entry)
 {
-     return entry->code;
+     return (jump_label_t)&entry->code + entry->code;
I'm paranoid about doing arithmetic on abstract types. What happens in
the future if jump_label_t becomes a pointer? You will get a different
result.
In general, I share your concern. In this case, however, jump_label_t
is typedef'd three lines up and is never used anywhere else.
Could we switch these calculations to something like:

        return (jump_label_t)((long)&entrty->code + entry->code);
jump_label_t is local to this .h file, so it can be defined as u32 or
u64 depending on the word size. I don't mind adding the extra cast,
but I am not sure if your paranoia is justified in this particular
case. Perhaps we should just use 'unsigned long' throughout?
quoted
+}
+
+static inline jump_label_t jump_entry_target(const struct jump_entry *entry)
+{
+     return (jump_label_t)&entry->target + entry->target;
 }

 static inline struct static_key *jump_entry_key(const struct jump_entry *entry)
 {
-     return (struct static_key *)((unsigned long)entry->key & ~1UL);
+     unsigned long key = (unsigned long)&entry->key + entry->key;
+
+     return (struct static_key *)(key & ~1UL);
 }

 static inline bool jump_entry_is_branch(const struct jump_entry *entry)
@@ -99,7 +106,7 @@ static inline void jump_entry_set_module_init(struct jump_entry *entry)
      entry->code = 0;
 }

-#define jump_label_swap              NULL
+void jump_label_swap(void *a, void *b, int size);

 #else        /* __ASSEMBLY__ */
@@ -114,8 +121,8 @@ static inline void jump_entry_set_module_init(struct jump_entry *entry)
      .byte           STATIC_KEY_INIT_NOP
      .endif
      .pushsection __jump_table, "aw"
-     _ASM_ALIGN
-     _ASM_PTR        .Lstatic_jump_\@, \target, \key
+     .balign         4
+     .long           .Lstatic_jump_\@ - ., \target - ., \key - .
      .popsection
 .endm
@@ -130,8 +137,8 @@ static inline void jump_entry_set_module_init(struct jump_entry *entry)
 .Lstatic_jump_after_\@:
      .endif
      .pushsection __jump_table, "aw"
-     _ASM_ALIGN
-     _ASM_PTR        .Lstatic_jump_\@, \target, \key + 1
+     .balign         4
+     .long           .Lstatic_jump_\@ - ., \target - ., \key - . + 1
      .popsection
 .endm
diff --git a/arch/x86/kernel/jump_label.c b/arch/x86/kernel/jump_label.c
index e56c95be2808..cc5034b42335 100644
--- a/arch/x86/kernel/jump_label.c
+++ b/arch/x86/kernel/jump_label.c
@@ -52,22 +52,24 @@ static void __jump_label_transform(struct jump_entry *entry,
                       * Jump label is enabled for the first time.
                       * So we expect a default_nop...
                       */
-                     if (unlikely(memcmp((void *)entry->code, default_nop, 5)
-                                  != 0))
-                             bug_at((void *)entry->code, __LINE__);
+                     if (unlikely(memcmp((void *)jump_entry_code(entry),
+                                         default_nop, 5) != 0))
+                             bug_at((void *)jump_entry_code(entry),
You have the functions already made before this patch. Perhaps we
should have a separate patch to use them (here and elsewhere) before
you make the conversion to using relative references. It will help out
in debugging and bisects. To know if the use of functions is an issue,
or the conversion of relative references is an issue.

I suggest splitting this into two patches.
Fair enough.

quoted
+                                    __LINE__);
              } else {
                      /*
                       * ...otherwise expect an ideal_nop. Otherwise
                       * something went horribly wrong.
                       */
-                     if (unlikely(memcmp((void *)entry->code, ideal_nop, 5)
-                                  != 0))
-                             bug_at((void *)entry->code, __LINE__);
+                     if (unlikely(memcmp((void *)jump_entry_code(entry),
+                                         ideal_nop, 5) != 0))
+                             bug_at((void *)jump_entry_code(entry),
+                                    __LINE__);
              }

              code.jump = 0xe9;
-             code.offset = entry->target -
-                             (entry->code + JUMP_LABEL_NOP_SIZE);
+             code.offset = jump_entry_target(entry) -
+                           (jump_entry_code(entry) + JUMP_LABEL_NOP_SIZE);
      } else {
              /*
               * We are disabling this jump label. If it is not what
@@ -76,14 +78,18 @@ static void __jump_label_transform(struct jump_entry *entry,
               * are converting the default nop to the ideal nop.
               */
              if (init) {
-                     if (unlikely(memcmp((void *)entry->code, default_nop, 5) != 0))
-                             bug_at((void *)entry->code, __LINE__);
+                     if (unlikely(memcmp((void *)jump_entry_code(entry),
+                                         default_nop, 5) != 0))
+                             bug_at((void *)jump_entry_code(entry),
+                                    __LINE__);
              } else {
                      code.jump = 0xe9;
-                     code.offset = entry->target -
-                             (entry->code + JUMP_LABEL_NOP_SIZE);
-                     if (unlikely(memcmp((void *)entry->code, &code, 5) != 0))
-                             bug_at((void *)entry->code, __LINE__);
+                     code.offset = jump_entry_target(entry) -
+                             (jump_entry_code(entry) + JUMP_LABEL_NOP_SIZE);
+                     if (unlikely(memcmp((void *)jump_entry_code(entry),
+                                  &code, 5) != 0))
+                             bug_at((void *)jump_entry_code(entry),
+                                    __LINE__);
              }
              memcpy(&code, ideal_nops[NOP_ATOMIC5], JUMP_LABEL_NOP_SIZE);
      }
@@ -97,10 +103,13 @@ static void __jump_label_transform(struct jump_entry *entry,
       *
       */
      if (poker)
-             (*poker)((void *)entry->code, &code, JUMP_LABEL_NOP_SIZE);
+             (*poker)((void *)jump_entry_code(entry), &code,
+                      JUMP_LABEL_NOP_SIZE);
      else
-             text_poke_bp((void *)entry->code, &code, JUMP_LABEL_NOP_SIZE,
-                          (void *)entry->code + JUMP_LABEL_NOP_SIZE);
+             text_poke_bp((void *)jump_entry_code(entry), &code,
+                          JUMP_LABEL_NOP_SIZE,
+                          (void *)jump_entry_code(entry) +
+                          JUMP_LABEL_NOP_SIZE);
 }

 void arch_jump_label_transform(struct jump_entry *entry,
@@ -140,4 +149,20 @@ __init_or_module void arch_jump_label_transform_static(struct jump_entry *entry,
              __jump_label_transform(entry, type, text_poke_early, 1);
 }

+void jump_label_swap(void *a, void *b, int size)
+{
+     long delta = (unsigned long)a - (unsigned long)b;
+     struct jump_entry *jea = a;
+     struct jump_entry *jeb = b;
+     struct jump_entry tmp = *jea;
+
+     jea->code       = jeb->code - delta;
+     jea->target     = jeb->target - delta;
+     jea->key        = jeb->key - delta;
+
+     jeb->code       = tmp.code + delta;
+     jeb->target     = tmp.target + delta;
+     jeb->key        = tmp.key + delta;
+}
+
 #endif
diff --git a/tools/objtool/special.c b/tools/objtool/special.c
index 84f001d52322..98ae55b39037 100644
--- a/tools/objtool/special.c
+++ b/tools/objtool/special.c
@@ -30,9 +30,9 @@
 #define EX_ORIG_OFFSET               0
 #define EX_NEW_OFFSET                4

-#define JUMP_ENTRY_SIZE              24
+#define JUMP_ENTRY_SIZE              12
 #define JUMP_ORIG_OFFSET     0
-#define JUMP_NEW_OFFSET              8
+#define JUMP_NEW_OFFSET              4

 #define ALT_ENTRY_SIZE               13
 #define ALT_ORIG_OFFSET              0
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help