| Message ID | 159803139578.514751.6905262413915309673.stgit@manet.1015granger.net (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | NFSD: Fix listxattr receive buffer size | expand |
Oops. Short description should read: NFS: Fix listxattr receive buffer size > On Aug 21, 2020, at 1:36 PM, Chuck Lever <chuck.lever@oracle.com> wrote: > > Certain NFSv4.2/RDMA tests fail with v5.9-rc1. > > rpcrdma_convert_kvec() runs off the end of the rl_segments array > because rq_rcv_buf.tail[0].iov_len holds a very large positive > value. The resultant kernel memory corruption is enough to crash > the client system. > > Callers of rpc_prepare_reply_pages() must reserve an extra XDR_UNIT > in the maximum decode size for a possible XDR pad of the contents > of the xdr_buf's pages. That guarantees the allocated receive buffer > will be large enough to accommodate the usual contents plus that XDR > pad word. > > encode_op_hdr() cannot add that extra word. If it does, > xdr_inline_pages() underruns the length of the tail iovec. > > Fixes: 3e1f02123fba ("NFSv4.2: add client side XDR handling for extended attributes") > Signed-off-by: Chuck Lever <chuck.lever@oracle.com> > --- > fs/nfs/nfs42xdr.c | 4 ++-- > 1 file changed, 2 insertions(+), 2 deletions(-) > > diff --git a/fs/nfs/nfs42xdr.c b/fs/nfs/nfs42xdr.c > index cc50085e151c..d0ddf90c9be4 100644 > --- a/fs/nfs/nfs42xdr.c > +++ b/fs/nfs/nfs42xdr.c > @@ -179,7 +179,7 @@ > 1 + nfs4_xattr_name_maxsz + 1) > #define decode_setxattr_maxsz (op_decode_hdr_maxsz + decode_change_info_maxsz) > #define encode_listxattrs_maxsz (op_encode_hdr_maxsz + 2 + 1) > -#define decode_listxattrs_maxsz (op_decode_hdr_maxsz + 2 + 1 + 1) > +#define decode_listxattrs_maxsz (op_decode_hdr_maxsz + 2 + 1 + 1 + 1) > #define encode_removexattr_maxsz (op_encode_hdr_maxsz + 1 + \ > nfs4_xattr_name_maxsz) > #define decode_removexattr_maxsz (op_decode_hdr_maxsz + \ > @@ -504,7 +504,7 @@ static void encode_listxattrs(struct xdr_stream *xdr, > { > __be32 *p; > > - encode_op_hdr(xdr, OP_LISTXATTRS, decode_listxattrs_maxsz + 1, hdr); > + encode_op_hdr(xdr, OP_LISTXATTRS, decode_listxattrs_maxsz, hdr); > > p = reserve_space(xdr, 12); > if (unlikely(!p)) > > -- Chuck Lever
On Fri, Aug 21, 2020 at 02:59:32PM -0400, Chuck Lever wrote: > Oops. Short description should read: > > NFS: Fix listxattr receive buffer size > > > > On Aug 21, 2020, at 1:36 PM, Chuck Lever <chuck.lever@oracle.com> wrote: > > > > Certain NFSv4.2/RDMA tests fail with v5.9-rc1. > > > > rpcrdma_convert_kvec() runs off the end of the rl_segments array > > because rq_rcv_buf.tail[0].iov_len holds a very large positive > > value. The resultant kernel memory corruption is enough to crash > > the client system. > > > > Callers of rpc_prepare_reply_pages() must reserve an extra XDR_UNIT > > in the maximum decode size for a possible XDR pad of the contents > > of the xdr_buf's pages. That guarantees the allocated receive buffer > > will be large enough to accommodate the usual contents plus that XDR > > pad word. > > > > encode_op_hdr() cannot add that extra word. If it does, > > xdr_inline_pages() underruns the length of the tail iovec. > > > > Fixes: 3e1f02123fba ("NFSv4.2: add client side XDR handling for extended attributes") > > Signed-off-by: Chuck Lever <chuck.lever@oracle.com> > > --- > > fs/nfs/nfs42xdr.c | 4 ++-- > > 1 file changed, 2 insertions(+), 2 deletions(-) > > > > diff --git a/fs/nfs/nfs42xdr.c b/fs/nfs/nfs42xdr.c > > index cc50085e151c..d0ddf90c9be4 100644 > > --- a/fs/nfs/nfs42xdr.c > > +++ b/fs/nfs/nfs42xdr.c > > @@ -179,7 +179,7 @@ > > 1 + nfs4_xattr_name_maxsz + 1) > > #define decode_setxattr_maxsz (op_decode_hdr_maxsz + decode_change_info_maxsz) > > #define encode_listxattrs_maxsz (op_encode_hdr_maxsz + 2 + 1) > > -#define decode_listxattrs_maxsz (op_decode_hdr_maxsz + 2 + 1 + 1) > > +#define decode_listxattrs_maxsz (op_decode_hdr_maxsz + 2 + 1 + 1 + 1) > > #define encode_removexattr_maxsz (op_encode_hdr_maxsz + 1 + \ > > nfs4_xattr_name_maxsz) > > #define decode_removexattr_maxsz (op_decode_hdr_maxsz + \ > > @@ -504,7 +504,7 @@ static void encode_listxattrs(struct xdr_stream *xdr, > > { > > __be32 *p; > > > > - encode_op_hdr(xdr, OP_LISTXATTRS, decode_listxattrs_maxsz + 1, hdr); > > + encode_op_hdr(xdr, OP_LISTXATTRS, decode_listxattrs_maxsz, hdr); > > > > p = reserve_space(xdr, 12); > > if (unlikely(!p)) > > > > > > -- > Chuck Lever A detail of the sunrpc/xdr code I wasn't familiar with.. thanks for fixing this. - Frank
diff --git a/fs/nfs/nfs42xdr.c b/fs/nfs/nfs42xdr.c index cc50085e151c..d0ddf90c9be4 100644 --- a/fs/nfs/nfs42xdr.c +++ b/fs/nfs/nfs42xdr.c @@ -179,7 +179,7 @@ 1 + nfs4_xattr_name_maxsz + 1) #define decode_setxattr_maxsz (op_decode_hdr_maxsz + decode_change_info_maxsz) #define encode_listxattrs_maxsz (op_encode_hdr_maxsz + 2 + 1) -#define decode_listxattrs_maxsz (op_decode_hdr_maxsz + 2 + 1 + 1) +#define decode_listxattrs_maxsz (op_decode_hdr_maxsz + 2 + 1 + 1 + 1) #define encode_removexattr_maxsz (op_encode_hdr_maxsz + 1 + \ nfs4_xattr_name_maxsz) #define decode_removexattr_maxsz (op_decode_hdr_maxsz + \ @@ -504,7 +504,7 @@ static void encode_listxattrs(struct xdr_stream *xdr, { __be32 *p; - encode_op_hdr(xdr, OP_LISTXATTRS, decode_listxattrs_maxsz + 1, hdr); + encode_op_hdr(xdr, OP_LISTXATTRS, decode_listxattrs_maxsz, hdr); p = reserve_space(xdr, 12); if (unlikely(!p))
Certain NFSv4.2/RDMA tests fail with v5.9-rc1. rpcrdma_convert_kvec() runs off the end of the rl_segments array because rq_rcv_buf.tail[0].iov_len holds a very large positive value. The resultant kernel memory corruption is enough to crash the client system. Callers of rpc_prepare_reply_pages() must reserve an extra XDR_UNIT in the maximum decode size for a possible XDR pad of the contents of the xdr_buf's pages. That guarantees the allocated receive buffer will be large enough to accommodate the usual contents plus that XDR pad word. encode_op_hdr() cannot add that extra word. If it does, xdr_inline_pages() underruns the length of the tail iovec. Fixes: 3e1f02123fba ("NFSv4.2: add client side XDR handling for extended attributes") Signed-off-by: Chuck Lever <chuck.lever@oracle.com> --- fs/nfs/nfs42xdr.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)