Re: [Qemu-devel] [PATCH V4 04/13] hw/9pfs: File system helper process for qemu 9p proxy FS

[Stefan Hajnoczi]

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).