qemu-devel.nongnu.org archive mirror
 help / color / mirror / Atom feed
* [Qemu-devel] [PATCH 0/2] linux-user: dhclient support
@ 2012-12-31 19:37 Laurent Vivier
  2012-12-31 19:37 ` [Qemu-devel] [PATCH 1/2] linux-user: Add setsockopt(SO_ATTACH_FILTER) Laurent Vivier
  2012-12-31 19:38 ` [Qemu-devel] [PATCH 2/2] linux-user: SOCK_PACKET uses network endian to encode protocol in socket() Laurent Vivier
  0 siblings, 2 replies; 12+ messages in thread
From: Laurent Vivier @ 2012-12-31 19:37 UTC (permalink / raw)
  To: qemu-devel; +Cc: Riku Voipio

This two patches allow to use dhclient to configure IP addresses
in a linux container running the linux-user version of qemu.

[PATCH 1/2] linux-user: Add setsockopt(SO_ATTACH_FILTER)
[PATCH 2/2] linux-user: SOCK_PACKET uses network endian to encode

^ permalink raw reply	[flat|nested] 12+ messages in thread

* [Qemu-devel] [PATCH 1/2] linux-user: Add setsockopt(SO_ATTACH_FILTER)
  2012-12-31 19:37 [Qemu-devel] [PATCH 0/2] linux-user: dhclient support Laurent Vivier
@ 2012-12-31 19:37 ` Laurent Vivier
  2012-12-31 20:56   ` Peter Maydell
  2012-12-31 19:38 ` [Qemu-devel] [PATCH 2/2] linux-user: SOCK_PACKET uses network endian to encode protocol in socket() Laurent Vivier
  1 sibling, 1 reply; 12+ messages in thread
From: Laurent Vivier @ 2012-12-31 19:37 UTC (permalink / raw)
  To: qemu-devel; +Cc: Riku Voipio, Laurent Vivier

This is needed to be able to run dhclient.

Signed-off-by: Laurent Vivier <laurent@vivier.eu>
---
 linux-user/syscall.c      |   34 +++++++++++++++++++++++++++++++++-
 linux-user/syscall_defs.h |   12 ++++++++++++
 2 files changed, 45 insertions(+), 1 deletion(-)

diff --git a/linux-user/syscall.c b/linux-user/syscall.c
index e99adab..000b640 100644
--- a/linux-user/syscall.c
+++ b/linux-user/syscall.c
@@ -98,6 +98,7 @@ int __clone2(int (*fn)(void *), void *child_stack_base,
 #include <linux/fb.h>
 #include <linux/vt.h>
 #include <linux/dm-ioctl.h>
+#include <linux/filter.h>
 #include "linux_loop.h"
 #include "cpu-uname.h"
 
@@ -1491,6 +1492,38 @@ static abi_long do_setsockopt(int sockfd, int level, int optname,
         break;
     case TARGET_SOL_SOCKET:
         switch (optname) {
+        case TARGET_SO_ATTACH_FILTER: {
+                struct target_sock_fprog *tfprog;
+                struct target_sock_filter *tfilter;
+                struct sock_fprog fprog;
+                struct sock_filter *filter;
+                int i;
+
+                if (optlen != sizeof(*tfprog))
+                    return -TARGET_EINVAL;
+                if (!lock_user_struct(VERIFY_READ, tfprog, optval_addr, 0))
+                    return -TARGET_EFAULT;
+                if (!lock_user_struct(VERIFY_READ, tfilter,
+                                      tswapal(tfprog->filter), 0))
+                    return -TARGET_EFAULT;
+
+                fprog.len = tswap16(tfprog->len);
+                filter = alloca(fprog.len * sizeof(*filter));
+                for (i = 0; i < fprog.len; i ++) {
+                    filter[i].code = tswap16(tfilter[i].code);
+                    filter[i].jt = tfilter[i].jt;
+                    filter[i].jf = tfilter[i].jf;
+                    filter[i].k = tswap32(tfilter[i].k);
+                }
+                fprog.filter = filter;
+
+                ret = get_errno(setsockopt(sockfd, SOL_SOCKET,
+                                SO_ATTACH_FILTER, &fprog, sizeof(fprog)));
+
+                unlock_user_struct(tfilter, tfprog->filter, 1);
+                unlock_user_struct(tfprog, optval_addr, 1);
+                return ret;
+        }
             /* Options with 'int' argument.  */
         case TARGET_SO_DEBUG:
 		optname = SO_DEBUG;
@@ -1548,7 +1581,6 @@ static abi_long do_setsockopt(int sockfd, int level, int optname,
         case TARGET_SO_SNDTIMEO:
 		optname = SO_SNDTIMEO;
 		break;
-            break;
         default:
             goto unimplemented;
         }
diff --git a/linux-user/syscall_defs.h b/linux-user/syscall_defs.h
index d4589e7..501735f 100644
--- a/linux-user/syscall_defs.h
+++ b/linux-user/syscall_defs.h
@@ -119,6 +119,18 @@ struct target_sockaddr {
     uint8_t sa_data[14];
 };
 
+struct target_sock_filter {
+    target_ushort code;
+    uint8_t jt;
+    uint8_t jf;
+    target_uint k;
+};
+
+struct target_sock_fprog {
+    target_ushort len;
+    abi_ulong filter;
+};
+
 struct target_in_addr {
     uint32_t s_addr; /* big endian */
 };
-- 
1.7.10.4

^ permalink raw reply related	[flat|nested] 12+ messages in thread

* [Qemu-devel] [PATCH 2/2] linux-user: SOCK_PACKET uses network endian to encode protocol in socket()
  2012-12-31 19:37 [Qemu-devel] [PATCH 0/2] linux-user: dhclient support Laurent Vivier
  2012-12-31 19:37 ` [Qemu-devel] [PATCH 1/2] linux-user: Add setsockopt(SO_ATTACH_FILTER) Laurent Vivier
