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

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

[Matt Behrens]

[-- Attachment #1: Type: text/plain, Size: 1914 bytes --]

Hi there,

The Palm LifeDrive and T|X have a "Drive Mode" functionality that exposes
the onboard FAT32 partitions as USB drives.  However, Palm's idea of what
constitutes a FAT32 partition is a bit... weak. ;)  Below are Matthew
Garrett's insights on the filesystem probe failure from the hal list;
he suggested I get in touch with the udev developers.

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
with more info can be found at
https://bugs.launchpad.net/ubuntu/+source/hal/+bug/81219.

It's unfortunately not enough to format the partition with mkdosfs or
fsck it into compliance; the device will "fix" the partition at some
point in the future rendering it unmountable again.  (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.

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

3) The loop looking for the root dir appears to jump out past the code 
that sets the filesystem label. This results in it ignoring the UUID 
and filesystem name.

You probably need to get in touch with the udev developers to get this 
fixed - I'm not sure which of these checks could result in false 
positives unless care is taken. Once libvolume_id is fixed hal should 
just work.

-- 
Matthew Garrett | mjg59@srcf.ucam.org
_______________________________________________
hal mailing list
hal@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/hal

----- End forwarded message -----

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 189 bytes --]

Re: LifeDrive filsystem probe fails [mjg59@srcf.ucam.org: Re: LifeDrive and T|X not seen by hal]

[Kay Sievers]

On Tue, Nov 11, 2008 at 15:29, Matt Behrens <matt@zigg.com> wrote:
> The Palm LifeDrive and T|X have a "Drive Mode" functionality that exposes
> the onboard FAT32 partitions as USB drives.  However, Palm's idea of what
> constitutes a FAT32 partition is a bit... weak. ;)  Below are Matthew
> Garrett's insights on the filesystem probe failure from the hal list;
> he suggested I get in touch with the udev developers.
>
> 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
> with more info can be found at
> https://bugs.launchpad.net/ubuntu/+source/hal/+bug/81219.
>
> It's unfortunately not enough to format the partition with mkdosfs or
> fsck it into compliance; the device will "fix" the partition at some
> point in the future rendering it unmountable again.

Nice! Seems really smart. :)

> (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.
>
> 2) The FAT32 signature in the fsinfo block isn't present. This causes
> the code to bail.

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.

> 3) The loop looking for the root dir appears to jump out past the code
> that sets the filesystem label. This results in it ignoring the UUID
> and filesystem name.

Is it recognized as FAT, but we jump out of it? Or do we decide that
it is not FAT at all? Is it something that we can fix?

> You probably need to get in touch with the udev developers to get this
> fixed - I'm not sure which of these checks could result in false
> positives unless care is taken. Once libvolume_id is fixed hal should
> just work.

I guess, someone should try to get Palm to come up with a fix for that
thing they call FAT. :)

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
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.

It's a really scary thing, to guess filesystems based on a few bytes
we read out of a random byte stream. Other operating systems have far
less trouble to have less checks, because they don't support several
dozens of filesystems, and usually have formatting applications which
are not that broken as the Linux ones, regarding to clearing out old
signatures before applying a new format.

Thanks,
Kay

Re: LifeDrive filsystem probe fails [mjg59@srcf.ucam.org: Re: LifeDrive and T|X not seen by hal]

[Matthew Garrett]

On Tue, Nov 11, 2008 at 04:40:43PM +0100, Kay Sievers wrote:
> > 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.
> >
> > 2) The FAT32 signature in the fsinfo block isn't present. This causes
> > the code to bail.
> 
> 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.

Mm. Some form of "No, really, it's FAT" option keyed off the USB id?

> > 3) The loop looking for the root dir appears to jump out past the code
> > that sets the filesystem label. This results in it ignoring the UUID
> > and filesystem name.
> 
> Is it recognized as FAT, but we jump out of it? Or do we decide that
> it is not FAT at all? Is it something that we can fix?

That's after I patch the signature checks out. The set_label and 
set_uuid calls never get made because of the goto found in the loop 
looking for the root cluster.

> > You probably need to get in touch with the udev developers to get this
> > fixed - I'm not sure which of these checks could result in false
> > positives unless care is taken. Once libvolume_id is fixed hal should
> > just work.
> 
> I guess, someone should try to get Palm to come up with a fix for that
> thing they call FAT. :)

Yeah, good luck with that :)

-- 
Matthew Garrett | mjg59@srcf.ucam.org

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

