Subject: kern/32003: msdosfs doesn't properly zero out high cluster data on non-FAT32 msdos filesystems.
To: None <kern-bug-people@netbsd.org, gnats-admin@netbsd.org,>
From: Brian Buhrow <buhrow@lothlorien.nfbcal.org>
List: netbsd-bugs
Date: 11/04/2005 17:43:00
>Number:         32003
>Category:       kern
>Synopsis:       The msdosfs code doesn't insure that deHighClust in directory entries is zero on non-FAT32 filesystems.
>Confidential:   no
>Severity:       serious
>Priority:       high
>Responsible:    kern-bug-people
>State:          open
>Class:          sw-bug
>Submitter-Id:   net
>Arrival-Date:   Fri Nov 04 17:43:00 +0000 2005
>Originator:     Brian Buhrow
>Release:        NetBSD 2.0 and above
>Organization:
NFB of California
	
>Environment:
	
	
System: NetBSD lothlorien.nfbcal.org 2.0 NetBSD 2.0 (NFBNETBSD) #0: Tue Jan 4 10:03:02 PST 2005 buhrow@lothlorien.nfbcal.org:/usr/src/sys/arch/i386/compile/NFBNETBSD i386
Architecture: i386
Machine: i386
>Description:
	
	When creating and changing files in the msdos format, the msdosfs code
in NetBSD, and I believe freeBSD as well, doesn't insure that the
deHighClust bytes are cleared for non-FAT32 filesystems.  These bytes are
not supposed to be used in non-FAT32 filesystems, and Windows XP, at least,
does properly ignore these bytes.  However, there are some portable
devices, such as MP3 players, cameras, etc. which pay attention to the
deHighClust values, even if the filesystem is not FAT32.  The existing code
in NetBSD does not explicitly set the values of these bytes in non-FAT32
situations, meaning random garbage gets put there.  This has the effect of
causing files created under NetBSD to be sometimes corrupt from the
appliance's perspective.  The following patch insures that the deHighClust
values are set to 0 for non-FAT32 filesystems.  I have tested this patch
with a known picky device, and it works well.
	If this could be pulled up to NetBSD-3 and NetBSD-2, that would be great.
>How-To-Repeat:
	
	Mount a non-FAT32 filesystem, and create several files or directories
on that filesystem.  Then, use hexdump to examine the contents of your new
directory entries.  You'll most likely see odd values in the spaces reserved
for the deHighClust information.  However, because the data which is
installed there is what ever happens to be in RAM, it might be that
everything looks good.  It's taken me several months of constant use to
discover the misbehavior, because much of the time, zero was was what was
installed.
	Alternatively, if you have a picky device, and you've noticed that
NetBSD constructed files seem to be corrupt from time to time, while ones
written by Windows always work, you're probably encountering the bug.
>Fix:
	This patch fixes the problem.
-Brian

Index: denode.h
===================================================================
RCS file: /cvsroot/src/sys/fs/msdosfs/denode.h,v
retrieving revision 1.4
diff -u -r1.4 denode.h
--- denode.h	7 Sep 2003 22:09:11 -0000	1.4
+++ denode.h	4 Nov 2005 17:17:37 -0000
@@ -199,6 +199,8 @@
 
 #define DE_EXTERNALIZE32(dp, dep)			\
 	 putushort((dp)->deHighClust, (dep)->de_StartCluster >> 16)
+#define DE_EXTERNALIZE16(dp, dep)			\
+	 putushort((dp)->deHighClust, 0 >> 16)
 #define DE_EXTERNALIZE(dp, dep)				\
 	(memcpy((dp)->deName, (dep)->de_Name, 11),	\
 	 (dp)->deAttributes = (dep)->de_Attributes,	\
@@ -211,7 +213,7 @@
 	 putushort((dp)->deStartCluster, (dep)->de_StartCluster), \
 	 putulong((dp)->deFileSize,			\
 	     ((dep)->de_Attributes & ATTR_DIRECTORY) ? 0 : (dep)->de_FileSize), \
-	 (FAT32((dep)->de_pmp) ? DE_EXTERNALIZE32((dp), (dep)) : 0))
+	 (FAT32((dep)->de_pmp) ? DE_EXTERNALIZE32((dp), (dep)) : DE_EXTERNALIZE16((dp), (dep))))
 
 #define	de_forw		de_chain[0]
 #define	de_back		de_chain[1]
Index: msdosfs_vnops.c
===================================================================
RCS file: /cvsroot/src/sys/fs/msdosfs/msdosfs_vnops.c,v
retrieving revision 1.9
diff -u -r1.9 msdosfs_vnops.c
--- msdosfs_vnops.c	26 Jan 2004 10:39:30 -0000	1.9
+++ msdosfs_vnops.c	4 Nov 2005 17:17:38 -0000
@@ -1141,7 +1141,10 @@
 		if (FAT32(pmp)) {
 			putushort(dotdotp->deHighClust,
 				dp->de_StartCluster >> 16);
-		}
+		} else {
+			putushort(dotdotp->deHighClust,
+				0 >> 16);
+			}
 		if ((error = bwrite(bp)) != 0) {
 			/* XXX should really panic here, fs is corrupt */
 			VOP_UNLOCK(fvp, 0);
@@ -1268,6 +1271,9 @@
 	if (FAT32(pmp)) {
 		putushort(denp[0].deHighClust, newcluster >> 16);
 		putushort(denp[1].deHighClust, pdep->de_StartCluster >> 16);
+	} else {
+		putushort(denp[0].deHighClust, 0 >> 16);
+		putushort(denp[1].deHighClust, 0 >> 16);
 	}
 
 	if (async)

>Unformatted: