On Thu, 2012-09-06 at 15:46 +0100, Sachin Prabhu wrote: > On Mon, 2012-09-03 at 19:11 +0000, Myklebust, Trond wrote: > > > I encountered 2 problems. > > > 1) The if condition should be srclen >= pgbase + acl_len > > > 2) There is a second _copy_from_pages which copies to the the acl to the > > > passed buffer in __nfs4_get_acl_uncached(). > > > > The second copy from pages should already be covered by the checks in > > decode_getacl. Alright, since this is not obvious, then clearly we need > > to make it so. How about the following? > > I could reproduce the crash with the second _copy_from_pages using the > patch to PyNFS from > http://www.spinics.net/lists/linux-nfs/msg32359.html > > The patch below works fine for both cases apart from a small bug which > I've pointed to below. > > > 8<------------------------------------------------------------------ > > From 5040240245a046bd58c383806b3f161ee8b5823b Mon Sep 17 00:00:00 2001 > > From: Trond Myklebust <Trond.Myklebust@xxxxxxxxxx> > > Date: Sun, 26 Aug 2012 11:44:43 -0700 > > Subject: [PATCH] NFSv4: Fix buffer overflow checking in > > __nfs4_get_acl_uncached > > + /* Check for receive buffer overflow */ > > + if (attrlen > (xdr->nwords << 2) || > > + attrlen + pg_offset > xdr->buf->page_len) { > > Here we need to use res->acl_data_offset which points to the start of > the ACL data instead of pg_offset which points to the start of the pages > section of the xdr_buf. > > Once I changed this if condition, I was able to successfully test with > my reproducer. Oops... Yes, of course you are right. I'll make the modification and push out the changed patch. Thanks very much for testing! Do you want me to add a "Tested-by:" line? -- Trond Myklebust Linux NFS client maintainer NetApp Trond.Myklebust@xxxxxxxxxx www.netapp.com ��.n��������+%������w��{.n�����{��w��� jg��� �����ݢj����G���� ���j:+v���w�m������w�������h�����٥
