On Friday, December 09, 2011 12:01:14 AM Stefan Hajnoczi wrote:
> 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.
But whats the issue in trying to recover in this case?
>
> > + return -ENOBUFS;
> > + }
>
> header->size is (signed) int and we didn't check for header->size < 0.
> Please use an unsigned type.
I will fix in next version
>
> > + 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).
I will fix in next version
