Re: NFS & long symlinks = stack overflow

[Trond Myklebust <trond.myklebust@fys.uio.no>]

På su , 16/05/2004 klokka 00:55, skreiv
viro@parcelfarce.linux.theplanet.co.uk:
> v2 has a hard limit in protocol (<= 1Kb).  However, we shouldn't assume that
> server is sane...

True... The other thing is that we need to return ENAMETOOLONG rather
than EIO.
Finally, the NFS readlink() methods all take a buffer length argument.
Use that instead of assuming PAGE_SIZE...

OK... How about the following?

Cheers,
  Trond

 nfs2xdr.c |   11 +++++++----
 nfs3xdr.c |   11 +++++++----
 nfs4xdr.c |   11 ++++++-----
 3 files changed, 20 insertions(+), 13 deletions(-)

diff -u --recursive --new-file --show-c-function linux-2.6.6-01-reconnect/fs/nfs/nfs2xdr.c linux-2.6.6-02-symlink_overflow/fs/nfs/nfs2xdr.c
--- linux-2.6.6-01-reconnect/fs/nfs/nfs2xdr.c	2004-05-16 17:07:24.000000000 -0400
+++ linux-2.6.6-02-symlink_overflow/fs/nfs/nfs2xdr.c	2004-05-16 17:36:46.000000000 -0400
@@ -511,8 +511,8 @@ static int
 nfs_xdr_readlinkargs(struct rpc_rqst *req, u32 *p, struct nfs_readlinkargs *args)
 {
 	struct rpc_auth *auth = req->rq_task->tk_auth;
+	unsigned int count = args->count - 5;
 	unsigned int replen;
-	u32 count = args->count - 4;
 
 	p = xdr_encode_fhandle(p, args->fh);
 	req->rq_slen = xdr_adjust_iovec(req->rq_svec, p);
@@ -547,12 +547,15 @@ nfs_xdr_readlinkres(struct rpc_rqst *req
 	strlen = (u32*)kmap_atomic(rcvbuf->pages[0], KM_USER0);
 	/* Convert length of symlink */
 	len = ntohl(*strlen);
-	if (len > rcvbuf->page_len)
-		len = rcvbuf->page_len;
+	if (len > rcvbuf->page_len) {
+		dprintk(KERN_WARNING "nfs: server returned giant symlink!\n");
+		kunmap_atomic(strlen, KM_USER0);
+		return -ENAMETOOLONG;
+	}
 	*strlen = len;
 	/* NULL terminate the string we got */
 	string = (char *)(strlen + 1);
-	string[len] = 0;
+	string[len] = '\0';
 	kunmap_atomic(strlen, KM_USER0);
 	return 0;
 }
diff -u --recursive --new-file --show-c-function linux-2.6.6-01-reconnect/fs/nfs/nfs3xdr.c linux-2.6.6-02-symlink_overflow/fs/nfs/nfs3xdr.c
--- linux-2.6.6-01-reconnect/fs/nfs/nfs3xdr.c	2004-05-16 17:08:10.000000000 -0400
+++ linux-2.6.6-02-symlink_overflow/fs/nfs/nfs3xdr.c	2004-05-16 17:36:39.000000000 -0400
@@ -702,8 +702,8 @@ static int
 nfs3_xdr_readlinkargs(struct rpc_rqst *req, u32 *p, struct nfs3_readlinkargs *args)
 {
 	struct rpc_auth *auth = req->rq_task->tk_auth;
+	unsigned int count = args->count - 5;
 	unsigned int replen;
-	u32 count = args->count - 4;
 
 	p = xdr_encode_fhandle(p, args->fh);
 	req->rq_slen = xdr_adjust_iovec(req->rq_svec, p);
@@ -742,12 +742,15 @@ nfs3_xdr_readlinkres(struct rpc_rqst *re
 	strlen = (u32*)kmap_atomic(rcvbuf->pages[0], KM_USER0);
 	/* Convert length of symlink */
 	len = ntohl(*strlen);
-	if (len > rcvbuf->page_len)
-		len = rcvbuf->page_len;
+	if (len > rcvbuf->page_len) {
+		dprintk(KERN_WARNING "nfs: server returned giant symlink!\n");
+		kunmap_atomic(strlen, KM_USER0);
+		return -ENAMETOOLONG;
+	}
 	*strlen = len;
 	/* NULL terminate the string we got */
 	string = (char *)(strlen + 1);
-	string[len] = 0;
+	string[len] = '\0';
 	kunmap_atomic(strlen, KM_USER0);
 	return 0;
 }
diff -u --recursive --new-file --show-c-function linux-2.6.6-01-reconnect/fs/nfs/nfs4xdr.c linux-2.6.6-02-symlink_overflow/fs/nfs/nfs4xdr.c
--- linux-2.6.6-01-reconnect/fs/nfs/nfs4xdr.c	2004-05-16 17:08:07.000000000 -0400
+++ linux-2.6.6-02-symlink_overflow/fs/nfs/nfs4xdr.c	2004-05-16 17:36:29.000000000 -0400
@@ -947,7 +947,8 @@ static int encode_readdir(struct xdr_str
 static int encode_readlink(struct xdr_stream *xdr, const struct nfs4_readlink *readlink, struct rpc_rqst *req)
 {
 	struct rpc_auth *auth = req->rq_task->tk_auth;
-	int replen;
+	unsigned int count = readlink->count - 5;
+	unsigned int replen;
 	uint32_t *p;
 
 	RESERVE_SPACE(4);
@@ -958,7 +959,7 @@ static int encode_readlink(struct xdr_st
 	 *      + OP_READLINK + status  = 7
 	 */
 	replen = (RPC_REPHDRSIZE + auth->au_rslack + 7) << 2;
-	xdr_inline_pages(&req->rq_rcv_buf, replen, readlink->pages, 0, readlink->count);
+	xdr_inline_pages(&req->rq_rcv_buf, replen, readlink->pages, 0, count);
 	
 	return 0;
 }
@@ -2921,10 +2922,10 @@ static int decode_readlink(struct xdr_st
 	 */
 	strlen = (uint32_t *) kmap_atomic(rcvbuf->pages[0], KM_USER0);
 	len = ntohl(*strlen);
-	if (len > PAGE_CACHE_SIZE - 5) {
-		printk(KERN_WARNING "nfs: server returned giant symlink!\n");
+	if (len > rcvbuf->page_len) {
+		dprintk(KERN_WARNING "nfs: server returned giant symlink!\n");
 		kunmap_atomic(strlen, KM_USER0);
-		return -EIO;
+		return -ENAMETOOLONG;
 	}
 	*strlen = len;