Re: [PATCH] nfs client, kernel 2.4.31: readlink result overflow

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Assar wrote:

Peter Staubach <[email protected]> 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);


One other thing -- it doesn't seem particularly correct to me to just
silently truncate the symbolic link contents.  If the contents can not
be handled correctly because they are too large, then some sort of error
should be generated.  Silently truncating could lead to interoperability
issues when multiple clients handle the contents in different fashions,
some truncating, some returning errors, and some handling the long returns.

   Thanx...

      ps
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [email protected]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[Index of Archives]     [Kernel Newbies]     [Netfilter]     [Bugtraq]     [Photo]     [Gimp]     [Yosemite News]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Video 4 Linux]     [Linux for the blind]
  Powered by Linux