| Message ID | 20190730191303.206365-11-thgarnie@chromium.org (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | x86: PIE support to extend KASLR randomization | expand |
On Tue, Jul 30, 2019 at 12:12:54PM -0700, Thomas Garnier wrote: > if PIE is enabled, switch the paravirt assembly constraints to be > compatible. The %c/i constrains generate smaller code so is kept by > default. > > Position Independent Executable (PIE) support will allow to extend the > KASLR randomization range below 0xffffffff80000000. > > Signed-off-by: Thomas Garnier <thgarnie@chromium.org> > Acked-by: Juergen Gross <jgross@suse.com> > --- > arch/x86/include/asm/paravirt_types.h | 25 +++++++++++++++++++++---- > 1 file changed, 21 insertions(+), 4 deletions(-) > > diff --git a/arch/x86/include/asm/paravirt_types.h b/arch/x86/include/asm/paravirt_types.h > index 70b654f3ffe5..fd7dc37d0010 100644 > --- a/arch/x86/include/asm/paravirt_types.h > +++ b/arch/x86/include/asm/paravirt_types.h > @@ -338,9 +338,25 @@ extern struct paravirt_patch_template pv_ops; > #define PARAVIRT_PATCH(x) \ > (offsetof(struct paravirt_patch_template, x) / sizeof(void *)) > > +#ifdef CONFIG_X86_PIE > +#define paravirt_opptr_call "a" > +#define paravirt_opptr_type "p" > + > +/* > + * Alternative patching requires a maximum of 7 bytes but the relative call is > + * only 6 bytes. If PIE is enabled, add an additional nop to the call > + * instruction to ensure patching is possible. > + */ > +#define PARAVIRT_CALL_POST "nop;" I'm confused; where does the 7 come from? The relative call is 6 bytes, a normal call is 5 bytes (which is what we normally replace them with), and the longest 'native' sequence we seem to have is also 6 bytes (.cpu_usergs_sysret64). > +#else > +#define paravirt_opptr_call "c" > +#define paravirt_opptr_type "i" > +#define PARAVIRT_CALL_POST "" > +#endif > + > #define paravirt_type(op) \ > [paravirt_typenum] "i" (PARAVIRT_PATCH(op)), \ > - [paravirt_opptr] "i" (&(pv_ops.op)) > + [paravirt_opptr] paravirt_opptr_type (&(pv_ops.op)) > #define paravirt_clobber(clobber) \ > [paravirt_clobber] "i" (clobber) > > @@ -379,9 +395,10 @@ int paravirt_disable_iospace(void); > * offset into the paravirt_patch_template structure, and can therefore be > * freely converted back into a structure offset. > */ > -#define PARAVIRT_CALL \ > - ANNOTATE_RETPOLINE_SAFE \ > - "call *%c[paravirt_opptr];" > +#define PARAVIRT_CALL \ > + ANNOTATE_RETPOLINE_SAFE \ > + "call *%" paravirt_opptr_call "[paravirt_opptr];" \ > + PARAVIRT_CALL_POST
On Wed, Jul 31, 2019 at 02:53:06PM +0200, Peter Zijlstra wrote: > On Tue, Jul 30, 2019 at 12:12:54PM -0700, Thomas Garnier wrote: > > if PIE is enabled, switch the paravirt assembly constraints to be > > compatible. The %c/i constrains generate smaller code so is kept by > > default. > > > > Position Independent Executable (PIE) support will allow to extend the > > KASLR randomization range below 0xffffffff80000000. > > > > Signed-off-by: Thomas Garnier <thgarnie@chromium.org> > > Acked-by: Juergen Gross <jgross@suse.com> > > --- > > arch/x86/include/asm/paravirt_types.h | 25 +++++++++++++++++++++---- > > 1 file changed, 21 insertions(+), 4 deletions(-) > > > > diff --git a/arch/x86/include/asm/paravirt_types.h b/arch/x86/include/asm/paravirt_types.h > > index 70b654f3ffe5..fd7dc37d0010 100644 > > --- a/arch/x86/include/asm/paravirt_types.h > > +++ b/arch/x86/include/asm/paravirt_types.h > > @@ -338,9 +338,25 @@ extern struct paravirt_patch_template pv_ops; > > #define PARAVIRT_PATCH(x) \ > > (offsetof(struct paravirt_patch_template, x) / sizeof(void *)) > > > > +#ifdef CONFIG_X86_PIE > > +#define paravirt_opptr_call "a" > > +#define paravirt_opptr_type "p" > > + > > +/* > > + * Alternative patching requires a maximum of 7 bytes but the relative call is > > + * only 6 bytes. If PIE is enabled, add an additional nop to the call > > + * instruction to ensure patching is possible. > > + */ > > +#define PARAVIRT_CALL_POST "nop;" > > I'm confused; where does the 7 come from? The relative call is 6 bytes, Well, before it, the relative CALL is a CALL reg/mem64, i.e. the target is mem64. For example: ffffffff81025c45: ff 14 25 68 37 02 82 callq *0xffffffff82023768 That address there is practically pv_ops + offset. Now, in the opcode bytes you have 0xff opcode, ModRM byte 0x14 and SIB byte 0x25, and 4 bytes imm32 offset. And this is 7 bytes. What it becomes is: ffffffff81025cd0: ff 15 fa d9 ff 00 callq *0xffd9fa(%rip) # ffffffff820236d0 <pv_ops+0x30> ffffffff81025cd6: 90 nop which is a RIP-relative, i.e., opcode 0xff, ModRM byte 0x15 and imm32. And this is 6 bytes. And since the paravirt patching doesn't do NOP padding like the alternatives patching does, you need to pad with a byte. Thomas, please add the gist of this to the comments because this incomprehensible machinery better be documented as detailed as possible. Thx.
On Mon, Aug 12, 2019 at 5:54 AM Borislav Petkov <bp@alien8.de> wrote: > > On Wed, Jul 31, 2019 at 02:53:06PM +0200, Peter Zijlstra wrote: > > On Tue, Jul 30, 2019 at 12:12:54PM -0700, Thomas Garnier wrote: > > > if PIE is enabled, switch the paravirt assembly constraints to be > > > compatible. The %c/i constrains generate smaller code so is kept by > > > default. > > > > > > Position Independent Executable (PIE) support will allow to extend the > > > KASLR randomization range below 0xffffffff80000000. > > > > > > Signed-off-by: Thomas Garnier <thgarnie@chromium.org> > > > Acked-by: Juergen Gross <jgross@suse.com> > > > --- > > > arch/x86/include/asm/paravirt_types.h | 25 +++++++++++++++++++++---- > > > 1 file changed, 21 insertions(+), 4 deletions(-) > > > > > > diff --git a/arch/x86/include/asm/paravirt_types.h b/arch/x86/include/asm/paravirt_types.h > > > index 70b654f3ffe5..fd7dc37d0010 100644 > > > --- a/arch/x86/include/asm/paravirt_types.h > > > +++ b/arch/x86/include/asm/paravirt_types.h > > > @@ -338,9 +338,25 @@ extern struct paravirt_patch_template pv_ops; > > > #define PARAVIRT_PATCH(x) \ > > > (offsetof(struct paravirt_patch_template, x) / sizeof(void *)) > > > > > > +#ifdef CONFIG_X86_PIE > > > +#define paravirt_opptr_call "a" > > > +#define paravirt_opptr_type "p" > > > + > > > +/* > > > + * Alternative patching requires a maximum of 7 bytes but the relative call is > > > + * only 6 bytes. If PIE is enabled, add an additional nop to the call > > > + * instruction to ensure patching is possible. > > > + */ > > > +#define PARAVIRT_CALL_POST "nop;" > > > > I'm confused; where does the 7 come from? The relative call is 6 bytes, > > Well, before it, the relative CALL is a CALL reg/mem64, i.e. the target > is mem64. For example: > > > ffffffff81025c45: ff 14 25 68 37 02 82 callq *0xffffffff82023768 > > That address there is practically pv_ops + offset. > > Now, in the opcode bytes you have 0xff opcode, ModRM byte 0x14 and SIB > byte 0x25, and 4 bytes imm32 offset. And this is 7 bytes. > > What it becomes is: > > ffffffff81025cd0: ff 15 fa d9 ff 00 callq *0xffd9fa(%rip) # ffffffff820236d0 <pv_ops+0x30> > ffffffff81025cd6: 90 nop > > which is a RIP-relative, i.e., opcode 0xff, ModRM byte 0x15 and imm32. > And this is 6 bytes. > > And since the paravirt patching doesn't do NOP padding like the > alternatives patching does, you need to pad with a byte. > > Thomas, please add the gist of this to the comments because this > incomprehensible machinery better be documented as detailed as possible. Sorry for the late reply, busy couple months. Will add it. > > Thx. > > -- > Regards/Gruss, > Boris. > > Good mailing practices for 400: avoid top-posting and trim the reply.
diff --git a/arch/x86/include/asm/paravirt_types.h b/arch/x86/include/asm/paravirt_types.h index 70b654f3ffe5..fd7dc37d0010 100644 --- a/arch/x86/include/asm/paravirt_types.h +++ b/arch/x86/include/asm/paravirt_types.h @@ -338,9 +338,25 @@ extern struct paravirt_patch_template pv_ops; #define PARAVIRT_PATCH(x) \ (offsetof(struct paravirt_patch_template, x) / sizeof(void *)) +#ifdef CONFIG_X86_PIE +#define paravirt_opptr_call "a" +#define paravirt_opptr_type "p" + +/* + * Alternative patching requires a maximum of 7 bytes but the relative call is + * only 6 bytes. If PIE is enabled, add an additional nop to the call + * instruction to ensure patching is possible. + */ +#define PARAVIRT_CALL_POST "nop;" +#else +#define paravirt_opptr_call "c" +#define paravirt_opptr_type "i" +#define PARAVIRT_CALL_POST "" +#endif + #define paravirt_type(op) \ [paravirt_typenum] "i" (PARAVIRT_PATCH(op)), \ - [paravirt_opptr] "i" (&(pv_ops.op)) + [paravirt_opptr] paravirt_opptr_type (&(pv_ops.op)) #define paravirt_clobber(clobber) \ [paravirt_clobber] "i" (clobber) @@ -379,9 +395,10 @@ int paravirt_disable_iospace(void); * offset into the paravirt_patch_template structure, and can therefore be * freely converted back into a structure offset. */ -#define PARAVIRT_CALL \ - ANNOTATE_RETPOLINE_SAFE \ - "call *%c[paravirt_opptr];" +#define PARAVIRT_CALL \ + ANNOTATE_RETPOLINE_SAFE \ + "call *%" paravirt_opptr_call "[paravirt_opptr];" \ + PARAVIRT_CALL_POST /* * These macros are intended to wrap calls through one of the paravirt