| Message ID | 20240125144654.12794-1-mora@netapp.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | NFSD: fix nfsd4_listxattr_validate_cookie | expand |
On Thu, 2024-01-25 at 07:46 -0700, Jorge Mora wrote: > If LISTXATTRS is sent with a correct cookie but a small maxcount, > this could lead function nfsd4_listxattr_validate_cookie to > return NFS4ERR_BAD_COOKIE. If maxcount = 20, then second check > on function gives RHS = 3 thus any cookie larger than 3 returns > NFS4ERR_BAD_COOKIE. > > There is no need to validate the cookie on the return XDR buffer > since attribute referenced by cookie will be the first in the > return buffer. > > Fixes: 23e50fe3a5e6 ("nfsd: implement the xattr functions and en/decode logic") > Signed-off-by: Jorge Mora <mora@netapp.com> > --- > fs/nfsd/nfs4xdr.c | 7 +------ > 1 file changed, 1 insertion(+), 6 deletions(-) > > diff --git a/fs/nfsd/nfs4xdr.c b/fs/nfsd/nfs4xdr.c > index 913d566519bf..7f957061c4d5 100644 > --- a/fs/nfsd/nfs4xdr.c > +++ b/fs/nfsd/nfs4xdr.c > @@ -5116,16 +5116,11 @@ nfsd4_listxattr_validate_cookie(struct nfsd4_listxattrs *listxattrs, > > /* > * If the cookie is larger than the maximum number we can fit > - * in either the buffer we just got back from vfs_listxattr, or, > - * XDR-encoded, in the return buffer, it's invalid. > + * in the buffer we just got back from vfs_listxattr, it's invalid. > */ > if (cookie > (listxattrs->lsxa_len) / (XATTR_USER_PREFIX_LEN + 2)) > return nfserr_badcookie; > > - if (cookie > (listxattrs->lsxa_maxcount / > - (XDR_QUADLEN(XATTR_USER_PREFIX_LEN + 2) + 4))) > - return nfserr_badcookie; > - > *offsetp = (u32)cookie; > return 0; > } The cookie is generated by the server, and the client generates the maxcount value. They don't really have anything to do with one another, so yeah, I agree that this check is nonsensical. Reviewed-by: Jeff Layton <jlayton@kernel.org>
diff --git a/fs/nfsd/nfs4xdr.c b/fs/nfsd/nfs4xdr.c index 913d566519bf..7f957061c4d5 100644 --- a/fs/nfsd/nfs4xdr.c +++ b/fs/nfsd/nfs4xdr.c @@ -5116,16 +5116,11 @@ nfsd4_listxattr_validate_cookie(struct nfsd4_listxattrs *listxattrs, /* * If the cookie is larger than the maximum number we can fit - * in either the buffer we just got back from vfs_listxattr, or, - * XDR-encoded, in the return buffer, it's invalid. + * in the buffer we just got back from vfs_listxattr, it's invalid. */ if (cookie > (listxattrs->lsxa_len) / (XATTR_USER_PREFIX_LEN + 2)) return nfserr_badcookie; - if (cookie > (listxattrs->lsxa_maxcount / - (XDR_QUADLEN(XATTR_USER_PREFIX_LEN + 2) + 4))) - return nfserr_badcookie; - *offsetp = (u32)cookie; return 0; }
If LISTXATTRS is sent with a correct cookie but a small maxcount, this could lead function nfsd4_listxattr_validate_cookie to return NFS4ERR_BAD_COOKIE. If maxcount = 20, then second check on function gives RHS = 3 thus any cookie larger than 3 returns NFS4ERR_BAD_COOKIE. There is no need to validate the cookie on the return XDR buffer since attribute referenced by cookie will be the first in the return buffer. Fixes: 23e50fe3a5e6 ("nfsd: implement the xattr functions and en/decode logic") Signed-off-by: Jorge Mora <mora@netapp.com> --- fs/nfsd/nfs4xdr.c | 7 +------ 1 file changed, 1 insertion(+), 6 deletions(-)