2009-03-17 00:24:49

by Jeremy Fitzhardinge

[permalink] [raw]
Subject: [PATCH] x86-paravirt: prevent gcc from generating the wrong addressing mode

When we generate a call sequence for calling a paravirtualized
function, we presume that the generated code is "call *0xXXXXX",
which is a 6 byte opcode; this is larger than a normal
direct call, and so we can patch a direct call over it.

At the moment, however we give gcc enough rope to hang us by
putting the address in a register and generating a two byte
indirect-via-register call. Prevent this by explicitly
dereferencing the function pointer and passing it into the
asm as a constant.

This prevents crashes in VMI, as it cannot handle unpatchable
callsites.

Signed-off-by: Jeremy Fitzhardinge <[email protected]>
Cc: Alok Kataria <[email protected]>

diff --git a/arch/x86/include/asm/paravirt.h b/arch/x86/include/asm/paravirt.h
index d4fec1f..6922d16 100644
--- a/arch/x86/include/asm/paravirt.h
+++ b/arch/x86/include/asm/paravirt.h
@@ -395,7 +395,7 @@ extern struct pv_lock_ops pv_lock_ops;

#define paravirt_type(op) \
[paravirt_typenum] "i" (PARAVIRT_PATCH(op)), \
- [paravirt_opptr] "m" (op)
+ [paravirt_opptr] "i" (&(op))
#define paravirt_clobber(clobber) \
[paravirt_clobber] "i" (clobber)

@@ -449,7 +449,7 @@ 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 "call *%[paravirt_opptr];"
+#define PARAVIRT_CALL "call *%c[paravirt_opptr];"

/*
* These macros are intended to wrap calls through one of the paravirt


2009-03-17 00:59:46

by Alok Kataria

[permalink] [raw]
Subject: Re: [PATCH] x86-paravirt: prevent gcc from generating the wrong addressing mode

Thanks Jeremy.

Acked-by: Alok N Kataria <[email protected]>

On Mon, 2009-03-16 at 17:24 -0700, Jeremy Fitzhardinge wrote:
> When we generate a call sequence for calling a paravirtualized
> function, we presume that the generated code is "call *0xXXXXX",
> which is a 6 byte opcode; this is larger than a normal
> direct call, and so we can patch a direct call over it.
>
> At the moment, however we give gcc enough rope to hang us by
> putting the address in a register and generating a two byte
> indirect-via-register call. Prevent this by explicitly
> dereferencing the function pointer and passing it into the
> asm as a constant.
>
> This prevents crashes in VMI, as it cannot handle unpatchable
> callsites.
>
> Signed-off-by: Jeremy Fitzhardinge <[email protected]>
> Cc: Alok Kataria <[email protected]>
>
> diff --git a/arch/x86/include/asm/paravirt.h b/arch/x86/include/asm/paravirt.h
> index d4fec1f..6922d16 100644
> --- a/arch/x86/include/asm/paravirt.h
> +++ b/arch/x86/include/asm/paravirt.h
> @@ -395,7 +395,7 @@ extern struct pv_lock_ops pv_lock_ops;
>
> #define paravirt_type(op) \
> [paravirt_typenum] "i" (PARAVIRT_PATCH(op)), \
> - [paravirt_opptr] "m" (op)
> + [paravirt_opptr] "i" (&(op))
> #define paravirt_clobber(clobber) \
> [paravirt_clobber] "i" (clobber)
>
> @@ -449,7 +449,7 @@ 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 "call *%[paravirt_opptr];"
> +#define PARAVIRT_CALL "call *%c[paravirt_opptr];"
>
> /*
> * These macros are intended to wrap calls through one of the paravirt
>
>

2009-03-17 02:13:23

by Jeremy Fitzhardinge

[permalink] [raw]
Subject: [tip:x86/urgent] x86, paravirt: prevent gcc from generating the wrong addressing mode

Commit-ID: 42854dc0a6320ff36722749acafa0697522d9556
Gitweb: http://git.kernel.org/tip/42854dc0a6320ff36722749acafa0697522d9556
Author: Jeremy Fitzhardinge <[email protected]>
AuthorDate: Mon, 16 Mar 2009 17:24:34 -0700
Commit: H. Peter Anvin <[email protected]>
CommitDate: Mon, 16 Mar 2009 18:36:31 -0700

x86, paravirt: prevent gcc from generating the wrong addressing mode

Impact: fix crash on VMI (VMware)

When we generate a call sequence for calling a paravirtualized
function, we presume that the generated code is "call *0xXXXXX",
which is a 6 byte opcode; this is larger than a normal
direct call, and so we can patch a direct call over it.

At the moment, however we give gcc enough rope to hang us by
putting the address in a register and generating a two byte
indirect-via-register call. Prevent this by explicitly
dereferencing the function pointer and passing it into the
asm as a constant.

This prevents crashes in VMI, as it cannot handle unpatchable
callsites.

Signed-off-by: Jeremy Fitzhardinge <[email protected]>
Cc: Alok Kataria <[email protected]>
LKML-Reference: <[email protected]>
Signed-off-by: H. Peter Anvin <[email protected]>


---
arch/x86/include/asm/paravirt.h | 4 ++--
1 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/arch/x86/include/asm/paravirt.h b/arch/x86/include/asm/paravirt.h
index e299287..0c212d5 100644
--- a/arch/x86/include/asm/paravirt.h
+++ b/arch/x86/include/asm/paravirt.h
@@ -360,7 +360,7 @@ extern struct pv_lock_ops pv_lock_ops;

#define paravirt_type(op) \
[paravirt_typenum] "i" (PARAVIRT_PATCH(op)), \
- [paravirt_opptr] "m" (op)
+ [paravirt_opptr] "i" (&(op))
#define paravirt_clobber(clobber) \
[paravirt_clobber] "i" (clobber)

@@ -412,7 +412,7 @@ 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 "call *%[paravirt_opptr];"
+#define PARAVIRT_CALL "call *%c[paravirt_opptr];"

/*
* These macros are intended to wrap calls through one of the paravirt