xfs
[Top] [All Lists]

Re: [PATCH 09/18] nfsd: implement pNFS operations

To: Christoph Hellwig <hch@xxxxxx>
Subject: Re: [PATCH 09/18] nfsd: implement pNFS operations
From: Tom Haynes <thomas.haynes@xxxxxxxxxxxxxxx>
Date: Mon, 12 Jan 2015 09:54:11 -0800
Cc: "J. Bruce Fields" <bfields@xxxxxxxxxxxx>, Jeff Layton <jlayton@xxxxxxxxxxxxxxx>, linux-nfs@xxxxxxxxxxxxxxx, linux-fsdevel@xxxxxxxxxxxxxxx, xfs@xxxxxxxxxxx
Delivered-to: xfs@xxxxxxxxxxx
In-reply-to: <1420561721-9150-10-git-send-email-hch@xxxxxx>
References: <1420561721-9150-1-git-send-email-hch@xxxxxx> <1420561721-9150-10-git-send-email-hch@xxxxxx>
User-agent: Mutt/1.5.23 (2014-03-12)
On Tue, Jan 06, 2015 at 05:28:32PM +0100, Christoph Hellwig wrote:

> +#ifdef CONFIG_NFSD_PNFS
> +static __be32
> +nfsd4_encode_getdeviceinfo(struct nfsd4_compoundres *resp, __be32 nfserr,
> +             struct nfsd4_getdeviceinfo *gdev)
> +{
> +     struct xdr_stream *xdr = &resp->xdr;
> +     const struct nfsd4_layout_ops *ops =
> +             nfsd4_layout_ops[gdev->gd_layout_type];
> +     u32 starting_len = xdr->buf->len, needed_len;
> +     __be32 *p;
> +
> +     dprintk("%s: err %d\n", __func__, nfserr);
> +     if (nfserr)
> +             goto out;

In nfsd4_block_get_device_info_simple(), gdp->gd_device might have
been allocated, but sb->s_export_op->get_uuid() might have returned
an error, which would cause a leak here.

> +
> +     p = xdr_reserve_space(xdr, 4);
> +     if (!p)
> +             return nfserr_resource;

gdp->gd_device can be leaked here.

> +     *p++ = cpu_to_be32(gdev->gd_layout_type);
> +
> +     /* If maxcount is 0 then just update notifications */
> +     if (gdev->gd_maxcount != 0) {
> +             nfserr = ops->encode_getdeviceinfo(xdr, gdev);
> +             if (nfserr) {
> +                     /*
> +                      * We don't bother to burden the layout drivers with
> +                      * enforcing gd_maxcount, just tell the client to
> +                      * come back with a bigger buffer if it's not enough.
> +                      */
> +                     if (xdr->buf->len + 4 > gdev->gd_maxcount)
> +                             goto toosmall;
> +                     goto out;
> +             }
> +     }
> +
> +     if (gdev->gd_notify_types) {
> +             p = xdr_reserve_space(xdr, 4 + 4);
> +             if (!p)
> +                     return nfserr_resource;

gdp->gd_device can be leaked here.

> +             *p++ = cpu_to_be32(1);                  /* bitmap length */
> +             *p++ = cpu_to_be32(gdev->gd_notify_types);
> +     } else {
> +             p = xdr_reserve_space(xdr, 4);
> +             if (!p)
> +                     return nfserr_resource;

gdp->gd_device can be leaked here.

> +             *p++ = 0;
> +     }
> +
> +out:
> +     kfree(gdev->gd_device);
> +     dprintk("%s: done: %d\n", __func__, be32_to_cpu(nfserr));
> +     return nfserr;
> +
> +toosmall:
> +     dprintk("%s: maxcount too small\n", __func__);
> +     needed_len = xdr->buf->len + 4 /* notifications */;
> +     xdr_truncate_encode(xdr, starting_len);
> +     p = xdr_reserve_space(xdr, 4);
> +     if (!p)
> +             return nfserr_resource;
> +     *p++ = cpu_to_be32(needed_len);
> +     nfserr = nfserr_toosmall;
> +     goto out;
> +}
> +

<Prev in Thread] Current Thread [Next in Thread>