Peter Staubach <staubach@xxxxxxxxxx> writes:
Yes, I think that there is a bug in the boundary checking. I think that:
if (len > rcvbuf->page_len)
should be
if (len >= rcvbuf->page_len - sizeof(u32) || len > 1024)
Thanks for your feedback. The patch to 2.4.31 that incorporates your
suggsted changes is here:
diff -u linux-2.4.31.orig/fs/nfs/nfs2xdr.c linux-2.4.31/fs/nfs/nfs2xdr.c
--- linux-2.4.31.orig/fs/nfs/nfs2xdr.c 2002-11-28 18:53:15.000000000 -0500
+++ linux-2.4.31/fs/nfs/nfs2xdr.c 2005-09-14 15:33:30.000000000 -0400
@@ -571,8 +571,8 @@
strlen = (u32*)kmap(rcvbuf->pages[0]);
/* Convert length of symlink */
len = ntohl(*strlen);
- if (len > rcvbuf->page_len)
- len = rcvbuf->page_len;
+ if (len >= rcvbuf->page_len - sizeof(u32) || len > NFS2_MAXPATHLEN)
+ len = rcvbuf->page_len - sizeof(u32) - 1;
*strlen = len;
/* NULL terminate the string we got */
string = (char *)(strlen + 1);
diff -u linux-2.4.31.orig/fs/nfs/nfs3xdr.c linux-2.4.31/fs/nfs/nfs3xdr.c
--- linux-2.4.31.orig/fs/nfs/nfs3xdr.c 2003-11-28 13:26:21.000000000 -0500
+++ linux-2.4.31/fs/nfs/nfs3xdr.c 2005-09-14 15:33:53.000000000 -0400
@@ -759,8 +759,8 @@
strlen = (u32*)kmap(rcvbuf->pages[0]);
/* Convert length of symlink */
len = ntohl(*strlen);
- if (len > rcvbuf->page_len)
- len = rcvbuf->page_len;
+ if (len >= rcvbuf->page_len - sizeof(u32))
+ len = rcvbuf->page_len - sizeof(u32) - 1;
*strlen = len;
/* NULL terminate the string we got */
string = (char *)(strlen + 1);