From mboxrd@z Thu Jan  1 00:00:00 1970
Received: from eggs.gnu.org ([140.186.70.92]:60714)
	by lists.gnu.org with esmtp (Exim 4.71)
	(envelope-from <stefanha@gmail.com>) id 1RYjC0-00025r-6J
	for qemu-devel@nongnu.org; Thu, 08 Dec 2011 13:59:41 -0500
Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71)
	(envelope-from <stefanha@gmail.com>) id 1RYjBy-0007G3-NH
	for qemu-devel@nongnu.org; Thu, 08 Dec 2011 13:59:40 -0500
Received: from mail-ww0-f53.google.com ([74.125.82.53]:46929)
	by eggs.gnu.org with esmtp (Exim 4.71)
	(envelope-from <stefanha@gmail.com>) id 1RYjBy-0007Fy-Is
	for qemu-devel@nongnu.org; Thu, 08 Dec 2011 13:59:38 -0500
Received: by wgbds1 with SMTP id ds1so3698014wgb.10
	for <qemu-devel@nongnu.org>; Thu, 08 Dec 2011 10:59:37 -0800 (PST)
Date: Thu, 8 Dec 2011 18:31:14 +0000
From: Stefan Hajnoczi <stefanha@gmail.com>
Message-ID: <20111208183114.GC20998@stefanha-thinkpad.localdomain>
References: <1323101930-27163-1-git-send-email-mohan@in.ibm.com>
	<1323101930-27163-5-git-send-email-mohan@in.ibm.com>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <1323101930-27163-5-git-send-email-mohan@in.ibm.com>
Subject: Re: [Qemu-devel] [PATCH V4 04/13] hw/9pfs: File system helper
 process for qemu 9p proxy FS
List-Id: <qemu-devel.nongnu.org>
List-Unsubscribe: <https://lists.nongnu.org/mailman/options/qemu-devel>,
	<mailto:qemu-devel-request@nongnu.org?subject=unsubscribe>
List-Archive: <http://lists.nongnu.org/archive/html/qemu-devel>
List-Post: <mailto:qemu-devel@nongnu.org>
List-Help: <mailto:qemu-devel-request@nongnu.org?subject=help>
List-Subscribe: <https://lists.nongnu.org/mailman/listinfo/qemu-devel>,
	<mailto:qemu-devel-request@nongnu.org?subject=subscribe>
To: "M. Mohan Kumar" <mohankumar.m@gmail.com>
Cc: "M. Mohan Kumar" <mohan@in.ibm.com>, qemu-devel@nongnu.org, aneesh.kumar@linux.vnet.ibm.com

On Mon, Dec 05, 2011 at 09:48:41PM +0530, M. Mohan Kumar wrote:
> +static int read_request(int sockfd, struct iovec *iovec, ProxyHeader *header)
> +{
> +    int retval;
> +
> +    /*
> +     * read the request header.
> +     */
> +    iovec->iov_len = 0;
> +    retval = socket_read(sockfd, iovec->iov_base, PROXY_HDR_SZ);
> +    if (retval < 0) {
> +        return retval;
> +    }
> +    iovec->iov_len = PROXY_HDR_SZ;
> +    retval = proxy_unmarshal(iovec, 0, "dd", &header->type, &header->size);
> +    if (retval < 0) {
> +        return retval;
> +    }
> +    /*
> +     * We can't process message.size > PROXY_MAX_IO_SZ, read the complete
> +     * message from the socket and ignore it. This ensures that
> +     * we can correctly handle the next request. We also return
> +     * ENOBUFS as error to indicate we ran out of buffer space.
> +     */
> +    if (header->size > PROXY_MAX_IO_SZ) {
> +        int count, size;
> +        size = header->size;
> +        while (size > 0) {
> +            count = MIN(PROXY_MAX_IO_SZ, size);
> +            count = socket_read(sockfd, iovec->iov_base + PROXY_HDR_SZ, count);
> +            if (count < 0) {
> +                return count;
> +            }
> +            size -= count;
> +        }

I'm not sure recovery attempts are worthwhile here.  The client is
buggy, perhaps just refuse further work.

> +        return -ENOBUFS;
> +    }

header->size is (signed) int and we didn't check for header->size < 0.
Please use an unsigned type.

> +    if (chroot(rpath) < 0) {
> +        do_perror("chroot");
> +        goto error;
> +    }
> +    umask(0);

We haven't changed into the chroot yet, we need chdir("/").  Otherwise
the current working directory is outside the chroot (and allows trivial
escape).