@ 2012-12-31 19:38 ` Laurent Vivier
  2012-12-31 21:32   ` Peter Maydell
  1 sibling, 1 reply; 12+ messages in thread
From: Laurent Vivier @ 2012-12-31 19:38 UTC (permalink / raw)
  To: qemu-devel; +Cc: Riku Voipio, Laurent Vivier

From: Laurent Vivier <Laurent@Vivier.EU>

in PACKET(7) :
                                 protocol is the  IEEE  802.3  protocol
number in network order.  See the <linux/if_ether.h> include file for a
list of allowed protocols.  When protocol is  set  to  htons(ETH_P_ALL)
then all protocols are received.  All incoming packets of that protocol
type will be passed to the packet socket before they are passed to  the
protocols implemented in the kernel.

Signed-off-by: Laurent Vivier <Laurent@Vivier.EU>
---
 include/exec/user/abitypes.h |   22 ++++++++++++++++++++++
 linux-user/syscall.c         |    8 +++++++-
 2 files changed, 29 insertions(+), 1 deletion(-)

diff --git a/include/exec/user/abitypes.h b/include/exec/user/abitypes.h
index fe7f662..f4f526a 100644
--- a/include/exec/user/abitypes.h
+++ b/include/exec/user/abitypes.h
@@ -15,6 +15,15 @@ static inline abi_ulong tswapal(abi_ulong v)
     return tswap32(v);
 }
 
+static inline abi_ulong abi_ntohl(abi_ulong v)
+{
+#if defined(HOST_BIG_ENDIAN)
+    return v;
+#else
+    return bswap_32(v);
+#endif
+}
+
 #else
 typedef target_ulong abi_ulong;
 typedef target_long abi_long;
@@ -32,5 +41,18 @@ static inline abi_ulong tswapal(abi_ulong v)
     return tswapl(v);
 }
 
+static inline abi_ulong abi_ntohl(abi_ulong v)
+{
+#if defined(HOST_BIG_ENDIAN)
+    return v;
+#else
+#if TARGET_LONG_SIZE == 4
+    return bswap_32(v);
+#else
+    return bswap_64(v);
+#endif
+#endif
+}
+
 #endif
 #endif
