Messages in this thread | | | Date | Wed, 14 Sep 2005 16:15:10 -0400 | From | Peter Staubach <> | Subject | Re: [PATCH] nfs client, kernel 2.4.31: readlink result overflow |
| |
Assar wrote:
>Peter Staubach <staubach@redhat.com> 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 majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
| |