NFS & long symlinks = stack overflow

NFS & long symlinks = stack overflow

[Oleg Drokin]

Hello!

   For some time already I am investigating problems where fsstress run on
   NFS where NFSD is run on ext3 crashes in something like a hour.
   (but does not crash on reiserfs). On x86 crash looks like stack overflow
   (deref of pointer with last 3 digits being zero in kmap directly after
   get_current() call on 2.4.2x).

   Finally I was able to reduce a test case to two simple operations
   that reproduce the problem 100% reliably:

[root@ranma root]# mount ranma:/testing /mnt -t nfs
[root@ranma root]# cd /mnt
[root@ranma mnt]# perl -e 'symlink("a"x4095, "f")'; ls -la f
Segmentation fault

   (btw if you try to pass in something like 4090 worth of symbols, then
   subsequent ls won't crash, but last few symbols of link content will be
   corrupted)

   The crash happens on both 2.4 and 2.6. Below are backtraces for both
   (pretty much identical), but I think it is clear that stack was overflowed
   in some other place then the crash itself.
   Also fsstress crashes over time if run in UML (and again if you run
   nfsd from reiserfs, it does not crash, so I think this confirms the
   problem is related, as reiserfs imposes more strict limit on symlink length),
   but the crash is usually because some slab structures are corrupted and
   I do not yet have two commands that will reproduce the problem in uml
   instantly.

   2.6.6-bk1 trace:
Unable to handle kernel paging request at virtual address fffd1000
 printing eip:
