Re: [Qemu-devel] Fix for accept

[Pablo Virolainen]

Fabrice Bellard wrote:

> Hi,
>
> OK for the bug report, but the fix is not correct because the problem
> is generic. [get|put]_user() and the other functions should be used
> everywhere to communicate with the "user" space and to generate the
> -EFAULT error if the address is not correct. For that purpose the host
> signal SIGSEGV can be catched and asm macros can be used to see if it
> is an expected seg fault (in this case [get|put]_user must return an
> error code) or if it is a QEMU bug. Note that exactly the same system
> is used inside the Linux kernel and I don't think it is necessary to
> invent something else.
>
> Regards,
>
> Fabrice.

Hello,

So I should write something like following instead?

        if (!get_user(addrlen,&target_addrlen)) {
          return -EFAULT
        }

The code seems to assume target_sockaddr == sockaddr, so why allocate
temporary buffer and then do copying?
One could implement SOCKOP_[accept|getsockname|getpeername] with same
code. Perhaps something like

static long do_socketcall_helper(target_ulong vptr,
                 int (*func)(int,struct sockaddr*,socklen_t*))
{
    const int n = sizeof(target_ulong);
    if (access_ok(VERIFY_READ,vptr,n*3)) {
        int ret;
        int sockfd=tgetl(vptr);
        target_ulong target_addr = tgetl(vptr + n);
        target_ulong target_addrlen = tgetl(vptr + 2 * n);
        struct sockaddr *addr=(struct sockaddr *)target_addr;
        socklen_t addrlen;
  
        if (!get_user(addrlen,&target_addrlen) ||
!acces_ok(VERIFY_WRITE,target_addrlen,4)) {
              return -EFAULT;
        }
   
        ret=get_errno(func(sockfd, addr, &addrlen));
        if (!is_error(ret)) {
            host_to_target_sockaddr(target_addr, addr, addrlen);
            tput32(target_addrlen, addrlen);
       }
        return ret;
    }
    return -EFAULT;
}

....

    case SOCKOP_accept:
        ret = do_socketcall_helper(vptr,accept);
        break;
    case SOCKOP_getsockname:
        ret = do_socketcall_helper(vptr,getsockname);
        break;
    case SOCKOP_getpeername:
        ret = do_socketcall_helper(vptr,getpeername);
        break;


Pablo