kern/84983: udf filesystem: stat-ting files could randomly fail

[Andriy Gapon]

>Number:         84983
>Category:       kern
>Synopsis:       udf filesystem: stat-ting files could randomly fail
>Confidential:   no
>Severity:       non-critical
>Priority:       medium
>Responsible:    freebsd-bugs
>State:          open
>Quarter:        
>Keywords:       
>Date-Required:
>Class:          sw-bug
>Submitter-Id:   current-users
>Arrival-Date:   Tue Aug 16 09:50:21 GMT 2005
>Closed-Date:
>Last-Modified:
>Originator:     Andriy Gapon
>Release:        FreeBSD 5.4-RELEASE-p3 i386
>Organization:
>Environment:
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.
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;
--- lookup.patch ends here ---


>Release-Note:
>Audit-Trail:
>Unformatted: