RE: [Qemu-devel] [PATCH 12/17] linux-user: unix sockets - fix runningdbus

[Krumme, Chris]

Hello Riku, 

> -----Original Message-----
> From: 
> address@hidden 
> [mailto:address@hidden
> rg] On Behalf Of address@hidden
> Sent: Tuesday, March 31, 2009 3:41 PM
> To: address@hidden
> Subject: [Qemu-devel] [PATCH 12/17] linux-user: unix sockets 
> - fix runningdbus
> 
> From: Riku Voipio <address@hidden>
> 
> dbus sends too short (according to man 7 unix) addrlen for it's
> unix socket. I've been told that happens with other applications
> as well. Linux kernel doesn't appear to mind, so I guess
> we whould be tolerant as well. Expand sockaddr with +1 to fit
> the \0 of the pathname passed.
> 
> (scratchbox1 qemu had a very different workaround for the same issue).
> 
> Signed-off-by: Riku Voipio <address@hidden>
> ---
>  linux-user/syscall.c |   29 +++++++++++++++++++++++++++--
>  1 files changed, 27 insertions(+), 2 deletions(-)
> 
> diff --git a/linux-user/syscall.c b/linux-user/syscall.c
> index 56868ff..c6b0b74 100644
> --- a/linux-user/syscall.c
> +++ b/linux-user/syscall.c
> @@ -44,6 +44,7 @@
>  #include <signal.h>
>  #include <sched.h>
>  #include <sys/socket.h>
> +#include <sys/un.h>
>  #include <sys/uio.h>
>  #include <sys/poll.h>
>  #include <sys/times.h>
> @@ -735,13 +736,37 @@ static inline abi_long 
> target_to_host_sockaddr(struct sockaddr *addr,
>                                                 abi_ulong target_addr,
>                                                 socklen_t len)
>  {
> +    const socklen_t unix_maxlen = sizeof (struct sockaddr_un);
> +    sa_family_t sa_family;
>      struct target_sockaddr *target_saddr;
>  
>      target_saddr = lock_user(VERIFY_READ, target_addr, len, 1);
>      if (!target_saddr)
>          return -TARGET_EFAULT;
> +
> +    sa_family = tswap16(target_saddr->sa_family);
> +
> +    /* Oops. The caller might send a incomplete sun_path; sun_path
> +     * must be terminated by \0 (see the manual page), but
> +     * unfortunately it is quite common to specify sockaddr_un
> +     * length as "strlen(x->sun_path)" while it should be
> +     * "strlen(...) + 1". We'll fix that here if needed.
> +     * Linux kernel has a similar feature.
> +     */
> +
> +    if (sa_family == AF_UNIX) {
> +        if (len < unix_maxlen) {
> +            char *cp = (char*)target_saddr;
> +
> +            if ( cp[len-1] && !cp[len] )

This could be a little weird if len were 0.

Thanks

Chris

> +                len++;
> +        }
> +        if (len > unix_maxlen)
> +            len = unix_maxlen;
> +    }
> +
>      memcpy(addr, target_saddr, len);
> -    addr->sa_family = tswap16(target_saddr->sa_family);
> +    addr->sa_family = sa_family;
>      unlock_user(target_saddr, target_addr, 0);
>  
>      return 0;
> @@ -1195,7 +1220,7 @@ static abi_long do_bind(int sockfd, 
> abi_ulong target_addr,
>      if (addrlen < 0 || addrlen > MAX_SOCK_ADDR)
>          return -TARGET_EINVAL;
>  
> -    addr = alloca(addrlen);
> +    addr = alloca(addrlen+1);
>  
>      target_to_host_sockaddr(addr, target_addr, addrlen);
>      return get_errno(bind(sockfd, addr, addrlen));
> -- 
> 1.6.2.1
> 
> 
> 
>