[Karel Zak]

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

Re: LifeDrive filsystem probe fails [mjg59@srcf.ucam.org: Re: LifeDrive and T|X not seen by hal]

[Kay Sievers]

On Thu, Nov 13, 2008 at 13:55, Karel Zak <kzak@redhat.com> wrote:
> 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.

Sure, and it happily detects for years old left-over signatures as FAT
which are not FAT anymore. :) And the kernel sometimes even mounts
them. and likely corrupts the image by using it as FAT. Blkid is just
not exposed to the crap out there. It is not used for any widely
deployed auto-mount setup. Udev and HAL get all the stuff, and that's
why there are more checks added to volume_id. Maybe they can be made
less strict, but we need to be very very careful, Auto-mounting is a
really dangerous business, and you risk data loss by mounting stuff as
FAT, which isn't FAT anymore - which we have seem several times when
we started with HAL auto-mounting and the /dev/disk/ links.

>> > 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.

It's mandatory by the Microsoft FAT spec. But seems, they don't
require it in their implementation, but that's easy if you need to
support like 4 filesystems. :) We can maybe allow them all to be \0's.
Maybe!

>  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

> 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.

Sounds fine.

>  * 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.

Sure, but that's no reason. The kernel allows you to mount a mkswap'd
image too, as FAT or as swap, just as you like. :) Autodetection can
not pass everything the kernel accepts.

Maybe we can change the check to accept all bytes in the signature
beeing \0. This check exists for the reason that we had too many
wrongly detected FAT volumes, which are a formatted differently, and
even the kernel sometimes accepts, the no-loger-FAT volume and
corrupts the new filesystem. Just like it does after mkswap.

Kay

Re: LifeDrive filsystem probe fails [mjg59@srcf.ucam.org: Re: LifeDrive and T|X not seen by hal]

[Kay Sievers]

On Thu, Nov 13, 2008 at 14:40, Kay Sievers <kay.sievers@vrfy.org> wrote:
> On Thu, Nov 13, 2008 at 13:55, Karel Zak <kzak@redhat.com> wrote:
>> 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.
>
> Sure, and it happily detects for years old left-over signatures as FAT
> which are not FAT anymore. :) And the kernel sometimes even mounts
> them. and likely corrupts the image by using it as FAT. Blkid is just
> not exposed to the crap out there. It is not used for any widely
> deployed auto-mount setup. Udev and HAL get all the stuff, and that's
> why there are more checks added to volume_id. Maybe they can be made
> less strict, but we need to be very very careful, Auto-mounting is a
> really dangerous business, and you risk data loss by mounting stuff as
> FAT, which isn't FAT anymore - which we have seem several times when
> we started with HAL auto-mounting and the /dev/disk/ links.
>
>>> > 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.
>
> It's mandatory by the Microsoft FAT spec. But seems, they don't
> require it in their implementation, but that's easy if you need to
> support like 4 filesystems. :) We can maybe allow them all to be \0's.
> Maybe!
>
>>  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
>
>> 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.
>
> Sounds fine.
>
>>  * 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.
>
> Sure, but that's no reason. The kernel allows you to mount a mkswap'd
> image too, as FAT or as swap, just as you like. :) Autodetection can
> not pass everything the kernel accepts.
>
> Maybe we can change the check to accept all bytes in the signature
> beeing \0. This check exists for the reason that we had too many
> wrongly detected FAT volumes, which are a formatted differently, and
> even the kernel sometimes accepts, the no-loger-FAT volume and
> corrupts the new filesystem. Just like it does after mkswap.

I think a reasonable solution is to always probe for all filesystems
we know, and don't return a single result when we find several
non-compatible filesystems.

We would need to group some filesytems to allow things like udf and
iso9660, hfs(+) and iso9660, hfs and hfs+ to co-exist at the same time
for the same volume. But most other combinations should by default not
return a probing result, and just print an error with the conflicting
signatures we have found. Users would be required to remove the wrong
signature, or reformat the volume if they want auto-mounting to work.

We can optionally limit this behavior to devices which are not
read-only, like most optical media, and where mis-detection does not
do any harm.

That way we should be able to prevent common mis-detections and
prevent damage by auto-mounting, in the common scenario of
re-formatted volumes with a different filesystem, but left-over old
signatures. As said earlier, the kernel allows sometimes to mount the
filesystem in the "old" format and writes to it, and may just corrupt
the "new" filesystem by doing that.

