Patchwork linux-user: add string type in rtentry struct

login
register
mail settings
Submitter Laurent Vivier
Date Dec. 20, 2012, 8:56 p.m.
Message ID <1356036973-19376-1-git-send-email-laurent@vivier.eu>
Download mbox | patch
Permalink /patch/207706/
State New
Headers show

Comments

Laurent Vivier - Dec. 20, 2012, 8:56 p.m.
This allows to pass the device name.

You can test this with the "route" command.

WITHOUT this patch:

$ sudo route add -net default gw 10.0.3.1 eth0
SIOCADDRT: Bad address
$ netstat -nr
Kernel IP routing table
Destination     Gateway         Genmask         Flags   MSS Window  irtt Iface
10.0.3.0        0.0.0.0         255.255.255.0   U         0 0          0 eth0

WITH this patch:

$ sudo route add -net default gw 10.0.3.1 eth0
$ netstat -nr
Kernel IP routing table
Destination     Gateway         Genmask         Flags   MSS Window  irtt Iface
0.0.0.0         10.0.3.1        0.0.0.0         UG        0 0          0 eth0
10.0.3.0        0.0.0.0         255.255.255.0   U         0 0          0 eth0

Signed-off-by: Laurent Vivier <laurent@vivier.eu>
---
 linux-user/syscall.c       |   64 ++++++++++++++++++++++++++++++++++++++++++++
 linux-user/syscall_types.h |    4 ++-
 2 files changed, 67 insertions(+), 1 deletion(-)
Laurent Vivier - Jan. 1, 2013, 11:09 p.m.
Ping !

