From owner-freebsd-bugs@FreeBSD.ORG Wed Aug 17 00:38:08 2005 Return-Path: X-Original-To: freebsd-bugs@freebsd.org Delivered-To: freebsd-bugs@freebsd.org Received: from mx1.FreeBSD.org (mx1.freebsd.org [216.136.204.125]) by hub.freebsd.org (Postfix) with ESMTP id 0DAC216A43D; Wed, 17 Aug 2005 00:38:08 +0000 (GMT) (envelope-from bde@zeta.org.au) Received: from mailout2.pacific.net.au (mailout2.pacific.net.au [61.8.0.115]) by mx1.FreeBSD.org (Postfix) with ESMTP id 3D24343D45; Wed, 17 Aug 2005 00:38:07 +0000 (GMT) (envelope-from bde@zeta.org.au) Received: from mailproxy2.pacific.net.au (mailproxy2.pacific.net.au [61.8.0.87]) by mailout2.pacific.net.au (8.13.4/8.13.4/Debian-3) with ESMTP id j7H0c1o7020458; Wed, 17 Aug 2005 10:38:01 +1000 Received: from epsplex.bde.org (katana.zip.com.au [61.8.7.246]) by mailproxy2.pacific.net.au (8.13.4/8.13.4/Debian-3) with ESMTP id j7H0bwkw015401; Wed, 17 Aug 2005 10:37:59 +1000 Date: Wed, 17 Aug 2005 10:37:58 +1000 (EST) From: Bruce Evans X-X-Sender: bde@epsplex.bde.org To: Andriy Gapon In-Reply-To: <200508160942.j7G9gPJe054914@oddity.topspin.kiev.ua> Message-ID: <20050817101317.U988@epsplex.bde.org> References: <200508160942.j7G9gPJe054914@oddity.topspin.kiev.ua> MIME-Version: 1.0 Content-Type: TEXT/PLAIN; charset=US-ASCII; format=flowed Cc: freebsd-bugs@freebsd.org, FreeBSD-gnats-submit@freebsd.org Subject: Re: kern/84983: udf filesystem: stat-ting files could randomly fail X-BeenThere: freebsd-bugs@freebsd.org X-Mailman-Version: 2.1.5 Precedence: list List-Id: Bug reports List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Wed, 17 Aug 2005 00:38:08 -0000 On Tue, 16 Aug 2005, Andriy Gapon wrote: > System: > FreeBSD 5.4-RELEASE-p3 #3: Sat Jul 9 17:02:15 EEST 2005 i386 > >> Description: > Sometimes stat(2) on files located on UDF fs unexpectedly fails, at the same > time "udf: invalid FID fragment" kernel messages are produced. > umount+mount in such situation usually cures the problem, but sometimes > I have to do kldunload udf in between. > The symptom very much looks like use of unitialized variable/memory. Indeed. > Brief search for a culprit made me suspect udf_node.diroff field. > This is how udf_node structures are allocated: > > udf_vfsops.c: struct udf_node *unode; > ... > unode = uma_zalloc(udf_zone_node, M_WAITOK); > > i.e. there is no M_ZERO while allocating udf_node and diroff field does > not seem to be initialized explicitely: > > $ fgrep diroff *.[ch] > udf.h: long diroff; > udf_vnops.c: if (nameiop != LOOKUP || node->diroff == 0 || node->diroff > fsize) { > udf_vnops.c: offset = node->diroff; > udf_vnops.c: node->diroff = ds->offset + ds->off; > > as you can see diroff could be used before it is assigned and it is used in > udf_lookup() function as follows: > > if (nameiop != LOOKUP || node->diroff == 0 || node->diroff > fsize) { > offset = 0; > numdirpasses = 1; > } else { > offset = node->diroff; > numdirpasses = 2; > nchstats.ncs_2passes++; > } > > lookloop: > ds = udf_opendir(node, offset, fsize, udfmp); > > as you can see, if diroff belongs to interval (0, fsize] and nameiop is LOOKUP, > then offset variable will be assigned with its (random) value that, in turn, > would be passed down to udf_opendir and, thus, directory stream would contain > incorrect (arbitrary) data. > >> How-To-Repeat: > because of probabalistic nature of the bug, there is no definite recipe of > reproducing it. you could try to do a lot of operations (ls, stat) on files > on UDF fs and see if eventually directory udf_node will be allocated with > "good" junk at diroff position. > for me, this happens from time to time on its own. > >> Fix: > > if my theory is correct, then the following patch should fix the problem by > adding proper initialization to diroff field of udf_node: > > --- lookup.patch begins here --- > --- sys/fs/udf/udf_vfsops.c.orig Mon Aug 15 21:06:50 2005 > +++ sys/fs/udf/udf_vfsops.c Mon Aug 15 21:07:07 2005 > @@ -648,6 +648,7 @@ > return (error); > } > > + unode->diroff = 0; > unode->i_vnode = vp; > unode->hash_id = ino; > unode->i_devvp = udfmp->im_devvp; This is fixed accidentally in in -current. I sent essentially the above fix together with fixes for many other bugs in udf to the udf maintainer 4 months ago: % Date: Tue, 19 Apr 2005 21:24:34 +1000 (EST) % From: Bruce Evans % To: scottl@freebsd.org % Subject: Re: lots of bugs in udf % % On Sun, 10 Apr 2005, Bruce Evans wrote: % % > ... % > Here is my work-in-progress patch: % % Here is a better patch with some other bugs fixed: % % % Index: udf_vfsops.c % % =================================================================== % % RCS file: /home/ncvs/src/sys/fs/udf/udf_vfsops.c,v % % retrieving revision 1.18 % % diff -u -2 -r1.18 udf_vfsops.c % % --- udf_vfsops.c 23 Jun 2004 19:36:09 -0000 1.18 % % +++ udf_vfsops.c 19 Apr 2005 09:38:36 -0000 % % @@ -654,4 +654,5 @@ % % unode->i_dev = udfmp->im_dev; % % unode->udfmp = udfmp; % % + unode->diroff = 0; % % vp->v_data = unode; % % VREF(udfmp->im_devvp); % % unode->diroff was not initialized, so udf_lookup() tended to use garbage % offsets and fail after vnodes were recycled. % % The second pass in udf_lookup() should hide this problem, but doesn't % always because the second pass is not attempted after a bad fid. If % the stale offset is for a file in the same directory, then the fid is % not bad and the second pass works. % % This was fixed as an undocumented side effect of rev.1.28. uma_zalloc() % now M_ZEROs everything, although this is less needed than before because % changes associated with rev.1.28 removed fields from the struct, leaving % only 5 fields so they are very easy to initialized explicitly. A similar % change was made to ffs. It may be more needed there. I prefer the old % code. % % [... description of other bugs and fixes deleted] Bruce