From owner-freebsd-fs@FreeBSD.ORG Fri Jan 25 11:36:15 2013 Return-Path: Delivered-To: freebsd-fs@freebsd.org Received: from mx1.freebsd.org (mx1.FreeBSD.org [8.8.178.115]) by hub.freebsd.org (Postfix) with ESMTP id 07347AF1; Fri, 25 Jan 2013 11:36:15 +0000 (UTC) (envelope-from kostikbel@gmail.com) Received: from kib.kiev.ua (kib.kiev.ua [IPv6:2001:470:d5e7:1::1]) by mx1.freebsd.org (Postfix) with ESMTP id 3A7B72B3; Fri, 25 Jan 2013 11:36:14 +0000 (UTC) Received: from tom.home (kostik@localhost [127.0.0.1]) by kib.kiev.ua (8.14.6/8.14.6) with ESMTP id r0PBa6xe084024; Fri, 25 Jan 2013 13:36:06 +0200 (EET) (envelope-from kostikbel@gmail.com) DKIM-Filter: OpenDKIM Filter v2.7.4 kib.kiev.ua r0PBa6xe084024 Received: (from kostik@localhost) by tom.home (8.14.6/8.14.6/Submit) id r0PBa6Tv084023; Fri, 25 Jan 2013 13:36:06 +0200 (EET) (envelope-from kostikbel@gmail.com) X-Authentication-Warning: tom.home: kostik set sender to kostikbel@gmail.com using -f Date: Fri, 25 Jan 2013 13:36:06 +0200 From: Konstantin Belousov To: Rick Macklem Subject: Re: 9.1-stable crashes while copying data from a NFS mounted directory Message-ID: <20130125113606.GO2522@kib.kiev.ua> References: <201301241721.51102.jhb@freebsd.org> <1390810985.2342318.1359081578591.JavaMail.root@erie.cs.uoguelph.ca> MIME-Version: 1.0 Content-Type: multipart/signed; micalg=pgp-sha1; protocol="application/pgp-signature"; boundary="E0e4ihfNxLmjeTLW" Content-Disposition: inline In-Reply-To: <1390810985.2342318.1359081578591.JavaMail.root@erie.cs.uoguelph.ca> User-Agent: Mutt/1.5.21 (2010-09-15) X-Spam-Status: No, score=-2.0 required=5.0 tests=ALL_TRUSTED,BAYES_00, DKIM_ADSP_CUSTOM_MED,FREEMAIL_FROM,NML_ADSP_CUSTOM_MED autolearn=no version=3.3.2 X-Spam-Checker-Version: SpamAssassin 3.3.2 (2011-06-06) on tom.home Cc: freebsd-fs@freebsd.org, yongari@freebsd.org, Christian Gusenbauer X-BeenThere: freebsd-fs@freebsd.org X-Mailman-Version: 2.1.14 Precedence: list List-Id: Filesystems List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Fri, 25 Jan 2013 11:36:15 -0000 --E0e4ihfNxLmjeTLW Content-Type: text/plain; charset=us-ascii Content-Disposition: inline Content-Transfer-Encoding: quoted-printable On Thu, Jan 24, 2013 at 09:39:38PM -0500, Rick Macklem wrote: > John Baldwin wrote: > > On Thursday, January 24, 2013 4:22:12 pm Konstantin Belousov wrote: > > > On Thu, Jan 24, 2013 at 09:50:52PM +0100, Christian Gusenbauer > > > wrote: > > > > On Thursday 24 January 2013 20:37:09 Konstantin Belousov wrote: > > > > > On Thu, Jan 24, 2013 at 07:50:49PM +0100, Christian Gusenbauer > > > > > wrote: > > > > > > On Thursday 24 January 2013 19:07:23 Konstantin Belousov > > > > > > wrote: > > > > > > > On Thu, Jan 24, 2013 at 08:03:59PM +0200, Konstantin > > > > > > > Belousov wrote: > > > > > > > > On Thu, Jan 24, 2013 at 06:05:57PM +0100, Christian > > > > > > > > Gusenbauer wrote: > > > > > > > > > Hi! > > > > > > > > > > > > > > > > > > I'm using 9.1 stable svn revision 245605 and I get the > > > > > > > > > panic below > > > > > > > > > if I execute the following commands (as single user): > > > > > > > > > > > > > > > > > > # swapon -a > > > > > > > > > # dumpon /dev/ada0s3b > > > > > > > > > # mount -u / > > > > > > > > > # ifconfig age0 inet 192.168.2.2 mtu 6144 up > > > > > > > > > # mount -t nfs -o rsize=3D32768 data:/multimedia /mnt > > > > > > > > > # cp /mnt/Movies/test/a.m2ts /tmp > > > > > > > > > > > > > > > > > > then the system panics almost immediately. I'll attach > > > > > > > > > the stack > > > > > > > > > trace. > > > > > > > > > > > > > > > > > > Note, that I'm using jumbo frames (6144 byte) on a 1Gbit > > > > > > > > > network, > > > > > > > > > maybe that's the cause for the panic, because the bcopy > > > > > > > > > (see stack > > > > > > > > > frame #15) fails. > > > > > > > > > > > > > > > > > > Any clues? > > > > > > > > > > > > > > > > I tried a similar operation with the nfs mount of > > > > > > > > rsize=3D32768 and mtu > > > > > > > > 6144, but the machine runs HEAD and em instead of age. I > > > > > > > > was unable > > > > > > > > to reproduce the panic on the copy of the 5GB file from > > > > > > > > nfs mount. > > > > > > > > > > > > Hmmm, I did a quick test. If I do not change the MTU, so just > > > > > > configuring > > > > > > age0 with > > > > > > > > > > > > # ifconfig age0 inet 192.168.2.2 up > > > > > > > > > > > > then I can copy all files from the mounted directory without > > > > > > any > > > > > > problems, too. So it's probably age0 related? > > > > > > > > > > From your backtrace and the buffer printout, I see somewhat > > > > > strange thing. > > > > > The buffer data address is 0xffffff8171418000, while kernel > > > > > faulted > > > > > at the attempt to write at 0xffffff8171413000, which is is lower > > > > > then > > > > > the buffer data pointer, at the attempt to bcopy to the buffer. > > > > > > > > > > The other data suggests that there were no overflow of the data > > > > > from the > > > > > server response. So it might be that mbuf_len(mp) returned > > > > > negative number > > > > > ? I am not sure is it possible at all. > > > > > > > > > > Try this debugging patch, please. You need to add INVARIANTS etc > > > > > to the > > > > > kernel config. > > > > > > > > > > diff --git a/sys/fs/nfs/nfs_commonsubs.c > > > > > b/sys/fs/nfs/nfs_commonsubs.c > > > > > index efc0786..9a6bda5 100644 > > > > > --- a/sys/fs/nfs/nfs_commonsubs.c > > > > > +++ b/sys/fs/nfs/nfs_commonsubs.c > > > > > @@ -218,6 +218,7 @@ nfsm_mbufuio(struct nfsrv_descript *nd, > > > > > struct uio > > > > > *uiop, int siz) } > > > > > mbufcp =3D NFSMTOD(mp, caddr_t); > > > > > len =3D mbuf_len(mp); > > > > > + KASSERT(len > 0, ("len %d", len)); > > > > > } > > > > > xfer =3D (left > len) ? len : left; > > > > > #ifdef notdef > > > > > @@ -239,6 +240,8 @@ nfsm_mbufuio(struct nfsrv_descript *nd, > > > > > struct uio > > > > > *uiop, int siz) uiop->uio_resid -=3D xfer; > > > > > } > > > > > if (uiop->uio_iov->iov_len <=3D siz) { > > > > > + KASSERT(uiop->uio_iovcnt > 1, ("uio_iovcnt %d", > > > > > + uiop->uio_iovcnt)); > > > > > uiop->uio_iovcnt--; > > > > > uiop->uio_iov++; > > > > > } else { > > > > > > > > > > I thought that server have returned too long response, but it > > > > > seems to > > > > > be not the case from your data. Still, I think the patch below > > > > > might be > > > > > due. > > > > > > > > > > diff --git a/sys/fs/nfsclient/nfs_clrpcops.c > > > > > b/sys/fs/nfsclient/nfs_clrpcops.c index be0476a..a89b907 100644 > > > > > --- a/sys/fs/nfsclient/nfs_clrpcops.c > > > > > +++ b/sys/fs/nfsclient/nfs_clrpcops.c > > > > > @@ -1444,7 +1444,7 @@ nfsrpc_readrpc(vnode_t vp, struct uio > > > > > *uiop, struct > > > > > ucred *cred, NFSM_DISSECT(tl, u_int32_t *, NFSX_UNSIGNED); > > > > > eof =3D fxdr_unsigned(int, *tl); > > > > > } > > > > > - NFSM_STRSIZ(retlen, rsize); > > > > > + NFSM_STRSIZ(retlen, len); > > > > > error =3D nfsm_mbufuio(nd, uiop, retlen); > > > > > if (error) > > > > > goto nfsmout; > > > > > I think this patch is appropriate, although I don't see it as too > critical. It just tightens the "sanity check" on the read reply > length (which should never exceed what the client requested). I agree, but client cannot control the server response. Anyway, I think there is too much things that could go wrong if the server actively exploit the client code. >=20 > nfsm_mbufuio() shouldn't transfer more than the uio structure can > handle, even if the replied read size is larger than requested. Yes, this what happen, I suppose, due to the decrement of the uio_iovcnt and the EBADRPC error return at the beginning of loop. But IMO the situation should be catched and asserted instead. This is why I added KASSERT(uio_iovcnt > 1) before the decrement. I do not think that we should both add my KASSERT for iovcnt and leave the EBADRPC return. What is your preference there ? >=20 > It does seem that nfsm_mbufuio() should apply a sanity check on > m_len. I think m_len =3D=3D 0 is ok, but negative or very large should > be checked for. Maybe just return EBADRPC after a printf() instead > of a KASSERT(), as a safety belt against a trashed m_len from a driver > or ??? The same as with the overflowed size, it would only hide another bug in the kernel. Probably, the assert m_len >=3D 0 and other useful assertions should be performed in the central place in the network stack, to catch an error earlier and for all consumers. >=20 > rick >=20 > > > > I applied your patches and now I get a > > > > > > > > panic: len -4 > > > > cpuid =3D 1 > > > > KDB: enter: panic > > > > Dumping 377 out of 6116 > > > > MB:..5%..13%..22%..34%..43%..51%..64%..73%..81%..94% > > > > > > > This means that the age driver either produced corrupted mbuf chain, > > > or filled wrong negative value into the mbuf len field. I am quite > > > certain that the issue is in the driver. > > > > > > I added the net@ to Cc:, hopefully you could get help there. > >=20 > > And I've cc'd Pyun who has written most of this driver and is likely > > the one > > most familiar with its handling of jumbo frames. > >=20 > > -- > > John Baldwin > > _______________________________________________ > > freebsd-fs@freebsd.org mailing list > > http://lists.freebsd.org/mailman/listinfo/freebsd-fs > > To unsubscribe, send any mail to "freebsd-fs-unsubscribe@freebsd.org" --E0e4ihfNxLmjeTLW Content-Type: application/pgp-signature -----BEGIN PGP SIGNATURE----- Version: GnuPG v2.0.19 (FreeBSD) iQIcBAEBAgAGBQJRAm4lAAoJEJDCuSvBvK1BsXEP/1/LNhc9/ugRGyAzhZyv/osk DL6tuIcfMfKaPekPG8hxQL5L/Pp5U0CAPnxwylVva82/A0qMo5l0PhOCUSVNluMu dfDh81dBYbJkKAgAeiN5s03OwfJCqGEubReXCDRW4O0xF0gQlGVdYHK7Fhm2zYxX ZlwbrjC1wlVnMa2NZ6NFT8MtXV4/jhoJ7wtyYWmwpJGo1faFH1m1GBabqM++S1UA 3ARV4t+nLAsifKFr4AHhCmoa2IuGvrWnzUwBcwSN5EDHyJlVswDRazYxRK3vDgbz dnG7egSEXlb8W24lzw+Fw/mZxX4s0+krmeh6yd2aC65BaaDd6vPA+uG2CGvj//lM FiS4yopOKDg2exdOKrn2AukLalqrZLsrHFWcpMDgwxyou9rTZ9HnbIPuL4X3Rpwd j68lSQCmHnwDAzP4aEt+sjVv5eZqwC/sfxsaPmpAUjtml1UDi+0ERXxL2loq4gM4 39k+qVKu1iJa7t630UvOeihLePS9OuyF7BvCk8cyCUARtfx/lpi7sTWgpan7ngxC ZYfP6KwkaWjuCRn7KjODB4TB0qyOEA6iX5QccdNhwN6bm1R0FxjvHChechg5t0ik sCXJrTe0Kk0rpbsXmyNsTZICXUe5osYTt3PFmqFUZvxJW+RoIcvnOzqc1K/u3w63 wUXMcErVczRhnQiMl0Vr =1nmz -----END PGP SIGNATURE----- --E0e4ihfNxLmjeTLW--