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

[Steven Rostedt]

On Wed, 27 Dec 2017 08:50:33 +0000
Ard Biesheuvel <ard.biesheuvel@xxxxxxxxxx> wrote:

>  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.

Could we switch these calculations to something like:

	return (jump_label_t)((long)&entrty->code + entry->code);

> +}
> +
> +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.

-- Steve


> +				       __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