[Qemu-devel] [PATCH] linux-user: Safety belt for h2g

[Qemu-devel] [PATCH] linux-user: Safety belt for h2g

[Jan Kiszka]

h2g can only work on 64-bit hosts if the provided address is mappable to
the guest range. Neglecting this was already the source for several
bugs. Instrument the macro so that it will trigger earlier in the
future (at least as long as we have this kind of mapping mechanism).

Signed-off-by: Jan Kiszka <jan.kiszka@web.de>
---
 cpu-all.h |    8 +++++++-
 1 file changed, 7 insertions(+), 1 deletion(-)

Index: b/cpu-all.h
===================================================================
--- a/cpu-all.h
+++ b/cpu-all.h
@@ -659,6 +659,8 @@ static inline void stfq_be_p(void *ptr,
 /* MMU memory access macros */
 
 #if defined(CONFIG_USER_ONLY)
+#include <assert.h>
+
 /* On some host systems the guest address space is reserved on the host.
  * This allows the guest address space to be offset to a convenient location.
  */
@@ -667,7 +669,11 @@ static inline void stfq_be_p(void *ptr,
 
 /* All direct uses of g2h and h2g need to go away for usermode softmmu.  */
 #define g2h(x) ((void *)((unsigned long)(x) + GUEST_BASE))
-#define h2g(x) ((target_ulong)((unsigned long)(x) - GUEST_BASE))
+#define h2g(x) ({ \
+    unsigned long __ret = (unsigned long)(x) - GUEST_BASE; \
+    assert(__ret == (target_ulong)__ret); \
+    __ret; \
+})
 
 #define saddr(x) g2h(x)
 #define laddr(x) g2h(x)

[Qemu-devel] [PATCH] linux-user: Safety belt for h2g

[Kirill A. Shutemov]

From: Jan Kiszka <jan.kiszka@web.de>

h2g can only work on 64-bit hosts if the provided address is mappable to
the guest range. Neglecting this was already the source for several
bugs. Instrument the macro so that it will trigger earlier in the
future (at least as long as we have this kind of mapping mechanism).

Signed-off-by: Jan Kiszka <jan.kiszka@web.de>
Signed-off-by: Kirill A. Shutemov <kirill@shutemov.name>
---
 cpu-all.h |   10 +++++++++-
 1 files changed, 9 insertions(+), 1 deletions(-)

diff --git a/cpu-all.h b/cpu-all.h
index 73c7b4c..526ace2 100644
--- a/cpu-all.h
+++ b/cpu-all.h
@@ -621,6 +621,9 @@ static inline void stfq_be_p(void *ptr, float64 v)
 /* MMU memory access macros */
 
 #if defined(CONFIG_USER_ONLY)
+#include <assert.h>
+#include "qemu-types.h"
+
 /* On some host systems the guest address space is reserved on the host.
  * This allows the guest address space to be offset to a convenient location.
  */
@@ -629,7 +632,12 @@ static inline void stfq_be_p(void *ptr, float64 v)
 
 /* All direct uses of g2h and h2g need to go away for usermode softmmu.  */
 #define g2h(x) ((void *)((unsigned long)(x) + GUEST_BASE))
-#define h2g(x) ((target_ulong)((unsigned long)(x) - GUEST_BASE))
+#define h2g(x) ({ \
+    unsigned long __ret = (unsigned long)(x) - GUEST_BASE; \
+    /* Check if given address fits target address space */ \
+    assert(__ret == (abi_ulong)__ret); \
+    (abi_ulong)__ret; \
+})
 
 #define saddr(x) g2h(x)
 #define laddr(x) g2h(x)
-- 
1.6.0.2.GIT

Re: [Qemu-devel] [PATCH] linux-user: Safety belt for h2g

[Edgar E. Iglesias]

On Wed, Dec 03, 2008 at 01:29:39PM +0200, Kirill A. Shutemov wrote:
> From: Jan Kiszka <jan.kiszka@web.de>
> 
> h2g can only work on 64-bit hosts if the provided address is mappable to
> the guest range. Neglecting this was already the source for several
> bugs. Instrument the macro so that it will trigger earlier in the
> future (at least as long as we have this kind of mapping mechanism).
> 
> Signed-off-by: Jan Kiszka <jan.kiszka@web.de>
> Signed-off-by: Kirill A. Shutemov <kirill@shutemov.name>