If we find a raid signature, we must still stop here and just return
it, to possibly set it up as a raid member. It's common to see the
filesystem on top of raid while probing the raw disk.

We also need to add size-checks, or an option, to skip probing for all
filesystems on floppy disks and other really slow media, because they
are too slow to seek, and read like 64-128 kBytes, just to check for
other filesystems. Floppies are the main reason, we probe for FAT very
early in the sequence, it just takes too long to probe them.

After that, I think it is reasonable safe to remove some additional
checks from FAT, because we can be sure that it's not any other known
filesystem we know. I would still not return FAT32 if the fssig does
not match and is not all zero. Every time I see this check in
volume_id's FAT probing failing, it was all zero.

What do you think?

Kay

Re: LifeDrive filsystem probe fails [mjg59@srcf.ucam.org: Re: LifeDrive and T|X not seen by hal]

[Kay Sievers]

On Thu, Nov 13, 2008 at 16:07, Kay Sievers <kay.sievers@vrfy.org> wrote:
> On Thu, Nov 13, 2008 at 14:40, Kay Sievers <kay.sievers@vrfy.org> wrote:
>> On Thu, Nov 13, 2008 at 13:55, Karel Zak <kzak@redhat.com> wrote:
>>> 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.
>>
>> Sure, and it happily detects for years old left-over signatures as FAT
>> which are not FAT anymore. :) And the kernel sometimes even mounts
>> them. and likely corrupts the image by using it as FAT. Blkid is just
>> not exposed to the crap out there. It is not used for any widely
>> deployed auto-mount setup. Udev and HAL get all the stuff, and that's
>> why there are more checks added to volume_id. Maybe they can be made
>> less strict, but we need to be very very careful, Auto-mounting is a
>> really dangerous business, and you risk data loss by mounting stuff as
>> FAT, which isn't FAT anymore - which we have seem several times when
>> we started with HAL auto-mounting and the /dev/disk/ links.
>>
>>>> > 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.
>>
>> It's mandatory by the Microsoft FAT spec. But seems, they don't
>> require it in their implementation, but that's easy if you need to
>> support like 4 filesystems. :) We can maybe allow them all to be \0's.
>> Maybe!
>>
>>>  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
>>
>>> 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.
>>
>> Sounds fine.
>>
>>>  * 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.
>>
>> Sure, but that's no reason. The kernel allows you to mount a mkswap'd
>> image too, as FAT or as swap, just as you like. :) Autodetection can
>> not pass everything the kernel accepts.
>>
>> Maybe we can change the check to accept all bytes in the signature
>> beeing \0. This check exists for the reason that we had too many
>> wrongly detected FAT volumes, which are a formatted differently, and
>> even the kernel sometimes accepts, the no-loger-FAT volume and
>> corrupts the new filesystem. Just like it does after mkswap.
>
> I think a reasonable solution is to always probe for all filesystems
> we know, and don't return a single result when we find several
> non-compatible filesystems.
>
> We would need to group some filesytems to allow things like udf and
> iso9660, hfs(+) and iso9660, hfs and hfs+ to co-exist at the same time
> for the same volume. But most other combinations should by default not
> return a probing result, and just print an error with the conflicting
> signatures we have found. Users would be required to remove the wrong
> signature, or reformat the volume if they want auto-mounting to work.
>
> We can optionally limit this behavior to devices which are not
> read-only, like most optical media, and where mis-detection does not
> do any harm.
>
> That way we should be able to prevent common mis-detections and
> prevent damage by auto-mounting, in the common scenario of
> re-formatted volumes with a different filesystem, but left-over old
> signatures. As said earlier, the kernel allows sometimes to mount the
> filesystem in the "old" format and writes to it, and may just corrupt
> the "new" filesystem by doing that.
>
> If we find a raid signature, we must still stop here and just return
> it, to possibly set it up as a raid member. It's common to see the
> filesystem on top of raid while probing the raw disk.
>
> We also need to add size-checks, or an option, to skip probing for all
> filesystems on floppy disks and other really slow media, because they
> are too slow to seek, and read like 64-128 kBytes, just to check for
> other filesystems. Floppies are the main reason, we probe for FAT very
> early in the sequence, it just takes too long to probe them.
>
> After that, I think it is reasonable safe to remove some additional
> checks from FAT, because we can be sure that it's not any other known
> filesystem we know. I would still not return FAT32 if the fssig does
> not match and is not all zero. Every time I see this check in
> volume_id's FAT probing failing, it was all zero.

