Re: LifeDrive filsystem probe fails [mjg59@srcf.ucam.org: Re:

[Karel Zak <kzak@redhat.com>]

On Tue, Nov 11, 2008 at 04:40:43PM +0100, Kay Sievers wrote:
> > I've made an image of a newly-formatted partition and posted it at
> > http://launchpadlibrarian.net/19129151/sdb1.img.bz2 and the Ubuntu bug

 Thanks.

> > (Thought you can
> > force-mount it with a -t vfat.)
> >
> > ----- Forwarded message from Matthew Garrett <mjg59@srcf.ucam.org> -----
> >
> > From: Matthew Garrett <mjg59@srcf.ucam.org>
> > To: Matt Behrens <matt@zigg.com>
> > Subject: Re: LifeDrive and T|X not seen by hal
> >
> > Ok. The following things appear to be causing the failure:
> >
> > 1) The FAT signature in bytes 510 and 511 isn't present. This causes the
> > code to bail.

 Right.

  # hexdump -s 0x1fe -n 5 -C /dev/loop0
  000001fe  00 00 00 00 00                                    |.....|

 I'm not sure is the msdos signature (0x55 0xaa) at 510 and 511 is really
 mandatory.  For example libblkid does not require this signature:

 # blkid /dev/loop0 
 /dev/loop0: LABEL="LIFEDRIVE" UUID="1234-5678" TYPE="vfat"

 when the standard FAT magic string is present. It works for years.

> > 2) The FAT32 signature in the fsinfo block isn't present. This causes
> > the code to bail.

 Right. Frankly, I have no clue why volume_id is checking also the
 FSInfo block.

 IMHO the FSInfo block is optional (if fsinfo_sector is zero) and
 should be _ignored_ when the signature does not match. See

 http://mail.opensolaris.org/pipermail/ufs-discuss/2007-February/000813.html

 The linux kernel also does not require valid FSInfo signature, the
 wrong signature is only reported:

 FAT: Invalid FSINFO signature: 0x00000000, 0x00000000 (sector = 1)

 and the FSInfo block is ignored at all.

 Fortunately, we check many other things (sector counts, number of FAT
 tabs, media check, cluster size, ...) in FAT super block.

> Hmm, we can not really remove these checks, as there are many volumes
> out there which we would detect as FAT, which are not FAT. Most of
> these additional checks only got added after a several reports of

> mis-detection. Even partition tables may get detected as FAT, if we
> remove these checks, without adding new ones.

 The Palm uses the FAT32 magic string, so mis-detection based on bytes
 510 and 511 shouldn't happen.

> Maybe we should always probe for all known filesystems, and not stop
> at the first match. And if we only find FAT, and nothing else, we

 Maybe for some cases. The mount(8) supports list of FS types. I can
 imagine that the library returns more results. It could be
 useful for example for CD-ROMs where is more filesystems.

> might be able to say it's FAT even with less strict checks. Adding
> Karel to Cc, because we plan to merge volume_id and blkid some day.

 Right now I'm working on regression tests ... so strange FS images
 are welcomed ;-)


 The patch below is for optimistic people :-)

 ./vol_id /dev/loop0
 ID_FS_USAGE=filesystem
 ID_FS_TYPE=vfat
 ID_FS_VERSIONúT32
 ID_FS_UUID\x1234-5678
 ID_FS_UUID_ENC\x1234-5678
 ID_FS_LABEL=LIFEDRIVE
 ID_FS_LABEL_ENC=LIFEDRIVE
 ID_FS_LABEL_SAFE=LIFEDRIVE

    Karel


From 6147da52751e437e7f6aaa6f6f32253b35eed174 Mon Sep 17 00:00:00 2001
From: Karel Zak <kzak@redhat.com>
Date: Thu, 13 Nov 2008 13:07:48 +0100
Subject: [PATCH] volume_id: less paranoid FAT32 detection

This patch

 * makes the msdos signature (0x55 0xaa) at 510 and 511 optional when
   the standard FAT magic string is present.

 * removes FSInfo block detection. The block is optional and should be
   ignored when the block signature is not valid. The block is not
   required by Linux kernel.

Signed-off-by: Karel Zak <kzak@redhat.com>
---
 extras/volume_id/lib/fat.c |   23 ++++-------------------
 1 files changed, 4 insertions(+), 19 deletions(-)

diff --git a/extras/volume_id/lib/fat.c b/extras/volume_id/lib/fat.c
index 759e106..0685234 100644
--- a/extras/volume_id/lib/fat.c
+++ b/extras/volume_id/lib/fat.c
@@ -277,10 +277,6 @@ int volume_id_probe_vfat(struct volume_id *id, uint64_t off, uint64_t size)
 	if (buf = NULL)
 		return -1;
 
-	/* check signature */
-	if (buf[510] != 0x55 || buf[511] != 0xaa)
-		return -1;
-
 	vs = (struct vfat_super_block *) buf;
 	if (memcmp(vs->sysid, "NTFS", 4) = 0)
 		return -1;
@@ -301,6 +297,10 @@ int volume_id_probe_vfat(struct volume_id *id, uint64_t off, uint64_t size)
 	if (memcmp(vs->type.fat.magic, "FAT12   ", 8) = 0)
 		goto magic;
 
+	/* check signature */
+	if (buf[510] != 0x55 || buf[511] != 0xaa)
+		return -1;
+
 	/* some old floppies don't have a magic, expect the boot jump address to match */
 	if ((vs->boot_jump[0] != 0xeb || vs->boot_jump[2] != 0x90) &&
 	     vs->boot_jump[0] != 0xe9)
@@ -404,21 +404,6 @@ magic:
 	goto found;
 
 fat32:
-	/* FAT32 should have a valid signature in the fsinfo block */
-	fsinfo_sect = le16_to_cpu(vs->type.fat32.fsinfo_sector);
-	buf = volume_id_get_buffer(id, off + (fsinfo_sect * sector_size), 0x200);
-	if (buf = NULL)
-		return -1;
-	fsinfo = (struct fat32_fsinfo *) buf;
-	if (memcmp(fsinfo->signature1, "\x52\x52\x61\x41", 4) != 0)
-		return -1;
-	if (memcmp(fsinfo->signature2, "\x72\x72\x41\x61", 4) != 0)
-		return -1 ;
-
-	vs = (struct vfat_super_block *) volume_id_get_buffer(id, off, 0x200);
-	if (vs = NULL)
-		return -1;
-
 	strcpy(id->type_version, "FAT32");
 
 	/* FAT32 root dir is a cluster chain like any other directory */
-- 
1.5.6.5