Acked-by: Edgar E. Iglesias <edgar.iglesias@gmail.com>

> ---
>  cpu-all.h |   10 +++++++++-
>  1 files changed, 9 insertions(+), 1 deletions(-)
> 
> diff --git a/cpu-all.h b/cpu-all.h
> index 73c7b4c..526ace2 100644
> --- a/cpu-all.h
> +++ b/cpu-all.h
> @@ -621,6 +621,9 @@ static inline void stfq_be_p(void *ptr, float64 v)
>  /* MMU memory access macros */
>  
>  #if defined(CONFIG_USER_ONLY)
> +#include <assert.h>
> +#include "qemu-types.h"
> +
>  /* On some host systems the guest address space is reserved on the host.
>   * This allows the guest address space to be offset to a convenient location.
>   */
> @@ -629,7 +632,12 @@ static inline void stfq_be_p(void *ptr, float64 v)
>  
>  /* All direct uses of g2h and h2g need to go away for usermode softmmu.  */
>  #define g2h(x) ((void *)((unsigned long)(x) + GUEST_BASE))
> -#define h2g(x) ((target_ulong)((unsigned long)(x) - GUEST_BASE))
> +#define h2g(x) ({ \
> +    unsigned long __ret = (unsigned long)(x) - GUEST_BASE; \
> +    /* Check if given address fits target address space */ \
> +    assert(__ret == (abi_ulong)__ret); \
> +    (abi_ulong)__ret; \
> +})
>  
>  #define saddr(x) g2h(x)
>  #define laddr(x) g2h(x)
> -- 
> 1.6.0.2.GIT

Re: [Qemu-devel] [PATCH] linux-user: Safety belt for h2g

[Aurelien Jarno]

On Wed, Dec 03, 2008 at 01:29:39PM +0200, Kirill A. Shutemov wrote:
> From: Jan Kiszka <jan.kiszka@web.de>
> 
> h2g can only work on 64-bit hosts if the provided address is mappable to
> the guest range. Neglecting this was already the source for several
> bugs. Instrument the macro so that it will trigger earlier in the
> future (at least as long as we have this kind of mapping mechanism).
> 
> Signed-off-by: Jan Kiszka <jan.kiszka@web.de>
> Signed-off-by: Kirill A. Shutemov <kirill@shutemov.name>

Applied. I have seen the patch has been modified since Jan Kiszka posted
it to the mailing list. Not sure a Signed-off-by still applies in that
case.

> ---
>  cpu-all.h |   10 +++++++++-
>  1 files changed, 9 insertions(+), 1 deletions(-)
> 
> diff --git a/cpu-all.h b/cpu-all.h
> index 73c7b4c..526ace2 100644
> --- a/cpu-all.h
> +++ b/cpu-all.h
> @@ -621,6 +621,9 @@ static inline void stfq_be_p(void *ptr, float64 v)
>  /* MMU memory access macros */
>  
>  #if defined(CONFIG_USER_ONLY)
> +#include <assert.h>
> +#include "qemu-types.h"
> +
>  /* On some host systems the guest address space is reserved on the host.
>   * This allows the guest address space to be offset to a convenient location.
>   */
> @@ -629,7 +632,12 @@ static inline void stfq_be_p(void *ptr, float64 v)
>  
>  /* All direct uses of g2h and h2g need to go away for usermode softmmu.  */
>  #define g2h(x) ((void *)((unsigned long)(x) + GUEST_BASE))
> -#define h2g(x) ((target_ulong)((unsigned long)(x) - GUEST_BASE))
> +#define h2g(x) ({ \
> +    unsigned long __ret = (unsigned long)(x) - GUEST_BASE; \
> +    /* Check if given address fits target address space */ \
> +    assert(__ret == (abi_ulong)__ret); \
> +    (abi_ulong)__ret; \
> +})
>  
>  #define saddr(x) g2h(x)
>  #define laddr(x) g2h(x)
> -- 
> 1.6.0.2.GIT
> 
> 
> 
> 

