diff options
| author | NeilBrown <neilb@suse.com> | 2019-03-03 22:08:22 -0500 |
|---|---|---|
| committer | J. Bruce Fields <bfields@redhat.com> | 2019-03-05 16:41:33 -0500 |
| commit | b602345da6cbb135ba68cf042df8ec9a73da7981 (patch) | |
| tree | f557312934ce99b6fb59de6852c4f1dc221d9aba | |
| parent | c54f24e338ed2a35218f117a4a1afb5f9e2b4e64 (diff) | |
nfsd: fix memory corruption caused by readdir
If the result of an NFSv3 readdir{,plus} request results in the
"offset" on one entry having to be split across 2 pages, and is sized
so that the next directory entry doesn't fit in the requested size,
then memory corruption can happen.
When encode_entry() is called after encoding the last entry that fits,
it notices that ->offset and ->offset1 are set, and so stores the
offset value in the two pages as required. It clears ->offset1 but
*does not* clear ->offset.
Normally this omission doesn't matter as encode_entry_baggage() will
be called, and will set ->offset to a suitable value (not on a page
boundary).
But in the case where cd->buflen < elen and nfserr_toosmall is
returned, ->offset is not reset.
This means that nfsd3proc_readdirplus will see ->offset with a value 4
bytes before the end of a page, and ->offset1 set to NULL.
It will try to write 8bytes to ->offset.
If we are lucky, the next page will be read-only, and the system will
BUG: unable to handle kernel paging request at...
If we are unlucky, some innocent page will have the first 4 bytes
corrupted.
nfsd3proc_readdir() doesn't even check for ->offset1, it just blindly
writes 8 bytes to the offset wherever it is.
Fix this by clearing ->offset after it is used, and copying the
->offset handling code from nfsd3_proc_readdirplus into
nfsd3_proc_readdir.
(Note that the commit hash in the Fixes tag is from the 'history'
tree - this bug predates git).
Fixes: 0b1d57cf7654 ("[PATCH] kNFSd: Fix nfs3 dentry encoding")
Fixes-URL: https://git.kernel.org/pub/scm/linux/kernel/git/history/history.git/commit/?id=0b1d57cf7654
Cc: stable@vger.kernel.org (v2.6.12+)
Signed-off-by: NeilBrown <neilb@suse.com>
Signed-off-by: J. Bruce Fields <bfields@redhat.com>
| -rw-r--r-- | fs/nfsd/nfs3proc.c | 16 | ||||
| -rw-r--r-- | fs/nfsd/nfs3xdr.c | 1 |
2 files changed, 15 insertions, 2 deletions
diff --git a/fs/nfsd/nfs3proc.c b/fs/nfsd/nfs3proc.c index 9eb8086ea841..c9cf46e0c040 100644 --- a/fs/nfsd/nfs3proc.c +++ b/fs/nfsd/nfs3proc.c | |||
| @@ -463,8 +463,19 @@ nfsd3_proc_readdir(struct svc_rqst *rqstp) | |||
| 463 | &resp->common, nfs3svc_encode_entry); | 463 | &resp->common, nfs3svc_encode_entry); |
| 464 | memcpy(resp->verf, argp->verf, 8); | 464 | memcpy(resp->verf, argp->verf, 8); |
| 465 | resp->count = resp->buffer - argp->buffer; | 465 | resp->count = resp->buffer - argp->buffer; |
| 466 | if (resp->offset) | 466 | if (resp->offset) { |
| 467 | xdr_encode_hyper(resp->offset, argp->cookie); | 467 | loff_t offset = argp->cookie; |
| 468 | |||
| 469 | if (unlikely(resp->offset1)) { | ||
| 470 | /* we ended up with offset on a page boundary */ | ||
| 471 | *resp->offset = htonl(offset >> 32); | ||
| 472 | *resp->offset1 = htonl(offset & 0xffffffff); | ||
| 473 | resp->offset1 = NULL; | ||
| 474 | } else { | ||
| 475 | xdr_encode_hyper(resp->offset, offset); | ||
| 476 | } | ||
| 477 | resp->offset = NULL; | ||
| 478 | } | ||
| 468 | 479 | ||
| 469 | RETURN_STATUS(nfserr); | 480 | RETURN_STATUS(nfserr); |
| 470 | } | 481 | } |
| @@ -533,6 +544,7 @@ nfsd3_proc_readdirplus(struct svc_rqst *rqstp) | |||
| 533 | } else { | 544 | } else { |
| 534 | xdr_encode_hyper(resp->offset, offset); | 545 | xdr_encode_hyper(resp->offset, offset); |
| 535 | } | 546 | } |
| 547 | resp->offset = NULL; | ||
| 536 | } | 548 | } |
| 537 | 549 | ||
| 538 | RETURN_STATUS(nfserr); | 550 | RETURN_STATUS(nfserr); |
diff --git a/fs/nfsd/nfs3xdr.c b/fs/nfsd/nfs3xdr.c index 9b973f4f7d01..83919116d5cb 100644 --- a/fs/nfsd/nfs3xdr.c +++ b/fs/nfsd/nfs3xdr.c | |||
| @@ -921,6 +921,7 @@ encode_entry(struct readdir_cd *ccd, const char *name, int namlen, | |||
| 921 | } else { | 921 | } else { |
| 922 | xdr_encode_hyper(cd->offset, offset64); | 922 | xdr_encode_hyper(cd->offset, offset64); |
| 923 | } | 923 | } |
| 924 | cd->offset = NULL; | ||
| 924 | } | 925 | } |
| 925 | 926 | ||
| 926 | /* | 927 | /* |
