Bug 19086

On Wed, 7 Jun 2000, Mark Abene wrote:

> I just did some poking around, and the problem is *not* with the vn driver
> at all, but the msdosfs driver.  I just tried mounting a dos floppy and
> tried to read/write it, and ran into the exact same problem with
> "Argument list too long".  I have MSDOSFS compiled into the kernel.
> Unfortunately, I don't have 4.0-RELEASE on any Intel boxes, only Alpha.
> I can verify that this problem doesn't exist with 3.4-RELEASE on Intel.

The following quick fix seems to be sufficient on i386's with 64-bit longs:

---
diff -c2 msdosfsmount.h~ msdosfsmount.h
*** msdosfsmount.h~	Thu Jun  8 15:56:37 2000
--- msdosfsmount.h	Thu Jun  8 16:01:44 2000
***************
*** 85,89 ****
  	u_long pm_fatblocksec;	/* size of fat blocks in sectors */
  	u_long pm_fatsize;	/* size of fat in bytes */
! 	u_long pm_fatmask;	/* mask to use for fat numbers */
  	u_long pm_fsinfo;	/* fsinfo block number */
  	u_long pm_nxtfree;	/* next free cluster in fsinfo block */
--- 85,89 ----
  	u_long pm_fatblocksec;	/* size of fat blocks in sectors */
  	u_long pm_fatsize;	/* size of fat in bytes */
! 	u_int32_t pm_fatmask;	/* mask to use for fat numbers */
  	u_long pm_fsinfo;	/* fsinfo block number */
  	u_long pm_nxtfree;	/* next free cluster in fsinfo block */
---

pm_fatmask is 0xfff for 12-bit fats, so its complement is 0xfffffffffffff000
on machines with 64-bit longs, so comparisions like

    (cn | ~pmp->pm_fatmask) == CLUST_RSRVD)	/* CLUST_RSVRD is 0xfffffff6 */

always fail on such machines.  The quick fix fixes at least this bug on
machines with ints no larger than 32 bit.  The big is subtler on machines
with ints larger than 32 bits.  On such machines, u_int32_t has precisely
32 bits, so it must be smaller than int, so in the calculation of its
complement, it will be promoted before taking complements, again giving
too many high bits.

The correct fix is:
- replace ~pmp->pm_fatmask by (~pmp->pm_fatmask & 0xffffffff) everywhere
  (actually use a macro instead of 0xffffffff).
- finish implementing <inttypes.h> (actually <stdint.h>).
- use uint_least32_t from <inttypes.h> for pm_fatmask.  Don't use u_int32_t,
  since it is a deprecated spelling of uint32_t.  Don't use uint32_t or
  u_long, since they may be pessimal.  u_long _is_ pessimal on i386's with
  64-bit longs.  Similarly for many other types in the kernel.

Bruce

I just spent the past few hours narrowing the problem down to the very same
thing.  Only now with this patch the kernel panics with an "unexpected machine
check" when trying to write more than 1024 bytes to a dos floppy.
Not good.  The program counter shows it to be bombing out in the
unaligned_fixup routine.  I'm stumped for the time being.
I'll investigate further but would be really curious to hear if you know
why this should be happening.

Cheers,
-Mark

On Thu, Jun 08, 2000 at 04:47:24PM +1000, Bruce Evans wrote:
> The following quick fix seems to be sufficient on i386's with 64-bit longs:
> 
> ---
> diff -c2 msdosfsmount.h~ msdosfsmount.h
> *** msdosfsmount.h~	Thu Jun  8 15:56:37 2000
> --- msdosfsmount.h	Thu Jun  8 16:01:44 2000
> ***************
> *** 85,89 ****
>   	u_long pm_fatblocksec;	/* size of fat blocks in sectors */
>   	u_long pm_fatsize;	/* size of fat in bytes */
> ! 	u_long pm_fatmask;	/* mask to use for fat numbers */
>   	u_long pm_fsinfo;	/* fsinfo block number */
>   	u_long pm_nxtfree;	/* next free cluster in fsinfo block */
> --- 85,89 ----
>   	u_long pm_fatblocksec;	/* size of fat blocks in sectors */
>   	u_long pm_fatsize;	/* size of fat in bytes */
> ! 	u_int32_t pm_fatmask;	/* mask to use for fat numbers */
>   	u_long pm_fsinfo;	/* fsinfo block number */
>   	u_long pm_nxtfree;	/* next free cluster in fsinfo block */
> ---
>

