From mboxrd@z Thu Jan  1 00:00:00 1970
Received: from mailman by lists.gnu.org with tmda-scanned (Exim 4.43)
	id 1G1IPh-0007Jp-1L
	for qemu-devel@nongnu.org; Fri, 14 Jul 2006 03:48:41 -0400
Received: from exim by lists.gnu.org with spam-scanned (Exim 4.43)
	id 1G1IPf-0007Jd-Gh
	for qemu-devel@nongnu.org; Fri, 14 Jul 2006 03:48:40 -0400
Received: from [199.232.76.173] (helo=monty-python.gnu.org)
	by lists.gnu.org with esmtp (Exim 4.43) id 1G1IPf-0007Ja-9u
	for qemu-devel@nongnu.org; Fri, 14 Jul 2006 03:48:39 -0400
Received: from [217.30.189.230] (helo=mail.nomovok.com)
	by monty-python.gnu.org with esmtps
	(TLS-1.0:DHE_RSA_AES_256_CBC_SHA:32) (Exim 4.52) id 1G1IRb-0003XD-ES
	for qemu-devel@nongnu.org; Fri, 14 Jul 2006 03:50:39 -0400
Received: from [85.77.96.209] (GYZMMCMVIII.dsl.saunalahti.fi [85.77.96.209])
	(using TLSv1 with cipher DHE-RSA-AES256-SHA (256/256 bits))
	(No client certificate requested)
	by mail.nomovok.com (Postfix) with ESMTP id 619E0D94208
	for <qemu-devel@nongnu.org>; Fri, 14 Jul 2006 10:48:29 +0300 (EEST)
Message-ID: <44B74C5A.6000408@nomovok.com>
Date: Fri, 14 Jul 2006 10:48:42 +0300
From: Pablo Virolainen <Pablo.Virolainen@nomovok.com>
MIME-Version: 1.0
Subject: Re: [Qemu-devel] Fix for accept
References: <44B61EBE.1090907@nomovok.com> <44B6AFB5.1080403@bellard.org>
In-Reply-To: <44B6AFB5.1080403@bellard.org>
Content-Type: text/plain; charset=ISO-8859-1
Content-Transfer-Encoding: 7bit
Reply-To: qemu-devel@nongnu.org
List-Id: qemu-devel.nongnu.org
List-Unsubscribe: <http://lists.nongnu.org/mailman/listinfo/qemu-devel>,
	<mailto:qemu-devel-request@nongnu.org?subject=unsubscribe>
List-Archive: <http://lists.gnu.org/pipermail/qemu-devel>
List-Post: <mailto:qemu-devel@nongnu.org>
List-Help: <mailto:qemu-devel-request@nongnu.org?subject=help>
List-Subscribe: <http://lists.nongnu.org/mailman/listinfo/qemu-devel>,
	<mailto:qemu-devel-request@nongnu.org?subject=subscribe>
To: qemu-devel@nongnu.org

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