I've committed  these changes to volume_id, and FAT32 accepts
completely empty fsinfo signatures now.

Here is the Palm image with the volume_id git version:
  $ extras/volume_id/vol_id --size\x1640000 ../../volume_images/fat-palm.img
  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

We always probe for all filesystem types on volumes larger than a
floppy disk. If we find multiple signatures and one of the detected
filesystem types specifies that it can no co-exist with another known
filesystem, like swap and FAT, we do not return a probing result.

Here is the usual corrupted FAT+swap volume:
  $ extras/volume_id/vol_id --size\x1640000 swap+fat.img
  swap+fat.img: unknown volume type

  $ extras/volume_id/vol_id --debug --size\x1640000 swap+fat.img
  fat.c: cluster_count 0x3fd7
  util.c: get buffer off 0x10200(66048), len 0x4000
  util.c: read seekbuf off:0x10200 len:0x4000
  util.c: get buffer off 0x0(0), len 0x200
  volume_id.c: signature 'vfat' detected
  linux_swap.c: probing at offset 0x0
  util.c: get buffer off 0xff6(4086), len 0xa
  util.c: get buffer off 0x0(0), len 0x42c
  volume_id.c: signature 'swap' detected
  volume_id.c: conflicting signatures found, skip results
  swap+fat.img: unknown volume type

The currently released volume_id version says:
  $ extras/volume_id/vol_id ../udev/swap+fat.img
  ID_FS_USAGE=filesystem
  ID_FS_TYPE=vfat
  ID_FS_VERSIONúT16
  ID_FS_UUIDI1C-6648
  ID_FS_UUID_ENCI1C-6648
  ID_FS_LABEL=maybefat
  ID_FS_LABEL_ENC=maybefat
  ID_FS_LABEL_SAFE=maybefat

Here is the output of --probe-all
  $ extras/volume_id/vol_id --probe-all ../udev/swap+fat.img
  vfat
  swap

I hope that solves the problem properly.

Thanks,
Kay

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

[Karel Zak]

On Thu, Nov 13, 2008 at 08:01:34PM +0100, Kay Sievers wrote:
> I've committed  these changes to volume_id, and FAT32 accepts
> completely empty fsinfo signatures now.

 Thanks.

> We always probe for all filesystem types on volumes larger than a
> floppy disk. If we find multiple signatures and one of the detected
> filesystem types specifies that it can no co-exist with another known
> filesystem, like swap and FAT, we do not return a probing result.

 I see the patch (volume_id_probe_filesystem()) and a few things come
 to mind:

  - shouldn't be the relevant parts (label, uuid, version, ...) of
    the "struct volume_id" zeroized when you found a signature and
    before you call the next probing function?

  - it seems as overkill to use two for()s and probe two times for all
    filesystems. What about to store the first result and re-use it?

  - .. or at least never use the second for() when the fist for() found
    nothing ;-)

 For example see the patch below (it's incomplete, volume_id_cpy() and
 volume_id_remresult() are not implemented).

    Karel


diff --git a/extras/volume_id/lib/volume_id.c b/extras/volume_id/lib/volume_id.c
index aaa15e1..0f5cf2d 100644
--- a/extras/volume_id/lib/volume_id.c
+++ b/extras/volume_id/lib/volume_id.c
@@ -421,9 +421,12 @@ int volume_id_probe_filesystem(struct volume_id *id, uint64_t off, uint64_t size
 	 * smaller than a usual floppy disk.
 	 */
 	if (size > 1440 * 1024) {
+		struct volume_id first;
 		int found = 0;
 		int force_unique_result = 0;
 
+		memset(&first, 0, sizeof(first));
+
 		for (i = 0; i < ARRAY_SIZE(prober_filesystem); i++) {
 			int match;
 
@@ -437,20 +440,26 @@ int volume_id_probe_filesystem(struct volume_id *id, uint64_t off, uint64_t size
 					return -1;
 				}
 				found = 1;
+				volume_id_cpy(&first, id);
+				volume_id_remresult(id);
 			}
 		}
+		if (found) {
+			volume_id_cpy(id, &first);
+			goto found;
+		}
+		return -1;
 	}
 
 	/* return the first match */
 	for (i = 0; i < ARRAY_SIZE(prober_filesystem); i++) {
-		if (prober_filesystem[i].prober(id, off, size) = 0) {
-			info("signature '%s' detected\n", id->type);
+		if (prober_filesystem[i].prober(id, off, size) = 0)
 			goto found;
-		}
 	}
 	return -1;
 found:
 	/* If recognized, we free the allocated buffers */