On Thu, 8 Jun 2000, Mark Abene wrote:

> I just spent the past few hours narrowing the problem down to the very same
> thing.  Only now with this patch the kernel panics with an "unexpected machine
> check" when trying to write more than 1024 bytes to a dos floppy.
> Not good.  The program counter shows it to be bombing out in the
> unaligned_fixup routine.  I'm stumped for the time being.
> I'll investigate further but would be really curious to hear if you know
> why this should be happening.

msdosfs metadata is riddled with unaligned fields in Intel (little endian)
order.  naligned_fixup() apparently has problems fixing up the unaligned
accesses related to this.  Try changing getushort(), etc., in msdosfs to
do byte accesses even in the little endian case.  These macros currently
only do byte access to fix up the byte order in the big endian case.  It
should use something like ntohs() for that and be more careful about
unaligned accesses.

Bruce

On Fri, Jun 09, 2000 at 12:06:35AM +1000, Bruce Evans wrote:
> msdosfs metadata is riddled with unaligned fields in Intel (little endian)
> order.  naligned_fixup() apparently has problems fixing up the unaligned
> accesses related to this.  Try changing getushort(), etc., in msdosfs to
> do byte accesses even in the little endian case.  These macros currently
> only do byte access to fix up the byte order in the big endian case.  It
> should use something like ntohs() for that and be more careful about
> unaligned accesses.
> 
> Bruce
> 

That did the trick!  It's now working nicely.  Please accept the following
interim patch against 4.0-RELEASE, and thanks for the assistance!

**cut here**
--- msdosfs.orig/bpb.h	Fri Aug 27 20:48:07 1999
+++ msdosfs/bpb.h	Thu Jun  8 16:25:15 2000
@@ -114,7 +114,7 @@
  * use the macros for the big-endian case.
  */
 #include <machine/endian.h>
-#if (BYTE_ORDER == LITTLE_ENDIAN) 			/* && defined(UNALIGNED_ACCESS) */
+#if (BYTE_ORDER == LITTLE_ENDIAN) && !defined(__alpha__)	/* && defined(UNALIGNED_ACCESS) */
 #define	getushort(x)	*((u_int16_t *)(x))
 #define	getulong(x)	*((u_int32_t *)(x))
 #define	putushort(p, v)	(*((u_int16_t *)(p)) = (v))
--- msdosfs.orig/msdosfsmount.h	Thu Jan 27 09:43:07 2000
+++ msdosfs/msdosfsmount.h	Thu Jun  8 16:14:18 2000
@@ -84,7 +84,7 @@
 	u_long pm_fatblocksize;	/* size of fat blocks in bytes */
 	u_long pm_fatblocksec;	/* size of fat blocks in sectors */
 	u_long pm_fatsize;	/* size of fat in bytes */
-	u_long pm_fatmask;	/* mask to use for fat numbers */
+	u_int32_t pm_fatmask;	/* mask to use for fat numbers */
 	u_long pm_fsinfo;	/* fsinfo block number */
 	u_long pm_nxtfree;	/* next free cluster in fsinfo block */
 	u_int pm_fatmult;	/* these 2 values are used in fat */
**cut here**

Cheers,
-Mark

A quick note for the audit trail.  A quick fix applied by bde as:

	1.8       +8 -2      src/sys/msdosfs/bpb.h
	1.22      +2 -2      src/sys/msdosfs/msdosfsmount.h

Ciao,
Sheldon.

Comment 6 Bruce Evans 2000-10-27 10:51:40 UTC

State Changed
From-To: open->closed

Hopefully fixed.  Suggested patches applied without testing by me in all 
versions back to RELENG_3.