A mistake in calculating the space left in the header in nfs3_xdr_setaclargs() can cause memory corruption when setting a large number of acls. Reproducer: On Server: 1) Create directory /test and set mode 777. mkdir /test; chmod 777 /test 2) Add 200 users and set default acl for user on /test for i in {1..200}; do echo $i; useradd user$i; setfacl -m d:u:user$i:rwx /test;done 3) Add export /test in /etc/exports /test *(rw) On client 1) Mount server:/test mount server:/test /mnt 2) Create large number of directories on the the share. cd/mnt; for i in {1..1000}; do mkdir $i; done At this point, the client should crash. A change in call_header changes the value req->rq_snd_buf->head[0]->iov_len to reflect the exact size of the header. [PATCH] RPC: Ensure XDR iovec length is initialized correctly in call_header 334ccfd545bba9690515f2c5c167d5adb161989b The iov_len is set to the size of the header in call_header(). req->rq_slen = xdr_adjust_iovec(&req->rq_svec[0], p); nfs3_xdr_setaclargs() depends on the older behavior and uses this value when calculating the number of ACLs it can fit into the header. /* put as much of the acls into head as possible. */ len_in_head = min_t(unsigned int, buf->head->iov_len - base, len); len -= len_in_head; req->rq_slen = xdr_adjust_iovec(req->rq_svec, p + (len_in_head >> 2)); Since at this stage, iov_len < base, len_in_head will always be set to len. For a large number of ACLs, this will end up over-writing other parts of memory on the nfs client. The following patch which set len_in_head to 0 was tested with the reproducer and was found to fix the problem. --- fs/nfs/nfs3xdr.c.orig 2009-01-20 15:18:12.000000000 +0000 +++ fs/nfs/nfs3xdr.c 2009-01-20 15:33:45.000000000 +0000 @@ -691,7 +691,10 @@ nfs3_xdr_setaclargs(struct rpc_rqst *req *p++ = htonl(args->mask); base = (char *)p - (char *)buf->head->iov_base; /* put as much of the acls into head as possible. */ - len_in_head = min_t(unsigned int, buf->head->iov_len - base, len); + if ( buf->head->iov_len > base ) + len_in_head = min_t(unsigned int, buf->head->iov_len - base, len); + else + len_in_head = 0; len -= len_in_head; req->rq_slen = xdr_adjust_iovec(req->rq_svec, p + (len_in_head >> 2)); Thanks to Kevin Rudd who did the major legwork here to figure the problem and create the patch. Sachin Prabhu
Attachment:
signature.asc
Description: OpenPGP digital signature