Le jeudi 20 décembre 2012 à 21:56 +0100, Laurent Vivier a écrit :
> This allows to pass the device name.
> 
> You can test this with the "route" command.
> 
> WITHOUT this patch:
> 
> $ sudo route add -net default gw 10.0.3.1 eth0
> SIOCADDRT: Bad address
> $ netstat -nr
> Kernel IP routing table
> Destination     Gateway         Genmask         Flags   MSS Window  irtt Iface
> 10.0.3.0        0.0.0.0         255.255.255.0   U         0 0          0 eth0
> 
> WITH this patch:
> 
> $ sudo route add -net default gw 10.0.3.1 eth0
> $ netstat -nr
> Kernel IP routing table
> Destination     Gateway         Genmask         Flags   MSS Window  irtt Iface
> 0.0.0.0         10.0.3.1        0.0.0.0         UG        0 0          0 eth0
> 10.0.3.0        0.0.0.0         255.255.255.0   U         0 0          0 eth0
> 
> Signed-off-by: Laurent Vivier <laurent@vivier.eu>
> ---
>  linux-user/syscall.c       |   64 ++++++++++++++++++++++++++++++++++++++++++++
>  linux-user/syscall_types.h |    4 ++-
>  2 files changed, 67 insertions(+), 1 deletion(-)
> 
> diff --git a/linux-user/syscall.c b/linux-user/syscall.c
> index 501002b..c2a2343 100644
> --- a/linux-user/syscall.c
> +++ b/linux-user/syscall.c
> @@ -3691,6 +3691,70 @@ static IOCTLEntry ioctl_entries[] = {
>      { 0, 0, },
>  };
>  
> +static void target_to_host_string (void *dst, const void *src)
> +{
> +#if HOST_LONG_BITS == 32 && TARGET_ABI_BITS == 32
> +    if (*(uint32_t*)src == 0) {
> +        *(uint32_t*)dst = 0;
> +	return;
> +    }
> +    *(uint32_t *)dst = (uint32_t)g2h(tswap32(*(uint32_t *)src));
> +#elif HOST_LONG_BITS == 64 && TARGET_ABI_BITS == 32
> +    if (*(uint32_t*)src == 0) {
> +        *(uint64_t*)dst = 0;
> +	return;
> +    }
> +    *(uint64_t *)dst = (uint64_t)g2h(tswap32(*(uint32_t *)src));
> +#elif HOST_LONG_BITS == 64 && TARGET_ABI_BITS == 64
> +    if (*(uint64_t*)src == 0) {
> +        *(uint64_t*)dst = 0;
> +	return;
> +    }
> +    *(uint64_t *)dst = (uint64_t)g2h(tswap64(*(uint64_t *)src));
> +#elif HOST_LONG_BITS == 32 && TARGET_ABI_BITS == 64
> +    if (*(uint64_t*)src == 0) {
> +        *(uint32_t*)dst = 0;
> +	return;
> +    }
> +    *(uint32_t *)dst = (uint32_t)g2h(tswap64(*(uint64_t *)src));
> +#endif
> +}
> +
> +static void host_to_target_string (void *dst, const void *src)
> +{
> +#if HOST_LONG_BITS == 32 && TARGET_ABI_BITS == 32
> +    if (*(uint32_t*)src == 0) {
> +        *(uint32_t*)dst = 0;
> +	return;
> +    }
> +    *(uint32_t *)dst = tswap32(h2g(*(uint32_t *)src));
> +#elif HOST_LONG_BITS == 64 && TARGET_ABI_BITS == 32
> +    if (*(uint64_t*)src == 0) {
> +        *(uint32_t*)dst = 0;
> +	return;
> +    }
> +    *(uint32_t *)dst = tswap32(h2g(*(uint64_t *)src));
> +#elif HOST_LONG_BITS == 64 && TARGET_ABI_BITS == 64
> +    if (*(uint64_t*)src == 0) {
> +        *(uint64_t*)dst = 0;
> +	return;
> +    }
> +    *(uint64_t *)dst = tswap64(h2g(*(uint64_t *)src));
> +#elif HOST_LONG_BITS == 32 && TARGET_ABI_BITS == 64
> +    if (*(uint32_t*)src == 0) {
> +        *(uint64_t*)dst = 0;
> +	return;
> +    }
> +    *(uint64_t *)dst = tswap64(h2g(*(uint32_t *)src));
> +#endif
> +}
> +
> +static const StructEntry struct_string_def = {
> +    .convert = { host_to_target_string, target_to_host_string },
> +    .size = { sizeof(target_long), sizeof(long) },
> +    .align = { __alignof__(target_long), __alignof__(long) },
> +};
> +
>  /* ??? Implement proper locking for ioctls.  */
>  /* do_ioctl() Must return target values and target errnos. */
>  static abi_long do_ioctl(int fd, abi_long cmd, abi_long arg)
> diff --git a/linux-user/syscall_types.h b/linux-user/syscall_types.h
> index 44b6a58..51fc023 100644
> --- a/linux-user/syscall_types.h
> +++ b/linux-user/syscall_types.h
> @@ -14,9 +14,11 @@ STRUCT(serial_icounter_struct,
>  STRUCT(sockaddr,
>         TYPE_SHORT, MK_ARRAY(TYPE_CHAR, 14))
>  
> +STRUCT_SPECIAL(string)
> +
>  STRUCT(rtentry,
>         TYPE_ULONG, MK_STRUCT(STRUCT_sockaddr), MK_STRUCT(STRUCT_sockaddr), MK_STRUCT(STRUCT_sockaddr),
> -       TYPE_SHORT, TYPE_SHORT, TYPE_ULONG, TYPE_PTRVOID, TYPE_SHORT, TYPE_PTRVOID,
> +       TYPE_SHORT, TYPE_SHORT, TYPE_ULONG, TYPE_PTRVOID, TYPE_SHORT, MK_STRUCT(STRUCT_string),
>         TYPE_ULONG, TYPE_ULONG, TYPE_SHORT)
>  
>  STRUCT(ifmap,
Peter Maydell - Jan. 1, 2013, 11:51 p.m.
On 20 December 2012 20:56, Laurent Vivier <laurent@vivier.eu> wrote:
> This allows to pass the device name.

> +static void target_to_host_string (void *dst, const void *src)
> +{
> +#if HOST_LONG_BITS == 32 && TARGET_ABI_BITS == 32
> +    if (*(uint32_t*)src == 0) {
> +        *(uint32_t*)dst = 0;
> +       return;
> +    }
> +    *(uint32_t *)dst = (uint32_t)g2h(tswap32(*(uint32_t *)src));
> +#elif HOST_LONG_BITS == 64 && TARGET_ABI_BITS == 32
> +    if (*(uint32_t*)src == 0) {
> +        *(uint64_t*)dst = 0;
> +       return;
> +    }
> +    *(uint64_t *)dst = (uint64_t)g2h(tswap32(*(uint32_t *)src));
> +#elif HOST_LONG_BITS == 64 && TARGET_ABI_BITS == 64
> +    if (*(uint64_t*)src == 0) {
> +        *(uint64_t*)dst = 0;
> +       return;
> +    }
> +    *(uint64_t *)dst = (uint64_t)g2h(tswap64(*(uint64_t *)src));
> +#elif HOST_LONG_BITS == 32 && TARGET_ABI_BITS == 64
> +    if (*(uint64_t*)src == 0) {
> +        *(uint32_t*)dst = 0;
> +       return;
> +    }
> +    *(uint32_t *)dst = (uint32_t)g2h(tswap64(*(uint64_t *)src));
> +#endif
> +}
> +
> +static void host_to_target_string (void *dst, const void *src)
> +{
> +#if HOST_LONG_BITS == 32 && TARGET_ABI_BITS == 32
> +    if (*(uint32_t*)src == 0) {
> +        *(uint32_t*)dst = 0;
> +       return;
> +    }
> +    *(uint32_t *)dst = tswap32(h2g(*(uint32_t *)src));
> +#elif HOST_LONG_BITS == 64 && TARGET_ABI_BITS == 32
> +    if (*(uint64_t*)src == 0) {
> +        *(uint32_t*)dst = 0;
> +       return;
> +    }
> +    *(uint32_t *)dst = tswap32(h2g(*(uint64_t *)src));
> +#elif HOST_LONG_BITS == 64 && TARGET_ABI_BITS == 64
> +    if (*(uint64_t*)src == 0) {
> +        *(uint64_t*)dst = 0;
> +       return;
> +    }
> +    *(uint64_t *)dst = tswap64(h2g(*(uint64_t *)src));
> +#elif HOST_LONG_BITS == 32 && TARGET_ABI_BITS == 64
> +    if (*(uint32_t*)src == 0) {
> +        *(uint64_t*)dst = 0;
> +       return;
> +    }
> +    *(uint64_t *)dst = tswap64(h2g(*(uint32_t *)src));
> +#endif
> +}
> +
> +static const StructEntry struct_string_def = {
> +    .convert = { host_to_target_string, target_to_host_string },
> +    .size = { sizeof(target_long), sizeof(long) },
> +    .align = { __alignof__(target_long), __alignof__(long) },
> +};

This is the wrong approach, I'm afraid. Among other problems,
you don't have anywhere to catch the case of being passed a bad pointer
(should fail EFAULT). In general, the thunk_convert routines aren't designed
to handle converting pointed-to data and your patch is abusing MK_STRUCT
(which is for embedded structs, not pointed-to anything). Unless you feel
like doing the redesign of the thunk conversion code, the right way to handle
oddball ioctl parameters is to write a do_ioctl_fn for them (compare
SIOCGIFCONF, for example).

The kernel sources:
http://lxr.linux.no/#linux+v3.7.1/net/socket.c#L3175
show that it always copies a fixed 15 bytes, incidentally.

-- PMM

Patch

diff --git a/linux-user/syscall.c b/linux-user/syscall.c
index 501002b..c2a2343 100644
--- a/linux-user/syscall.c
+++ b/linux-user/syscall.c
@@ -3691,6 +3691,70 @@  static IOCTLEntry ioctl_entries[] = {
     { 0, 0, },
 };
 
+static void target_to_host_string (void *dst, const void *src)
+{
+#if HOST_LONG_BITS == 32 && TARGET_ABI_BITS == 32
+    if (*(uint32_t*)src == 0) {
+        *(uint32_t*)dst = 0;
+	return;
+    }
+    *(uint32_t *)dst = (uint32_t)g2h(tswap32(*(uint32_t *)src));
+#elif HOST_LONG_BITS == 64 && TARGET_ABI_BITS == 32
+    if (*(uint32_t*)src == 0) {
+        *(uint64_t*)dst = 0;
+	return;
+    }
+    *(uint64_t *)dst = (uint64_t)g2h(tswap32(*(uint32_t *)src));
+#elif HOST_LONG_BITS == 64 && TARGET_ABI_BITS == 64
+    if (*(uint64_t*)src == 0) {
+        *(uint64_t*)dst = 0;
+	return;
+    }
+    *(uint64_t *)dst = (uint64_t)g2h(tswap64(*(uint64_t *)src));
+#elif HOST_LONG_BITS == 32 && TARGET_ABI_BITS == 64
+    if (*(uint64_t*)src == 0) {
+        *(uint32_t*)dst = 0;
+	return;
+    }
+    *(uint32_t *)dst = (uint32_t)g2h(tswap64(*(uint64_t *)src));
+#endif
+}
+
+static void host_to_target_string (void *dst, const void *src)
+{
+#if HOST_LONG_BITS == 32 && TARGET_ABI_BITS == 32
+    if (*(uint32_t*)src == 0) {
+        *(uint32_t*)dst = 0;
+	return;
+    }
+    *(uint32_t *)dst = tswap32(h2g(*(uint32_t *)src));
+#elif HOST_LONG_BITS == 64 && TARGET_ABI_BITS == 32
+    if (*(uint64_t*)src == 0) {
+        *(uint32_t*)dst = 0;
+	return;
+    }
+    *(uint32_t *)dst = tswap32(h2g(*(uint64_t *)src));
+#elif HOST_LONG_BITS == 64 && TARGET_ABI_BITS == 64
+    if (*(uint64_t*)src == 0) {
+        *(uint64_t*)dst = 0;
+	return;
+    }
+    *(uint64_t *)dst = tswap64(h2g(*(uint64_t *)src));
+#elif HOST_LONG_BITS == 32 && TARGET_ABI_BITS == 64
+    if (*(uint32_t*)src == 0) {
+        *(uint64_t*)dst = 0;
+	return;
+    }
+    *(uint64_t *)dst = tswap64(h2g(*(uint32_t *)src));
+#endif
+}
+
+static const StructEntry struct_string_def = {
+    .convert = { host_to_target_string, target_to_host_string },
+    .size = { sizeof(target_long), sizeof(long) },
+    .align = { __alignof__(target_long), __alignof__(long) },
+};
+
 /* ??? Implement proper locking for ioctls.  */
 /* do_ioctl() Must return target values and target errnos. */
 static abi_long do_ioctl(int fd, abi_long cmd, abi_long arg)
diff --git a/linux-user/syscall_types.h b/linux-user/syscall_types.h
index 44b6a58..51fc023 100644
--- a/linux-user/syscall_types.h
+++ b/linux-user/syscall_types.h
@@ -14,9 +14,11 @@  STRUCT(serial_icounter_struct,
 STRUCT(sockaddr,
        TYPE_SHORT, MK_ARRAY(TYPE_CHAR, 14))
 
+STRUCT_SPECIAL(string)
+
 STRUCT(rtentry,
        TYPE_ULONG, MK_STRUCT(STRUCT_sockaddr), MK_STRUCT(STRUCT_sockaddr), MK_STRUCT(STRUCT_sockaddr),
-       TYPE_SHORT, TYPE_SHORT, TYPE_ULONG, TYPE_PTRVOID, TYPE_SHORT, TYPE_PTRVOID,
+       TYPE_SHORT, TYPE_SHORT, TYPE_ULONG, TYPE_PTRVOID, TYPE_SHORT, MK_STRUCT(STRUCT_string),
        TYPE_ULONG, TYPE_ULONG, TYPE_SHORT)
 
 STRUCT(ifmap,