| Message ID | 20170508155741.GC2644@fieldses.org (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
I'm also thinking of patching the server to bypass most of the individual encoders in the error case. There's only a couple that actually do anything there, most of just start with variations on if (nfserr) return nfserr; so I figure it's better to catch that once rather than giving them a chance to mess it up. I also wondered for a moment if there's a module-unload race here.... But the layout types aren't loadable modules on the server side, so OK. --b. On Mon, May 08, 2017 at 11:57:41AM -0400, bfields wrote: > From: "J. Bruce Fields" <bfields@redhat.com> > > In error cases, lgp->lg_layout_type may be out of bounds; so we > shouldn't be using it until after the check of nfserr. > > This was seen to crash nfsd threads when the server receives a LAYOUTGET > request with a large layout type. > > GETDEVICEINFO has the same problem. > > Reported-by: Ari Kauppi <Ari.Kauppi@synopsys.com> > Signed-off-by: J. Bruce Fields <bfields@redhat.com> > --- > fs/nfsd/nfs4xdr.c | 8 ++++---- > 1 file changed, 4 insertions(+), 4 deletions(-) > > diff --git a/fs/nfsd/nfs4xdr.c b/fs/nfsd/nfs4xdr.c > index 33017d652b1d..8e9652f37f1f 100644 > --- a/fs/nfsd/nfs4xdr.c > +++ b/fs/nfsd/nfs4xdr.c > @@ -4119,8 +4119,7 @@ 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]; > + const struct nfsd4_layout_ops *ops; > u32 starting_len = xdr->buf->len, needed_len; > __be32 *p; > > @@ -4137,6 +4136,7 @@ nfsd4_encode_getdeviceinfo(struct nfsd4_compoundres *resp, __be32 nfserr, > > /* If maxcount is 0 then just update notifications */ > if (gdev->gd_maxcount != 0) { > + ops = nfsd4_layout_ops[gdev->gd_layout_type]; > nfserr = ops->encode_getdeviceinfo(xdr, gdev); > if (nfserr) { > /* > @@ -4189,8 +4189,7 @@ nfsd4_encode_layoutget(struct nfsd4_compoundres *resp, __be32 nfserr, > struct nfsd4_layoutget *lgp) > { > struct xdr_stream *xdr = &resp->xdr; > - const struct nfsd4_layout_ops *ops = > - nfsd4_layout_ops[lgp->lg_layout_type]; > + const struct nfsd4_layout_ops *ops; > __be32 *p; > > dprintk("%s: err %d\n", __func__, nfserr); > @@ -4213,6 +4212,7 @@ nfsd4_encode_layoutget(struct nfsd4_compoundres *resp, __be32 nfserr, > *p++ = cpu_to_be32(lgp->lg_seg.iomode); > *p++ = cpu_to_be32(lgp->lg_layout_type); > > + ops = nfsd4_layout_ops[lgp->lg_layout_type]; > nfserr = ops->encode_layoutget(xdr, lgp); > out: > kfree(lgp->lg_content); > -- > 2.9.3 > -- To unsubscribe from this list: send the line "unsubscribe linux-nfs" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Looks fine,
Reviewed-by: Christoph Hellwig <hch@lst.de>
--
To unsubscribe from this list: send the line "unsubscribe linux-nfs" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --git a/fs/nfsd/nfs4xdr.c b/fs/nfsd/nfs4xdr.c index 33017d652b1d..8e9652f37f1f 100644 --- a/fs/nfsd/nfs4xdr.c +++ b/fs/nfsd/nfs4xdr.c @@ -4119,8 +4119,7 @@ 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]; + const struct nfsd4_layout_ops *ops; u32 starting_len = xdr->buf->len, needed_len; __be32 *p; @@ -4137,6 +4136,7 @@ nfsd4_encode_getdeviceinfo(struct nfsd4_compoundres *resp, __be32 nfserr, /* If maxcount is 0 then just update notifications */ if (gdev->gd_maxcount != 0) { + ops = nfsd4_layout_ops[gdev->gd_layout_type]; nfserr = ops->encode_getdeviceinfo(xdr, gdev); if (nfserr) { /* @@ -4189,8 +4189,7 @@ nfsd4_encode_layoutget(struct nfsd4_compoundres *resp, __be32 nfserr, struct nfsd4_layoutget *lgp) { struct xdr_stream *xdr = &resp->xdr; - const struct nfsd4_layout_ops *ops = - nfsd4_layout_ops[lgp->lg_layout_type]; + const struct nfsd4_layout_ops *ops; __be32 *p; dprintk("%s: err %d\n", __func__, nfserr); @@ -4213,6 +4212,7 @@ nfsd4_encode_layoutget(struct nfsd4_compoundres *resp, __be32 nfserr, *p++ = cpu_to_be32(lgp->lg_seg.iomode); *p++ = cpu_to_be32(lgp->lg_layout_type); + ops = nfsd4_layout_ops[lgp->lg_layout_type]; nfserr = ops->encode_layoutget(xdr, lgp); out: kfree(lgp->lg_content);