[Qemu-devel] [PATCH] linux-user: correctly align target_epoll_event

[Qemu-devel] [PATCH] linux-user: correctly align target_epoll_event

[Laurent Vivier]

According to comments in /usr/include/linux/eventpoll.h,
poll_event is packed only on x86_64.

And to be sure fields are correctly aligned in epoll_data,
use abi_XXX types for all of them.

Moreover, fd type is wrong: fd is int, not ulong.

This has been tested with a ppc guest on an x86_64 host:
without this patch, systemd crashes (core).

CC: Alexander Graf <agraf@suse.de>
CC: Peter Maydell <peter.maydell@linaro.org>
Signed-off-by: Laurent Vivier <laurent@vivier.eu>
---
I cc' Alexander because it has added the padding and the packed attribute,
Peter because it is the author of the original structure.

 linux-user/syscall_defs.h | 19 +++++++++++--------
 1 file changed, 11 insertions(+), 8 deletions(-)

diff --git a/linux-user/syscall_defs.h b/linux-user/syscall_defs.h
index 7ca33a6..f4e4122 100644
--- a/linux-user/syscall_defs.h
+++ b/linux-user/syscall_defs.h
@@ -2503,20 +2503,23 @@ struct target_mq_attr {
 #define FUTEX_CMD_MASK          ~(FUTEX_PRIVATE_FLAG | FUTEX_CLOCK_REALTIME)
 
 #ifdef CONFIG_EPOLL
+#if defined(TARGET_X86_64)
+#define TARGET_EPOLL_PACKED QEMU_PACKED
+#else
+#define TARGET_EPOLL_PACKED
+#endif
+
 typedef union target_epoll_data {
     abi_ulong ptr;
-    abi_ulong fd;
-    uint32_t u32;
-    uint64_t u64;
+    abi_int fd;
+    abi_uint u32;
+    abi_ullong u64;
 } target_epoll_data_t;
 
 struct target_epoll_event {
-    uint32_t events;
-#if defined(TARGET_ARM) || defined(TARGET_MIPS) || defined(TARGET_MIPS64)
-    uint32_t __pad;
-#endif
+    abi_uint events;
     target_epoll_data_t data;
-} QEMU_PACKED;
+} TARGET_EPOLL_PACKED;
 #endif
 struct target_rlimit64 {
     uint64_t rlim_cur;
-- 
2.4.3

Re: [Qemu-devel] [PATCH] linux-user: correctly align target_epoll_event

[Riku Voipio]

On lauantaina 3. lokakuuta 2015 18.14.06 EEST, Laurent Vivier wrote:
> According to comments in /usr/include/linux/eventpoll.h,
> poll_event is packed only on x86_64.
>
> And to be sure fields are correctly aligned in epoll_data,
> use abi_XXX types for all of them.
>
> Moreover, fd type is wrong: fd is int, not ulong.
>
> This has been tested with a ppc guest on an x86_64 host:
> without this patch, systemd crashes (core).

Applied to linux-user, thanks

> CC: Alexander Graf <agraf@suse.de>
> CC: Peter Maydell <peter.maydell@linaro.org>
> Signed-off-by: Laurent Vivier <laurent@vivier.eu>
> ---
> I cc' Alexander because it has added the padding and the packed attribute,
> Peter because it is the author of the original structure.
>
>  linux-user/syscall_defs.h | 19 +++++++++++--------
>  1 file changed, 11 insertions(+), 8 deletions(-)
>
> diff --git a/linux-user/syscall_defs.h b/linux-user/syscall_defs.h
> index 7ca33a6..f4e4122 100644
> --- a/linux-user/syscall_defs.h
> +++ b/linux-user/syscall_defs.h
> @@ -2503,20 +2503,23 @@ struct target_mq_attr {
>  #define FUTEX_CMD_MASK          ~(FUTEX_PRIVATE_FLAG | 
> FUTEX_CLOCK_REALTIME)
>  
>  #ifdef CONFIG_EPOLL
> +#if defined(TARGET_X86_64)
> +#define TARGET_EPOLL_PACKED QEMU_PACKED
> +#else
> +#define TARGET_EPOLL_PACKED
> +#endif
> +
>  typedef union target_epoll_data {
>      abi_ulong ptr;
> -    abi_ulong fd;
> -    uint32_t u32;
> -    uint64_t u64;
> +    abi_int fd;
> +    abi_uint u32;
> +    abi_ullong u64;
>  } target_epoll_data_t;
>  
>  struct target_epoll_event {
> -    uint32_t events;
> -#if defined(TARGET_ARM) || defined(TARGET_MIPS) || defined(TARGET_MIPS64)
> -    uint32_t __pad;
> -#endif
> +    abi_uint events;
>      target_epoll_data_t data;
> -} QEMU_PACKED;
> +} TARGET_EPOLL_PACKED;
>  #endif
>  struct target_rlimit64 {
>      uint64_t rlim_cur;