nfsd: fix memory corruption caused by readdir
authorNeilBrown <neilb@suse.com>
Mon, 4 Mar 2019 03:08:22 +0000 (14:08 +1100)
committerGreg Kroah-Hartman <gregkh@linuxfoundation.org>
Sat, 23 Mar 2019 13:35:28 +0000 (14:35 +0100)
commit b602345da6cbb135ba68cf042df8ec9a73da7981 upstream.

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>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
fs/nfsd/nfs3proc.c
fs/nfsd/nfs3xdr.c

index 1d0ce3c57d939d6de10418fe12cf4cd217d392e8..c0de4d6cd8576c2f96a53eae96480467b2c58bd5 100644 (file)
@@ -446,8 +446,19 @@ nfsd3_proc_readdir(struct svc_rqst *rqstp)
                                        &resp->common, nfs3svc_encode_entry);
        memcpy(resp->verf, argp->verf, 8);
        resp->count = resp->buffer - argp->buffer;
-       if (resp->offset)
-               xdr_encode_hyper(resp->offset, argp->cookie);
+       if (resp->offset) {
+               loff_t offset = argp->cookie;
+
+               if (unlikely(resp->offset1)) {
+                       /* we ended up with offset on a page boundary */
+                       *resp->offset = htonl(offset >> 32);
+                       *resp->offset1 = htonl(offset & 0xffffffff);
+                       resp->offset1 = NULL;
+               } else {
+                       xdr_encode_hyper(resp->offset, offset);
+               }
+               resp->offset = NULL;
+       }
 
        RETURN_STATUS(nfserr);
 }
@@ -516,6 +527,7 @@ nfsd3_proc_readdirplus(struct svc_rqst *rqstp)
                } else {
                        xdr_encode_hyper(resp->offset, offset);
                }
+               resp->offset = NULL;
        }
 
        RETURN_STATUS(nfserr);
index f38acd9054419606e3abd25060599960d38c6f2c..ef3e7878456ce3260624752120238191fc4e959d 100644 (file)
@@ -922,6 +922,7 @@ encode_entry(struct readdir_cd *ccd, const char *name, int namlen,
                } else {
                        xdr_encode_hyper(cd->offset, offset64);
                }
+               cd->offset = NULL;
        }
 
        /*