+	info("signature '%s' detected\n", id->type);
 	volume_id_free_buffer(id);
 	return 0;
 }

-- 
 Karel Zak  <kzak@redhat.com>

Re: LifeDrive filsystem probe fails [mjg59@srcf.ucam.org: Re: LifeDrive and T|X not seen by hal]

[Kay Sievers]

On Thu, Nov 20, 2008 at 14:17, Karel Zak <kzak@redhat.com> wrote:
> On Thu, Nov 13, 2008 at 08:01:34PM +0100, Kay Sievers wrote:
>> We always probe for all filesystem types on volumes larger than a
>> floppy disk. If we find multiple signatures and one of the detected
>> filesystem types specifies that it can no co-exist with another known
>> filesystem, like swap and FAT, we do not return a probing result.
>
>  I see the patch (volume_id_probe_filesystem()) and a few things come
>  to mind:
>
>  - shouldn't be the relevant parts (label, uuid, version, ...) of
>    the "struct volume_id" zeroized when you found a signature and
>    before you call the next probing function?

Sounds good, yes.

>  - it seems as overkill to use two for()s and probe two times for all
>    filesystems. What about to store the first result and re-use it?

We can do that, sure.

One thing to keep in mind for your new lib is to allocate all the
results individually and return them all to the caller, I guess. So
cleaning or copying results like here would not be needed, and the
caller can decide what to do with conflicting results.

>  - .. or at least never use the second for() when the fist for() found
>    nothing ;-)

Good point. :)

>  For example see the patch below (it's incomplete, volume_id_cpy() and
>  volume_id_remresult() are not implemented).
>
> --- a/extras/volume_id/lib/volume_id.c
> +++ b/extras/volume_id/lib/volume_id.c
> @@ -421,9 +421,12 @@ int volume_id_probe_filesystem(struct volume_id *id, uint64_t off, uint64_t size
>         * smaller than a usual floppy disk.
>         */
>        if (size > 1440 * 1024) {
> +               struct volume_id first;
>                int found = 0;
>                int force_unique_result = 0;
>
> +               memset(&first, 0, sizeof(first));
> +
>                for (i = 0; i < ARRAY_SIZE(prober_filesystem); i++) {
>                        int match;
>
> @@ -437,20 +440,26 @@ int volume_id_probe_filesystem(struct volume_id *id, uint64_t off, uint64_t size
>                                        return -1;
>                                }
>                                found = 1;
> +                               volume_id_cpy(&first, id);
> +                               volume_id_remresult(id);

Wouldn't that copy, and later return the last result?

Thanks,
Kay

Re: LifeDrive filsystem probe fails [mjg59@srcf.ucam.org: Re: LifeDrive and T|X not seen by hal]

[Kay Sievers]

On Thu, Nov 20, 2008 at 15:50, Kay Sievers <kay.sievers@vrfy.org> wrote:
> On Thu, Nov 20, 2008 at 14:17, Karel Zak <kzak@redhat.com> wrote:
>> On Thu, Nov 13, 2008 at 08:01:34PM +0100, Kay Sievers wrote:
>>> We always probe for all filesystem types on volumes larger than a
>>> floppy disk. If we find multiple signatures and one of the detected
>>> filesystem types specifies that it can no co-exist with another known
>>> filesystem, like swap and FAT, we do not return a probing result.
>>
>>  I see the patch (volume_id_probe_filesystem()) and a few things come
>>  to mind:
>>
>>  - shouldn't be the relevant parts (label, uuid, version, ...) of
>>    the "struct volume_id" zeroized when you found a signature and
>>    before you call the next probing function?
>
> Sounds good, yes.
>
>>  - it seems as overkill to use two for()s and probe two times for all
>>    filesystems. What about to store the first result and re-use it?
>
> We can do that, sure.
>
> One thing to keep in mind for your new lib is to allocate all the
> results individually and return them all to the caller, I guess. So
> cleaning or copying results like here would not be needed, and the
> caller can decide what to do with conflicting results.
>
>>  - .. or at least never use the second for() when the fist for() found
>>    nothing ;-)
>
> Good point. :)

I changed it. In case of multiple matches, it re-probes only the first
match again, all other cases are not probed twice.

I hope we can switch over to your new lib soon, so we don't do the
same stuff twice all the time. :)

Thanks,
Kay