f8ac41c0
*pde = 003eb067
*pte = 00000000
Oops: 0002 [#1]
SMP DEBUG_PAGEALLOC
CPU:    1
EIP:    0060:[<f8ac41c0>]    Not tainted
EFLAGS: 00010202   (2.6.6)
EIP is at nfs3_xdr_readlinkres+0x66/0xb1 [nfs]
eax: fffd0000   ebx: 00000074   ecx: 00000ffc   edx: 00000ffc
esi: f56630fc   edi: f56630bc   ebp: f5067c88   esp: f5067c74
ds: 007b   es: 007b   ss: 0068
Process fsstress (pid: 2016, threadinfo=f5066000 task=f5a3fa60)
Stack: 00000000 f5067e54 f8a404d4 f5067d40 f56630bc f5067cb0 f8a25b7b c1816060
       f4fc8a60 00000282 c1816d20 f8ac415a f5067d40 f5663120 f5663160 f5067cdc
       f8a1f1d0 d91e9474 f5067e54 00000002 c036a780 f8ac415a f56630bc f5067d40
Call Trace:
 [<f8a25b7b>] rpcauth_unwrap_resp+0x5b/0x87 [sunrpc]
 [<f8ac415a>] nfs3_xdr_readlinkres+0x0/0xb1 [nfs]
 [<f8a1f1d0>] call_decode+0x117/0x22e [sunrpc]
 [<f8ac415a>] nfs3_xdr_readlinkres+0x0/0xb1 [nfs]
 [<f8a237f0>] __rpc_execute+0x386/0x3fd [sunrpc]
 [<c0118758>] default_wake_function+0x0/0xc
 [<f8a1e71a>] rpc_call_sync+0x62/0xa1 [sunrpc]
 [<f8a22648>] rpc_run_timer+0x0/0xcf [sunrpc]
 [<f8ac12c9>] nfs3_rpc_wrapper+0x2d/0x6e [nfs]
 [<f8ac18cd>] nfs3_proc_readlink+0xa9/0xed [nfs]
 [<c0136877>] add_to_page_cache+0x5a/0x132
 [<f8abe066>] nfs_symlink_filler+0x62/0x101 [nfs]
 [<c0136983>] add_to_page_cache_lru+0x34/0x36
 [<c0138296>] read_cache_page+0x60/0x20e
 [<f8abe004>] nfs_symlink_filler+0x0/0x101 [nfs]
 [<f8abe18b>] nfs_getlink+0x86/0xe8 [nfs]
 [<f8abe214>] nfs_readlink+0x27/0x81 [nfs]
 [<c01662fe>] __user_walk+0x4b/0x4d
 [<c016130d>] sys_readlink+0x79/0x7d
 [<c0169eec>] filldir64+0x0/0xe3
 [<c015683b>] sys_close+0x7d/0xea
 [<c0103f35>] sysenter_past_esp+0x52/0x71

2.4.27-pre2 trace:
Unable to handle kernel paging request at virtual address ff8e8000
c01a2c64
*pde = 00005063
Oops: 0002
CPU:    0
EIP:    0010:[<c01a2c64>]    Not tainted
Using defaults from ksymoops -t elf32-i386 -a i386
EFLAGS: 00010206
eax: 00000ffc   ebx: 00000074   ecx: ff8e7000   edx: 00000ffc
esi: f5b220ac   edi: 00000000   ebp: f73f5d08   esp: f73f5cf4
ds: 0018   es: 0018   ss: 0018
Process ls (pid: 1094, stackpage=f73f5000)
Stack: f76a627c f73f5e84 f5b22074 f5b220cc f5b22108 f73f5d30 c02a3025 f5b22074 
       f76a6274 f73f5e84 c01a2bd0 f5b22074 f73f4000 f73f5d8c ffffe000 f73f5d6c 
       c02a7ab5 f73f5d8c f73f5d8c f73f5d5c f73f5df4 00000000 f73f4000 00000000 
Call Trace:    [<c02a3025>] [<c01a2bd0>] [<c02a7ab5>] [<c02a2413>] [<c02a6ac0>]
  [<c019fd86>] [<c01a030e>] [<c019c66c>] [<c0135937>] [<c019c719>] [<c019c600>]
  [<c019c7b5>] [<c015226c>] [<c01076fb>]
Code: c6 44 08 04 00 8b 46 10 8b 08 b8 00 e0 ff ff 21 e0 8b 50 30 


>>EIP; c01a2c64 <nfs3_xdr_readlinkres+94/140>   <=====

>>eax; 00000ffc Before first symbol
>>ecx; ff8e7000 <END_OF_CODE+6ef5921/????>
>>edx; 00000ffc Before first symbol
>>esi; f5b220ac <_end+3574d200/384e51b4>
>>ebp; f73f5d08 <_end+37020e5c/384e51b4>
>>esp; f73f5cf4 <_end+37020e48/384e51b4>

Trace; c02a3025 <call_decode+125/240>
Trace; c01a2bd0 <nfs3_xdr_readlinkres+0/140>
Trace; c02a7ab5 <__rpc_execute+f5/3c0>
Trace; c02a2413 <rpc_call_sync+73/c0>
Trace; c02a6ac0 <rpc_run_timer+0/f0>
Trace; c019fd86 <nfs3_rpc_wrapper+46/90>
Trace; c01a030e <nfs3_proc_readlink+8e/f0>
Trace; c019c66c <nfs_symlink_filler+6c/f0>
Trace; c0135937 <read_cache_page+97/160>
Trace; c019c719 <nfs_getlink+29/a0>
Trace; c019c600 <nfs_symlink_filler+0/f0>
Trace; c019c7b5 <nfs_readlink+25/a0>
Trace; c015226c <sys_readlink+9c/b0>
Trace; c01076fb <system_call+33/38>

Code;  c01a2c64 <nfs3_xdr_readlinkres+94/140>
00000000 <_EIP>:
Code;  c01a2c64 <nfs3_xdr_readlinkres+94/140>   <=====
   0:   c6 44 08 04 00            movb   $0x0,0x4(%eax,%ecx,1)   <=====
Code;  c01a2c69 <nfs3_xdr_readlinkres+99/140>
   5:   8b 46 10                  mov    0x10(%esi),%eax
Code;  c01a2c6c <nfs3_xdr_readlinkres+9c/140>
   8:   8b 08                     mov    (%eax),%ecx
Code;  c01a2c6e <nfs3_xdr_readlinkres+9e/140>
   a:   b8 00 e0 ff ff            mov    $0xffffe000,%eax
Code;  c01a2c73 <nfs3_xdr_readlinkres+a3/140>
   f:   21 e0                     and    %esp,%eax
Code;  c01a2c75 <nfs3_xdr_readlinkres+a5/140>
  11:   8b 50 30                  mov    0x30(%eax),%edx


Bye,
    Oleg

Re: NFS & long symlinks = stack overflow

[viro]

On Sat, May 15, 2004 at 04:21:49PM +0300, Oleg Drokin wrote:
> Hello!
> 
>    For some time already I am investigating problems where fsstress run on
>    NFS where NFSD is run on ext3 crashes in something like a hour.
>    (but does not crash on reiserfs). On x86 crash looks like stack overflow
>    (deref of pointer with last 3 digits being zero in kmap directly after
>    get_current() call on 2.4.2x).
> 
>    Finally I was able to reduce a test case to two simple operations
>    that reproduce the problem 100% reliably:
> 
> [root@ranma root]# mount ranma:/testing /mnt -t nfs
> [root@ranma root]# cd /mnt
> [root@ranma mnt]# perl -e 'symlink("a"x4095, "f")'; ls -la f
> Segmentation fault
> 
>    (btw if you try to pass in something like 4090 worth of symbols, then
>    subsequent ls won't crash, but last few symbols of link content will be
>    corrupted)

Lovely.  The real limit imposed by client (apparently not enforced, though)
is PAGE_CACHE_SIZE - 4 - 1.  What are the protocol limits?  I've been looking
into the same area for other reasons just now and all I could find was NFS v2
"no more than 1024 bytes", no information on v3 or v4.

Trond?

Re: NFS & long symlinks = stack overflow

[Trond Myklebust]

On Sat, 2004-05-15 at 09:53, viro@parcelfarce.linux.theplanet.co.uk
wrote:
> Lovely.  The real limit imposed by client (apparently not enforced, though)
> is PAGE_CACHE_SIZE - 4 - 1.  What are the protocol limits?  I've been looking
> into the same area for other reasons just now and all I could find was NFS v2
> "no more than 1024 bytes", no information on v3 or v4.

NFSv3 and v4 have no set limits on the link length other than that
enforced by the server.

Cheers,
  Trond

Re: NFS & long symlinks = stack overflow

[Trond Myklebust]

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;
 

Re: NFS & long symlinks = stack overflow

[viro]

On Sat, May 15, 2004 at 09:41:12PM -0700, Linus Torvalds wrote:
> 
> 
> On Sat, 15 May 2004, Trond Myklebust wrote:
> > 
> > Yes. The following patch (backported from the NFSv4 code) should do the
> > right thing...
> 
> Why isn't this needed for nfsv2, which has similar code? 

v2 has a hard limit in protocol (<= 1Kb).  However, we shouldn't assume that
server is sane...

Re: NFS & long symlinks = stack overflow

[Linus Torvalds]

On Sat, 15 May 2004, Trond Myklebust wrote:
> 
> Yes. The following patch (backported from the NFSv4 code) should do the
> right thing...

Why isn't this needed for nfsv2, which has similar code? 

		Linus

Re: NFS & long symlinks = stack overflow

[Oleg Drokin]

Trond Myklebust <trond.myklebust@fys.uio.no> wrote:
TM> On Sat, 2004-05-15 at 10:53, viro@parcelfarce.linux.theplanet.co.uk
TM> wrote:

>> Lovely...  How are other clients dealing with that?  Put a reasonable
>> limit on the size and return an error if READLINK brings more than that?
TM> Yes. The following patch (backported from the NFSv4 code) should do the
TM> right thing...

Yeah. That helps.
Thank you.

Bye,
    Oleg

Re: NFS & long symlinks = stack overflow

[Trond Myklebust]

On Sat, 2004-05-15 at 10:53, viro@parcelfarce.linux.theplanet.co.uk
wrote:

> Lovely...  How are other clients dealing with that?  Put a reasonable
> limit on the size and return an error if READLINK brings more than that?

Yes. The following patch (backported from the NFSv4 code) should do the
right thing...

Cheers,
  Trond

--- linux-2.6.6-rc3/fs/nfs/nfs3xdr.c.orig	2004-05-09 01:31:17.000000000 -0400
+++ linux-2.6.6-rc3/fs/nfs/nfs3xdr.c	2004-05-15 13:35:06.000000000 -0400
@@ -742,8 +742,11 @@ 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 > PAGE_CACHE_SIZE - 5) {
+		printk(KERN_WARNING "nfs: server returned giant symlink!\n");
+		kunmap_atomic(strlen, KM_USER0);
+		return -EIO;
+	}
 	*strlen = len;
 	/* NULL terminate the string we got */
 	string = (char *)(strlen + 1);

Re: NFS & long symlinks = stack overflow

[viro]

On Sat, May 15, 2004 at 04:30:28PM +0200, Pascal Schmidt wrote:
> On Sat, 15 May 2004 16:00:21 +0200, you wrote in linux.kernel:
> 
> > Lovely.  The real limit imposed by client (apparently not enforced, though)
> > is PAGE_CACHE_SIZE - 4 - 1.  What are the protocol limits?
> 
> None. An NFSv3 server can enforce an arbitrary limit on filename length,
> advertised via PATHCONF, though.

Lovely...  How are other clients dealing with that?  Put a reasonable
limit on the size and return an error if READLINK brings more than that?

Re: NFS & long symlinks = stack overflow

[Pascal Schmidt]

On Sat, 15 May 2004 16:00:21 +0200, you wrote in linux.kernel:

> Lovely.  The real limit imposed by client (apparently not enforced, though)
> is PAGE_CACHE_SIZE - 4 - 1.  What are the protocol limits?

None. An NFSv3 server can enforce an arbitrary limit on filename length,
advertised via PATHCONF, though.

-- 
Ciao,
Pascal