On Jan 25, 2008, at 6:15 PM, J. Bruce Fields wrote:
The length "nbytes" passed into read_buf should never be negative, but
we check only for too-large values of "nbytes", not for too-small
values. Make nbytes unsigned, so it's clear that the former tests are
sufficient. (Despite this read_buf() currently correctly returns
an xdr
error in the case of a negative length, thanks to an unsigned
comparison with size_of() and bounds-checking in kmalloc(). This
seems
very fragile, though.)
Signed-off-by: J. Bruce Fields <[EMAIL PROTECTED]>
---
fs/nfsd/nfs4xdr.c | 7 ++++++-
1 files changed, 6 insertions(+), 1 deletions(-)
diff --git a/fs/nfsd/nfs4xdr.c b/fs/nfsd/nfs4xdr.c
index 5733394..25c7ae2 100644
--- a/fs/nfsd/nfs4xdr.c
+++ b/fs/nfsd/nfs4xdr.c
@@ -148,7 +148,7 @@ xdr_error: \
} \
} while (0)
-static __be32 *read_buf(struct nfsd4_compoundargs *argp, int nbytes)
+static __be32 *read_buf(struct nfsd4_compoundargs *argp, u32 nbytes)
{
/* We want more bytes than seem to be available.
* Maybe we need a new page, maybe we have just run out
Changing nbytes to an unsigned introduces a mixed-sign comparison:
int avail = (char*)argp->end - (char*)argp->p;
__be32 *p;
if (avail + argp->pagelen < nbytes) <<<<<<
"avail" and "argp->pagelen" are signed ints.
return NULL;
if (avail + PAGE_SIZE < nbytes) /* need more than a page !! */
return NULL;
PAGE_SIZE is unsigned long (defined as "1UL << PAGE_SHIFT"), so
changing nbytes may have additional unintended consequences.
We need careful review of everywhere argp->pagelen is used throughout
this file. But I think just this simple fix is inadequate.
@@ -169,6 +169,11 @@ static __be32 *read_buf(struct
nfsd4_compoundargs *argp, int nbytes)
return NULL;
}
+ /*
+ * The following memcpy is safe because read_buf is always
+ * called with nbytes > avail, and the two cases above both
+ * guarantee p points to at least nbytes bytes.
+ */
memcpy(p, argp->p, avail);
/* step to next page */
argp->p = page_address(argp->pagelist[0]);
--
1.5.4.rc2.60.gb2e62
-
To unsubscribe from this list: send the line "unsubscribe linux-
nfs" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
--
Chuck Lever
chuck[dot]lever[at]oracle[dot]com
-
To unsubscribe from this list: send the line "unsubscribe linux-nfs" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
