kern/85503: panic: wrong dirclust using msdosfs in RELENG_6

[Dmitry Pryanishnikov]

>Number:         85503
>Category:       kern
>Synopsis:       panic: wrong dirclust using msdosfs in RELENG_6
>Confidential:   no
>Severity:       critical
>Priority:       medium
>Responsible:    freebsd-bugs
>State:          open
>Quarter:        
>Keywords:       
>Date-Required:
>Class:          sw-bug
>Submitter-Id:   current-users
>Arrival-Date:   Tue Aug 30 23:50:25 GMT 2005
>Closed-Date:
>Last-Modified:
>Originator:     Dmitry Pryanishnikov
>Release:        FreeBSD 6.0-BETA3 i386
>Organization:
Atlantis ISP
>Environment:
System:

 	FreeBSD 6.0-BETA3 #0: Mon Aug 22 22:59:46 UTC 2005
 	root at harlow.cse.buffalo.edu:/usr/obj/usr/src/sys/GENERIC i386

 	Large enough (24Gb) FAT32 slice.

>Description:

     RELENG_6 panics during 'du' run against large FAT32 slice with many
directories. This is the specific regression in RELENG_6 against earlier
code (e.g. RELENG_5). Here is the crash dump analysis:

Unread portion of the kernel message buffer:
panic: wrong dirclust

(kgdb) where
...
#12 0xc06321f7 in panic (fmt=0x282 <Address 0x282 out of bounds>)
     at /usr/src/sys/kern/kern_shutdown.c:537
#13 0xc05eebaf in deget (pmp=0xc14d9000, dirclust=673396, diroffset=0,
     depp=0xc6ffda00) at /usr/src/sys/fs/msdosfs/msdosfs_denode.c:142
#14 0xc05f143d in msdosfs_lookup (ap=0x0)
     at /usr/src/sys/fs/msdosfs/msdosfs_lookup.c:534
#15 0xc07fe1da in VOP_CACHEDLOOKUP_APV (vop=0x0, a=0xc6ffda7c)
     at vnode_if.c:150
#16 0xc067b66e in vfs_cache_lookup (ap=0x0) at vnode_if.h:82
#17 0xc07fe123 in VOP_LOOKUP_APV (vop=0xc08b55e0, a=0xc6ffdb14)
     at vnode_if.c:99
#18 0xc067f53e in lookup (ndp=0xc6ffdba0) at vnode_if.h:56
#19 0xc067ef06 in namei (ndp=0xc6ffdba0) at /usr/src/sys/kern/vfs_lookup.c:201
#20 0xc068aea7 in kern_lstat (td=0xc1376c00, path=0x0, pathseg=UIO_USERSPACE,
     sbp=0xc6ffdc74) at /usr/src/sys/kern/vfs_syscalls.c:2102
#21 0xc068ae43 in lstat (td=0xc1376c00, uap=0xc6ffdd04)
     at /usr/src/sys/kern/vfs_syscalls.c:2086
...
(kgdb) fr 13
#13 0xc05eebaf in deget (pmp=0xc14d9000, dirclust=673396, diroffset=0,
     depp=0xc6ffda00) at /usr/src/sys/fs/msdosfs/msdosfs_denode.c:142
142                     KASSERT((*depp)->de_dirclust == dirclust, ("wrong dirclust"));
(kgdb) list
...
140             if (nvp != NULL) {
141                     *depp = VTODE(nvp);
142                     KASSERT((*depp)->de_dirclust == dirclust, ("wrong dirclust"));

Looking into the source (sys/fs/msdosfs/msdosfs_denode.c, function deget)
gives the following:

         uint64_t inode;		// Our "inode" number must have 64 bits
 	...
 	inode = pmp->pm_bpcluster * dirclust + diroffset;

This is the apparent place of the error: all members of right-hand
expression are u_long (32 bits on i386), so inode will always be
evaluated as 32-bit integer. Check this:

(kgdb) print dirclust
$2 = 673396
(kgdb) print pmp->pm_bpcluster
$3 = 16384
(kgdb) print diroffset
$4 = 0
(kgdb) print inode
$5 = 2442985472

Actually 16384 * 673396 + 0 should give us 11032920064. So vfs_hash_get()
gave us another vnode with the same low-order 32 bits of inode:

(kgdb) print **depp
$7 = {de_vnode = 0xc1805aa0, de_flag = 0, de_dev = 0x0, de_dirclust = 149108,
   de_diroffset = 0, de_fndoffset = 0, de_fndcnt = 0, de_refcnt = 1,
   de_pmp = 0xc14d9000, de_Name = ".          ", de_Attributes = 16 '\020',
   de_LowerCase = 0 '\0', de_CHun = 47 '/', de_CTime = 39991,
   de_CDate = 12972, de_ADate = 12972, de_MTime = 39991, de_MDate = 12972,
   de_StartCluster = 149108, de_FileSize = 16384, de_fc = {{fc_frcn = 0,
     fc_fsrcn = 149108}, {fc_frcn = 0, fc_fsrcn = 149108}},
   de_modrev = 140731985215, de_lockf = 0x0, de_inode = 2442985472}

Original directory cluster gives (in 32-bit expression):

(kgdb) print /x 673396*16384
$8 = 0x919d0000

This (incorrect) one gives the same:

(kgdb) print /x 149108*16384
$9 = 0x919d0000

>How-To-Repeat:

  Just install 6.0-BETA3 on i386 machine, mount large FAT32 partition with many
directories and run 'du' against this partition.

>Fix:

  The primary bug (evaluation of the inode) can easily be fixed using cast:

 	inode = (uint64_t) pmp->pm_bpcluster * dirclust + diroffset;

But the real problem is that the second argument of vfs_hash_get() also
has the same 32-bit limitation on i386 (type u_int, 32 bits), so I think
it's the author of vfs_hash_get() approach (phk) who should decide whether to
promote uint64_t to vfs_hash_get() or decouple msdosfs from this function
at all.

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