2019-01-31 19:47:20

by Thomas Garnier

[permalink] [raw]
Subject: [PATCH v6 02/27] x86: Use symbol name in jump table for PIE support

Replace the %c constraint with %P. The %c is incompatible with PIE
because it implies an immediate value whereas %P reference a symbol.
Change the _ASM_PTR reference to .long for expected relocation size and
add a long padding to ensure entry alignment.

Position Independent Executable (PIE) support will allow to extend the
KASLR randomization range below 0xffffffff80000000.

Signed-off-by: Thomas Garnier <[email protected]>
---
arch/x86/include/asm/jump_label.h | 8 ++++----
1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/arch/x86/include/asm/jump_label.h b/arch/x86/include/asm/jump_label.h
index 65191ce8e1cf..e47fad8ee632 100644
--- a/arch/x86/include/asm/jump_label.h
+++ b/arch/x86/include/asm/jump_label.h
@@ -25,9 +25,9 @@ static __always_inline bool arch_static_branch(struct static_key *key, bool bran
".pushsection __jump_table, \"aw\" \n\t"
_ASM_ALIGN "\n\t"
".long 1b - ., %l[l_yes] - . \n\t"
- _ASM_PTR "%c0 + %c1 - .\n\t"
+ _ASM_PTR "%P0 - .\n\t"
".popsection \n\t"
- : : "i" (key), "i" (branch) : : l_yes);
+ : : "X" (&((char *)key)[branch]) : : l_yes);

return false;
l_yes:
@@ -42,9 +42,9 @@ static __always_inline bool arch_static_branch_jump(struct static_key *key, bool
".pushsection __jump_table, \"aw\" \n\t"
_ASM_ALIGN "\n\t"
".long 1b - ., %l[l_yes] - . \n\t"
- _ASM_PTR "%c0 + %c1 - .\n\t"
+ _ASM_PTR "%P0 - .\n\t"
".popsection \n\t"
- : : "i" (key), "i" (branch) : : l_yes);
+ : : "X" (&((char *)key)[branch]) : : l_yes);

return false;
l_yes:
--
2.20.1.495.gaa96b0ce6b-goog



2019-02-07 12:19:27

by Borislav Petkov

[permalink] [raw]
Subject: Re: [PATCH v6 02/27] x86: Use symbol name in jump table for PIE support

On Thu, Jan 31, 2019 at 11:24:09AM -0800, Thomas Garnier wrote:
> Replace the %c constraint with %P. The %c is incompatible with PIE
> because it implies an immediate value whereas %P reference a symbol.

How so?

AFAIK, %c requires a constant operand and if %P is used to print a
constant, it simply drops syntax-specific prefixes and does a bare
constant.

I guess that here

https://gcc.gnu.org/onlinedocs/gcc/Extended-Asm.html#x86Operandmodifiers

is not entirely correct as it should not say "If used for a constant"
for %P but say "symbol or constant".

But before/after asm doesn't show any difference. So what gives?

before:
# 39 "./arch/x86/include/asm/jump_label.h" 1
1:
.byte 0xe9
.long .L241 - 2f #
2:
.pushsection __jump_table, "aw"
.balign 8
.long 1b - ., .L241 - . #
.quad __use_tsc + 1 - . #,
.popsection

after:
# 39 "./arch/x86/include/asm/jump_label.h" 1
1:
.byte 0xe9
.long .L241 - 2f #
2:
.pushsection __jump_table, "aw"
.balign 8
.long 1b - ., .L241 - . #
.quad __use_tsc+1 - . #
.popsection

--
Regards/Gruss,
Boris.

Good mailing practices for 400: avoid top-posting and trim the reply.

2019-02-07 17:05:29

by Thomas Garnier

[permalink] [raw]
Subject: Re: [PATCH v6 02/27] x86: Use symbol name in jump table for PIE support

On Thu, Feb 7, 2019 at 4:17 AM Borislav Petkov <[email protected]> wrote:
>
> On Thu, Jan 31, 2019 at 11:24:09AM -0800, Thomas Garnier wrote:
> > Replace the %c constraint with %P. The %c is incompatible with PIE
> > because it implies an immediate value whereas %P reference a symbol.
>
> How so?
>
> AFAIK, %c requires a constant operand and if %P is used to print a
> constant, it simply drops syntax-specific prefixes and does a bare
> constant.
>
> I guess that here
>
> https://gcc.gnu.org/onlinedocs/gcc/Extended-Asm.html#x86Operandmodifiers
>
> is not entirely correct as it should not say "If used for a constant"
> for %P but say "symbol or constant".
>
> But before/after asm doesn't show any difference. So what gives?

I assume that's an optimisation done by gcc later. The P modifier in
the documentation does state that it is used to generate PIC code.

>
> before:
> # 39 "./arch/x86/include/asm/jump_label.h" 1
> 1:
> .byte 0xe9
> .long .L241 - 2f #
> 2:
> .pushsection __jump_table, "aw"
> .balign 8
> .long 1b - ., .L241 - . #
> .quad __use_tsc + 1 - . #,
> .popsection
>
> after:
> # 39 "./arch/x86/include/asm/jump_label.h" 1
> 1:
> .byte 0xe9
> .long .L241 - 2f #
> 2:
> .pushsection __jump_table, "aw"
> .balign 8
> .long 1b - ., .L241 - . #
> .quad __use_tsc+1 - . #
> .popsection
>
> --
> Regards/Gruss,
> Boris.
>
> Good mailing practices for 400: avoid top-posting and trim the reply.

2019-02-07 17:12:18

by Borislav Petkov

[permalink] [raw]
Subject: Re: [PATCH v6 02/27] x86: Use symbol name in jump table for PIE support

On Thu, Feb 07, 2019 at 09:04:45AM -0800, Thomas Garnier wrote:
> I assume that's an optimisation done by gcc later.

So why is that change even needed? Where does it break?

> The P modifier in the documentation does state that it is used to
> generate PIC code.

The documentation says:

"If used for a function, print the PLT suffix and generate PIC code. For
example, emit foo@PLT instead of ’foo’ for the function foo()."

when you use %P for a function. Which is not how it is used here.

--
Regards/Gruss,
Boris.

Good mailing practices for 400: avoid top-posting and trim the reply.

2019-02-07 23:56:22

by Thomas Garnier

[permalink] [raw]
Subject: Re: [PATCH v6 02/27] x86: Use symbol name in jump table for PIE support

On Thu, Feb 7, 2019 at 9:11 AM Borislav Petkov <[email protected]> wrote:
>
> On Thu, Feb 07, 2019 at 09:04:45AM -0800, Thomas Garnier wrote:
> > I assume that's an optimisation done by gcc later.
>
> So why is that change even needed? Where does it break?
>
> > The P modifier in the documentation does state that it is used to
> > generate PIC code.
>
> The documentation says:
>
> "If used for a function, print the PLT suffix and generate PIC code. For
> example, emit foo@PLT instead of ’foo’ for the function foo()."
>
> when you use %P for a function. Which is not how it is used here.

I did more checks about that. I think Ard's patch to make jump label
relative actually fixed the issue I had with them.

Thanks for spotting this, I will do additional checks and look at
removing this change.

>
> --
> Regards/Gruss,
> Boris.
>
> Good mailing practices for 400: avoid top-posting and trim the reply.