| Message ID | 20190131192533.34130-3-thgarnie@chromium.org (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | x86: PIE support and option to extend KASLR randomization | expand |
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
On Thu, Feb 7, 2019 at 4:17 AM Borislav Petkov <bp@alien8.de> 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.
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.
On Thu, Feb 7, 2019 at 9:11 AM Borislav Petkov <bp@alien8.de> 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.
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:
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 <thgarnie@chromium.org> --- arch/x86/include/asm/jump_label.h | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-)