-- 
  .''`.  Aurelien Jarno	            | GPG: 1024D/F1BCDB73
 : :' :  Debian developer           | Electrical Engineer
 `. `'   aurel32@debian.org         | aurelien@aurel32.net
   `-    people.debian.org/~aurel32 | www.aurel32.net

Re: [Qemu-devel] [PATCH] linux-user: Safety belt for h2g

[Andreas Färber]

Am 08.12.2008 um 19:15 schrieb Aurelien Jarno:

> On Wed, Dec 03, 2008 at 01:29:39PM +0200, Kirill A. Shutemov wrote:
>> From: Jan Kiszka <jan.kiszka@web.de>
>>
>> h2g can only work on 64-bit hosts if the provided address is  
>> mappable to
>> the guest range. Neglecting this was already the source for several
>> bugs. Instrument the macro so that it will trigger earlier in the
>> future (at least as long as we have this kind of mapping mechanism).
>>
>> Signed-off-by: Jan Kiszka <jan.kiszka@web.de>
>> Signed-off-by: Kirill A. Shutemov <kirill@shutemov.name>
>
> Applied. I have seen the patch has been modified since Jan Kiszka  
> posted
> it to the mailing list. Not sure a Signed-off-by still applies in that
> case.

Removing a Signed-off-by for code that is kept is a no-go to my  
knowledge. It's supposed to track through whom all the code went  
copyright- and GPL-wise, according to Kerneltrap.

Andreas

Re: [Qemu-devel] [PATCH] linux-user: Safety belt for h2g

[Jan Kiszka]

[-- Attachment #1: Type: text/plain, Size: 2123 bytes --]

Aurelien Jarno wrote:
> On Wed, Dec 03, 2008 at 01:29:39PM +0200, Kirill A. Shutemov wrote:
>> From: Jan Kiszka <jan.kiszka@web.de>
>>
>> h2g can only work on 64-bit hosts if the provided address is mappable to
>> the guest range. Neglecting this was already the source for several
>> bugs. Instrument the macro so that it will trigger earlier in the
>> future (at least as long as we have this kind of mapping mechanism).
>>
>> Signed-off-by: Jan Kiszka <jan.kiszka@web.de>
>> Signed-off-by: Kirill A. Shutemov <kirill@shutemov.name>
> 
> Applied. I have seen the patch has been modified since Jan Kiszka posted
> it to the mailing list. Not sure a Signed-off-by still applies in that
> case.

Kirill correctly pointed out to me that target_ulong should rather be
abi_ulong here and in the other patch.

Good to see these changes finally merged!

Jan

> 
>> ---
>>  cpu-all.h |   10 +++++++++-
>>  1 files changed, 9 insertions(+), 1 deletions(-)
>>
>> diff --git a/cpu-all.h b/cpu-all.h
>> index 73c7b4c..526ace2 100644
>> --- a/cpu-all.h
>> +++ b/cpu-all.h
>> @@ -621,6 +621,9 @@ static inline void stfq_be_p(void *ptr, float64 v)
>>  /* MMU memory access macros */
>>  
>>  #if defined(CONFIG_USER_ONLY)
>> +#include <assert.h>
>> +#include "qemu-types.h"
>> +
>>  /* On some host systems the guest address space is reserved on the host.
>>   * This allows the guest address space to be offset to a convenient location.
>>   */
>> @@ -629,7 +632,12 @@ static inline void stfq_be_p(void *ptr, float64 v)
>>  
>>  /* All direct uses of g2h and h2g need to go away for usermode softmmu.  */
>>  #define g2h(x) ((void *)((unsigned long)(x) + GUEST_BASE))
>> -#define h2g(x) ((target_ulong)((unsigned long)(x) - GUEST_BASE))
>> +#define h2g(x) ({ \
>> +    unsigned long __ret = (unsigned long)(x) - GUEST_BASE; \
>> +    /* Check if given address fits target address space */ \
>> +    assert(__ret == (abi_ulong)__ret); \
>> +    (abi_ulong)__ret; \
>> +})
>>  
>>  #define saddr(x) g2h(x)
>>  #define laddr(x) g2h(x)
>> -- 
>> 1.6.0.2.GIT
>>
>>
>>
>>
> 



[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 258 bytes --]