diff --git a/linux-user/syscall.c b/linux-user/syscall.c
index 000b640..29151a6 100644
--- a/linux-user/syscall.c
+++ b/linux-user/syscall.c
@@ -1874,7 +1874,7 @@ static void unlock_iovec(struct iovec *vec, abi_ulong target_addr,
 }
 
 /* do_socket() Must return target values and target errnos. */
-static abi_long do_socket(int domain, int type, int protocol)
+static abi_long do_socket(int domain, int type, abi_ulong protocol)
 {
 #if defined(TARGET_MIPS)
     switch(type) {
@@ -1900,6 +1900,12 @@ static abi_long do_socket(int domain, int type, int protocol)
 #endif
     if (domain == PF_NETLINK)
         return -EAFNOSUPPORT; /* do not NETLINK socket connections possible */
+    if (type == SOCK_PACKET) {
+        /* in this case, socket() needs a network endian short */
+        protocol = tswapal(protocol); /* restore network endian long */
+        protocol = abi_ntohl(protocol); /* a host endian long */
+        protocol = htons(protocol); /* network endian short */
+    }
     return get_errno(socket(domain, type, protocol));
 }
 
-- 
1.7.10.4

^ permalink raw reply related	[flat|nested] 12+ messages in thread

* Re: [Qemu-devel] [PATCH 1/2] linux-user: Add setsockopt(SO_ATTACH_FILTER)
  2012-12-31 19:37 ` [Qemu-devel] [PATCH 1/2] linux-user: Add setsockopt(SO_ATTACH_FILTER) Laurent Vivier
@ 2012-12-31 20:56   ` Peter Maydell
  0 siblings, 0 replies; 12+ messages in thread
From: Peter Maydell @ 2012-12-31 20:56 UTC (permalink / raw)
  To: Laurent Vivier; +Cc: Riku Voipio, qemu-devel

On 31 December 2012 19:37, Laurent Vivier <laurent@vivier.eu> wrote:
> This is needed to be able to run dhclient.
>
> Signed-off-by: Laurent Vivier <laurent@vivier.eu>
> ---
>  linux-user/syscall.c      |   34 +++++++++++++++++++++++++++++++++-
>  linux-user/syscall_defs.h |   12 ++++++++++++
>  2 files changed, 45 insertions(+), 1 deletion(-)
>
> diff --git a/linux-user/syscall.c b/linux-user/syscall.c
> index e99adab..000b640 100644
> --- a/linux-user/syscall.c
> +++ b/linux-user/syscall.c
> @@ -98,6 +98,7 @@ int __clone2(int (*fn)(void *), void *child_stack_base,
>  #include <linux/fb.h>
>  #include <linux/vt.h>
>  #include <linux/dm-ioctl.h>
> +#include <linux/filter.h>
>  #include "linux_loop.h"
>  #include "cpu-uname.h"
>
> @@ -1491,6 +1492,38 @@ static abi_long do_setsockopt(int sockfd, int level, int optname,
>          break;
>      case TARGET_SOL_SOCKET:
>          switch (optname) {
> +        case TARGET_SO_ATTACH_FILTER: {

This brace should go on a line of its own (lined up with the 'c' in case) IMHO.

> +                struct target_sock_fprog *tfprog;
> +                struct target_sock_filter *tfilter;
> +                struct sock_fprog fprog;
> +                struct sock_filter *filter;
> +                int i;
> +
> +                if (optlen != sizeof(*tfprog))
> +                    return -TARGET_EINVAL;

QEMU style requires braces on this if. You can use checkpatch.pl
to catch this kind of thing.

> +                if (!lock_user_struct(VERIFY_READ, tfprog, optval_addr, 0))
> +                    return -TARGET_EFAULT;
> +                if (!lock_user_struct(VERIFY_READ, tfilter,
> +                                      tswapal(tfprog->filter), 0))
> +                    return -TARGET_EFAULT;

This will fail to unlock tfprog in the failure case.

> +
> +                fprog.len = tswap16(tfprog->len);
> +                filter = alloca(fprog.len * sizeof(*filter));

Not sure an unconstrained-size alloca based on data from
the guest binary is a fantastic idea (though we no doubt
do something similar for some other syscalls).

> +                for (i = 0; i < fprog.len; i ++) {
> +                    filter[i].code = tswap16(tfilter[i].code);
> +                    filter[i].jt = tfilter[i].jt;
> +                    filter[i].jf = tfilter[i].jf;
> +                    filter[i].k = tswap32(tfilter[i].k);
> +                }
> +                fprog.filter = filter;
> +
> +                ret = get_errno(setsockopt(sockfd, SOL_SOCKET,
> +                                SO_ATTACH_FILTER, &fprog, sizeof(fprog)));
> +
> +                unlock_user_struct(tfilter, tfprog->filter, 1);
> +                unlock_user_struct(tfprog, optval_addr, 1);
> +                return ret;
> +        }
>              /* Options with 'int' argument.  */
>          case TARGET_SO_DEBUG:
>                 optname = SO_DEBUG;
> @@ -1548,7 +1581,6 @@ static abi_long do_setsockopt(int sockfd, int level, int optname,
>          case TARGET_SO_SNDTIMEO:
>                 optname = SO_SNDTIMEO;
>                 break;
> -            break;

Nice catch, but this is an unrelated change that should go in its own patch.

>          default:
>              goto unimplemented;
>          }

-- PMM

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [Qemu-devel] [PATCH 2/2] linux-user: SOCK_PACKET uses network endian to encode protocol in socket()
  2012-12-31 19:38 ` [Qemu-devel] [PATCH 2/2] linux-user: SOCK_PACKET uses network endian to encode protocol in socket() Laurent Vivier
@ 2012-12-31 21:32   ` Peter Maydell
  2012-12-31 22:19     ` Laurent Vivier
  0 siblings, 1 reply; 12+ messages in thread
From: Peter Maydell @ 2012-12-31 21:32 UTC (permalink / raw)
  To: Laurent Vivier; +Cc: Riku Voipio, qemu-devel

On 31 December 2012 19:38, Laurent Vivier <laurent@vivier.eu> wrote:
> @@ -1900,6 +1900,12 @@ static abi_long do_socket(int domain, int type, int protocol)
>  #endif
>      if (domain == PF_NETLINK)
>          return -EAFNOSUPPORT; /* do not NETLINK socket connections possible */
> +    if (type == SOCK_PACKET) {
> +        /* in this case, socket() needs a network endian short */
> +        protocol = tswapal(protocol); /* restore network endian long */
> +        protocol = abi_ntohl(protocol); /* a host endian long */
> +        protocol = htons(protocol); /* network endian short */
> +    }

Are you sure this is correct for little endian guests? I've only
desk-checked it rather than running a test program, but it looks
to me like you end up passing the wrong value to socket().

Also it seems rather involved since we swap things three times and
have an entirely new abi_* function. Either I'm completely confused
or it should be enough to just have

if (type == SOCK_PACKET) {
      protocol = tswap16(protocol);
}

-- PMM

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [Qemu-devel] [PATCH 2/2] linux-user: SOCK_PACKET uses network endian to encode protocol in socket()
  2012-12-31 21:32   ` Peter Maydell
@ 2012-12-31 22:19     ` Laurent Vivier
  2013-01-01 15:03       ` Peter Maydell
  0 siblings, 1 reply; 12+ messages in thread
From: Laurent Vivier @ 2012-12-31 22:19 UTC (permalink / raw)
  To: Peter Maydell; +Cc: Riku Voipio, qemu-devel

Le lundi 31 décembre 2012 à 21:32 +0000, Peter Maydell a écrit :
> On 31 December 2012 19:38, Laurent Vivier <laurent@vivier.eu> wrote:
> > @@ -1900,6 +1900,12 @@ static abi_long do_socket(int domain, int type, int protocol)
> >  #endif
> >      if (domain == PF_NETLINK)
> >          return -EAFNOSUPPORT; /* do not NETLINK socket connections possible */
> > +    if (type == SOCK_PACKET) {
> > +        /* in this case, socket() needs a network endian short */
> > +        protocol = tswapal(protocol); /* restore network endian long */
> > +        protocol = abi_ntohl(protocol); /* a host endian long */
> > +        protocol = htons(protocol); /* network endian short */
> > +    }
> 
> Are you sure this is correct for little endian guests? I've only
> desk-checked it rather than running a test program, but it looks
> to me like you end up passing the wrong value to socket().

I tried to find a solution working in every case.

> Also it seems rather involved since we swap things three times and
> have an entirely new abi_* function. Either I'm completely confused
> or it should be enough to just have
> 
> if (type == SOCK_PACKET) {
>       protocol = tswap16(protocol);
> }

works... sometime. In fact, work if target endianess is network endianess.

Correct me if I'm wrong.

target          host
little endian / big endian

memory   00 00 00 03
protocol 03000000
tswap16  00000000 -> don't work

tswapal()   00000003
abi_ntohl() 00000003
htons()     00000003 -> work

big endian / little endian:

memory    00 00 00 03
protocol  00000003
tswap16() 00000300 -> work

tswapal() 03000000
abi_ntohl() 00000003
htons()     00000300 -> work

little endian/little endian:

memory: 00 00 00 03 (network endian)
protocol : 03000000
tswap16() : 00000000 -> don't work

tswapal() 03000000
abi_ntohl() 00000003
htons()     00000300 -> work

big endian / big endian

memory 00 00 00 03
protocol 00000003
tswap16() 00000003 -> work

tswapal() 00000003
abi_ntohl() 00000003
htons()     00000003 -> work

Laurent

-- 
"Just play. Have fun. Enjoy the game."
- Michael Jordan

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [Qemu-devel] [PATCH 2/2] linux-user: SOCK_PACKET uses network endian to encode protocol in socket()
  2012-12-31 22:19     ` Laurent Vivier
@ 2013-01-01 15:03       ` Peter Maydell
  2013-01-01 17:27         ` Laurent Vivier
  0 siblings, 1 reply; 12+ messages in thread
From: Peter Maydell @ 2013-01-01 15:03 UTC (permalink / raw)
  To: Laurent Vivier; +Cc: Riku Voipio, qemu-devel

On 31 December 2012 22:19, Laurent Vivier <Laurent@vivier.eu> wrote:
> Le lundi 31 décembre 2012 à 21:32 +0000, Peter Maydell a écrit :
>> Also it seems rather involved since we swap things three times and
>> have an entirely new abi_* function. Either I'm completely confused
>> or it should be enough to just have
>>
>> if (type == SOCK_PACKET) {
>>       protocol = tswap16(protocol);
>> }

Looking more carefully at packet(7) this is actually the wrong
guard anyway. You need to check for
 (domain == AF_PACKET) || (type == SOCK_PACKET)

since SOCK_PACKET is the obsolete Linux 2.0 way of doing packet sockets.

> works... sometime. In fact, work if target endianess is network endianess.
>
> Correct me if I'm wrong.
>
> target          host
> little endian / big endian
>
> memory   00 00 00 03

Syscall arguments aren't generally passed in memory, they're
in registers (and if they were pased in memory for some architecture
then that arch would do a load-and-swap-from-memory in main.c).
So the value you see in do_socket() is always "the integer passed
as a syscall parameter, as a host-order integer".

So in this case, with a simple guest program:
#include <sys/socket.h>
#include <netpacket/packet.h>
#include <net/ethernet.h>
#include <arpa/inet.h>

int main(void) {
   return socket(AF_PACKET, SOCK_RAW, htons(ETH_P_ALL));
}

you will find that do_socket() in QEMU is passed either 0x3 [if the
guest is bigendian and the guest htons() is a no-op] or 0x0300
[if the guest is littleendian]. Since what we want to pass to the
host socket() call is 0x3 if the host is bigendian and 0x0300 if
the host is little endian, this amounts to needing to do a 16 bit
byteswap if the host and guest are different endianness, which
is exactly what tswap16() does. I checked with i386-to-i386
that do_socket() gets passed 0x300 and we correctly send it
through to the host socket().

-- PMM

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [Qemu-devel] [PATCH 2/2] linux-user: SOCK_PACKET uses network endian to encode protocol in socket()
  2013-01-01 15:03       ` Peter Maydell
@ 2013-01-01 17:27         ` Laurent Vivier
  2013-01-01 18:37           ` Laurent Vivier
  0 siblings, 1 reply; 12+ messages in thread
From: Laurent Vivier @ 2013-01-01 17:27 UTC (permalink / raw)
  To: Peter Maydell; +Cc: Riku Voipio, qemu-devel

Le mardi 01 janvier 2013 à 15:03 +0000, Peter Maydell a écrit :
> On 31 December 2012 22:19, Laurent Vivier <Laurent@vivier.eu> wrote:
> > Le lundi 31 décembre 2012 à 21:32 +0000, Peter Maydell a écrit :
> >> Also it seems rather involved since we swap things three times and
> >> have an entirely new abi_* function. Either I'm completely confused
> >> or it should be enough to just have
> >>
> >> if (type == SOCK_PACKET) {
> >>       protocol = tswap16(protocol);
> >> }
> 
> Looking more carefully at packet(7) this is actually the wrong
> guard anyway. You need to check for
>  (domain == AF_PACKET) || (type == SOCK_PACKET)

I agree.

> since SOCK_PACKET is the obsolete Linux 2.0 way of doing packet sockets.

But dhclient is always using this...

> > works... sometime. In fact, work if target endianess is network endianess.
> >
> > Correct me if I'm wrong.
> >
> > target          host
> > little endian / big endian
> >
> > memory   00 00 00 03
> 
> Syscall arguments aren't generally passed in memory, they're
> in registers (and if they were pased in memory for some architecture
> then that arch would do a load-and-swap-from-memory in main.c).
> So the value you see in do_socket() is always "the integer passed
> as a syscall parameter, as a host-order integer".

Yes, I missed that.

> So in this case, with a simple guest program:
> #include <sys/socket.h>
> #include <netpacket/packet.h>
> #include <net/ethernet.h>
> #include <arpa/inet.h>
> 
> int main(void) {
>    return socket(AF_PACKET, SOCK_RAW, htons(ETH_P_ALL));
> }
> 
> you will find that do_socket() in QEMU is passed either 0x3 [if the
> guest is bigendian and the guest htons() is a no-op] or 0x0300
> [if the guest is littleendian]. Since what we want to pass to the
> host socket() call is 0x3 if the host is bigendian and 0x0300 if
> the host is little endian, this amounts to needing to do a 16 bit
> byteswap if the host and guest are different endianness, which
> is exactly what tswap16() does. I checked with i386-to-i386
> that do_socket() gets passed 0x300 and we correctly send it
> through to the host socket().

Yes, I agree. I correct the patch.

Thank you,
Laurent

-- 
"Just play. Have fun. Enjoy the game."
- Michael Jordan

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [Qemu-devel] [PATCH 2/2] linux-user: SOCK_PACKET uses network endian to encode protocol in socket()
  2013-01-01 17:27         ` Laurent Vivier
@ 2013-01-01 18:37           ` Laurent Vivier
  2013-01-01 19:45             ` Peter Maydell
  0 siblings, 1 reply; 12+ messages in thread
From: Laurent Vivier @ 2013-01-01 18:37 UTC (permalink / raw)
  To: Peter Maydell; +Cc: Riku Voipio, qemu-devel

Le mardi 01 janvier 2013 à 18:27 +0100, Laurent Vivier a écrit :
> Le mardi 01 janvier 2013 à 15:03 +0000, Peter Maydell a écrit :
> > On 31 December 2012 22:19, Laurent Vivier <Laurent@vivier.eu> wrote:
> > > works... sometime. In fact, work if target endianess is network endianess.
> > >
> > > Correct me if I'm wrong.
> > >
> > > target          host
> > > little endian / big endian
> > >
> > > memory   00 00 00 03
> > 
> > Syscall arguments aren't generally passed in memory, they're
> > in registers (and if they were pased in memory for some architecture
> > then that arch would do a load-and-swap-from-memory in main.c).
> > So the value you see in do_socket() is always "the integer passed
> > as a syscall parameter, as a host-order integer".
> 
> Yes, I missed that.

But, in fact, for socketcall(), they are read from memory :

        static abi_long do_socketcall(int num, abi_ulong vptr)
        {
            abi_long ret;
            const int n = sizeof(abi_ulong);
        
            switch(num) {
            case SOCKOP_socket:
                {
                    abi_ulong domain, type, protocol;
        
                    if (get_user_ual(domain, vptr)
                        || get_user_ual(type, vptr + n)
                        || get_user_ual(protocol, vptr + 2 * n))
                        return -TARGET_EFAULT;
        
                    ret = do_socket(domain, type, protocol);
                }
                break;


So, I don't know if "tswap16()" is always correct. It works for
m68k-to-x86_64, but I don't understand how it can works for
i386-to-i386.

Your opinion ?

Regards,
Laurent

-- 
"Just play. Have fun. Enjoy the game."
- Michael Jordan

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [Qemu-devel] [PATCH 2/2] linux-user: SOCK_PACKET uses network endian to encode protocol in socket()
  2013-01-01 18:37           ` Laurent Vivier
@ 2013-01-01 19:45             ` Peter Maydell
  2013-01-01 22:12               ` Laurent Vivier
  0 siblings, 1 reply; 12+ messages in thread
From: Peter Maydell @ 2013-01-01 19:45 UTC (permalink / raw)
  To: Laurent Vivier; +Cc: Riku Voipio, qemu-devel

On 1 January 2013 18:37, Laurent Vivier <Laurent@vivier.eu> wrote:
> Le mardi 01 janvier 2013 à 18:27 +0100, Laurent Vivier a écrit :
>> Le mardi 01 janvier 2013 à 15:03 +0000, Peter Maydell a écrit :
>> > On 31 December 2012 22:19, Laurent Vivier <Laurent@vivier.eu> wrote:
>> > > works... sometime. In fact, work if target endianess is network endianess.
>> > >
>> > > Correct me if I'm wrong.
>> > >
>> > > target          host
>> > > little endian / big endian
>> > >
>> > > memory   00 00 00 03
>> >
>> > Syscall arguments aren't generally passed in memory, they're
>> > in registers (and if they were pased in memory for some architecture
>> > then that arch would do a load-and-swap-from-memory in main.c).
>> > So the value you see in do_socket() is always "the integer passed
>> > as a syscall parameter, as a host-order integer".
>>
>> Yes, I missed that.
>
> But, in fact, for socketcall(), they are read from memory :

Yes, this is because socketcall is weird. The actual kernel
implementation also reads them from memory:
  http://lxr.linux.no/#linux+v3.7.1/net/socket.c#L2443
as an array of unsigned longs. So as long as qemu also reads
them out of memory as an array of target abi_ulongs (which as
you can see we do) then we'll retrieve the same value (0x3 or
0x300) to pass to do_socket() as the guest program wrote into
its guest view of memory (since it should have written an
unsigned long). (What is happening here is that the guest
binary writes the protocol value to memory as an unsigned
long, so it goes in as 4 bytes in whichever order the guest uses;
qemu's get_user_ual() then rereads those 4 bytes, swapping
the value back so we get the same integer value the guest
program stored. Note that the guest doesn't write the protocol
argument as a 2 byte value!)

I would encourage you to write some simple test programs
and check them using strace (both of the native program and
of qemu running the program).

-- PMM

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [Qemu-devel] [PATCH 2/2] linux-user: SOCK_PACKET uses network endian to encode protocol in socket()
  2013-01-01 19:45             ` Peter Maydell
@ 2013-01-01 22:12               ` Laurent Vivier
  2013-01-01 22:50                 ` Peter Maydell
  0 siblings, 1 reply; 12+ messages in thread
From: Laurent Vivier @ 2013-01-01 22:12 UTC (permalink / raw)
  To: Peter Maydell; +Cc: Riku Voipio, qemu-devel

Le mardi 01 janvier 2013 à 19:45 +0000, Peter Maydell a écrit :
> On 1 January 2013 18:37, Laurent Vivier <Laurent@vivier.eu> wrote:
> > Le mardi 01 janvier 2013 à 18:27 +0100, Laurent Vivier a écrit :
> >> Le mardi 01 janvier 2013 à 15:03 +0000, Peter Maydell a écrit :
> >> > On 31 December 2012 22:19, Laurent Vivier <Laurent@vivier.eu> wrote:
> >> > > works... sometime. In fact, work if target endianess is network endianess.
> >> > >
> >> > > Correct me if I'm wrong.
> >> > >
> >> > > target          host
> >> > > little endian / big endian
> >> > >
> >> > > memory   00 00 00 03
> >> >
> >> > Syscall arguments aren't generally passed in memory, they're
> >> > in registers (and if they were pased in memory for some architecture
> >> > then that arch would do a load-and-swap-from-memory in main.c).
> >> > So the value you see in do_socket() is always "the integer passed
> >> > as a syscall parameter, as a host-order integer".
> >>
> >> Yes, I missed that.
> >
> > But, in fact, for socketcall(), they are read from memory :
> 
> Yes, this is because socketcall is weird. The actual kernel
> implementation also reads them from memory:
>   http://lxr.linux.no/#linux+v3.7.1/net/socket.c#L2443
> as an array of unsigned longs. So as long as qemu also reads
> them out of memory as an array of target abi_ulongs (which as
> you can see we do) then we'll retrieve the same value (0x3 or
> 0x300) to pass to do_socket() as the guest program wrote into
> its guest view of memory (since it should have written an
> unsigned long). (What is happening here is that the guest
> binary writes the protocol value to memory as an unsigned
> long, so it goes in as 4 bytes in whichever order the guest uses;
> qemu's get_user_ual() then rereads those 4 bytes, swapping
> the value back so we get the same integer value the guest
> program stored. Note that the guest doesn't write the protocol
> argument as a 2 byte value!)
> 
> I would encourage you to write some simple test programs
> and check them using strace (both of the native program and
> of qemu running the program).

OK, I will... but I think we will fall back to my original patch ;-)

Regards,
Laurent

-- 
"Just play. Have fun. Enjoy the game."
- Michael Jordan

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [Qemu-devel] [PATCH 2/2] linux-user: SOCK_PACKET uses network endian to encode protocol in socket()
  2013-01-01 22:12               ` Laurent Vivier
@ 2013-01-01 22:50                 ` Peter Maydell
  0 siblings, 0 replies; 12+ messages in thread
From: Peter Maydell @ 2013-01-01 22:50 UTC (permalink / raw)
  To: Laurent Vivier; +Cc: Riku Voipio, qemu-devel

On 1 January 2013 22:12, Laurent Vivier <Laurent@vivier.eu> wrote:
> Le mardi 01 janvier 2013 à 19:45 +0000, Peter Maydell a écrit :
>> I would encourage you to write some simple test programs
>> and check them using strace (both of the native program and
>> of qemu running the program).
>
> OK, I will... but I think we will fall back to my original patch ;-)

You'll need to provide test programs which work with that and
fail with the simple tswap16() approach first.

-- PMM

^ permalink raw reply	[flat|nested] 12+ messages in thread

end of thread, other threads:[~2013-01-01 22:50 UTC | newest]

Thread overview: 12+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2012-12-31 19:37 [Qemu-devel] [PATCH 0/2] linux-user: dhclient support Laurent Vivier
2012-12-31 19:37 ` [Qemu-devel] [PATCH 1/2] linux-user: Add setsockopt(SO_ATTACH_FILTER) Laurent Vivier
2012-12-31 20:56   ` Peter Maydell
2012-12-31 19:38 ` [Qemu-devel] [PATCH 2/2] linux-user: SOCK_PACKET uses network endian to encode protocol in socket() Laurent Vivier
2012-12-31 21:32   ` Peter Maydell
2012-12-31 22:19     ` Laurent Vivier
2013-01-01 15:03       ` Peter Maydell
2013-01-01 17:27         ` Laurent Vivier
2013-01-01 18:37           ` Laurent Vivier
2013-01-01 19:45             ` Peter Maydell
2013-01-01 22:12               ` Laurent Vivier
2013-01-01 22:50                 ` Peter Maydell

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).