2.5.5-pre1: mounting NTFS partitions -t VFAT

Hi,

Due to a recent change of filesystems, I found the following: 2.5.5-pre1 mounts my NTFS (win2k) partition as VFAT partition, if told to do so. The kernel returns errors, but the mount is there. One write to the partition was enough to destroy the entire NTFS partition.

Due to filesystem damage, I didn't test the behaviour of the VFAT driver on other filesystems yet.

Kernel 2.4.17 also returns errors, but there the mount fails.

Will try to debug the problem myself this afternoon. Sounds like the VFAT procedure ignores some errors.

Jos

At a guess (V)FAT is not checking the OEM identifier in the bootsector which for NTFS is defined as "NTFS " (i.e. NTFS followed by four ASCII spaces, 0x20 char code. The OEMid is at offset 3 (bytes) into the bootsector.

If you just add a check to (v)fat boot sector sanity verification (it does have one right? if not it REALLY ought to have one...) and make sure the OEMid is not the ntfs one the fat driver will never touch the ntfs partition. Even better if it would check for the correct FAT oem id but I think there may be several different ones so it may be easier to just make sure it is not NTFS.

Best regards,

Anton

Sorry, my fault.

The following patch should fix this bug. I'll submit it after test.

@@ -624,6 +624,18 @@
 	}
 
 	b = (struct fat_boot_sector *) bh->b_data;
+	if (!b->fats) {
+		if (!silent)
+			printk("FAT: bogus number of FAT structure\n");
+		brelse(bh);
+		goto out_invalid;
+	}
+	if (!b->reserved) {
+		if (!silent)
+			printk("FAT: bogus number of reserved sectors\n");
+		brelse(bh);
+		goto out_invalid;
+	}
 	if (!b->secs_track) {
 		if (!silent)
 			printk("FAT: bogus sectors-per-track value\n");

OGAWA Hirofumi <[email protected]>

Hi Ogawa,

Your patch seems to fix it more or less, not the way it should be fixed, imho. Partitions other than FAT return bogus information, but bogus is not always zero. Fortunately enough, one of those new if statements returns an error, but this is a "works for me" solution, not a decent one.

What lacks is a fingerprint detector, and iirc -long time ago- FAT has a very easy to detect fingerprint.

I'll dig into FAT documentation tonight.

Jos

No, that patch are validity check for FAT, not for you.

I read the document repeatedly and did much tests. If you read the document, you may use BS_OEMName or BS_FilSysType, however, these don't have a meaning.

OGAWA Hirofumi <[email protected]>

Hmmm. You seem to be right there. In my OS (IBM PC only) I checked the partition table (see below).

The first question I want answered: Should I just call myself stupid for trying to mount NTFS as VFAT, or should we consider this a real issue that needs fixing ? (I see the problem as a generic problem. There must be other combinations of filesystems and partition types that pass the test, but are wrong). IMHO the latter, for every lost partition makes an angry linux user.

Anyway. I have already been thinking further. Maybe I'm talking nonsense, but I'll give it a try.

The type of a partition is written in the partition table, or something similar. Maybe we should check that ?

While mounting a partition, the vfs layer tries to determine the partition type, and passes that info to the filesystem driver, which checks whether that partition type can be mounted by the driver. If no partition type is provided by the vfs layer (for the partition type is not available in the used partition table, or whatever), the fs driver must try to find out itself.

If you don't want the calls to change, another option is to move the check to the vfs. Problem there however is that the vfs needs a "filesystem <-> partition type" table. On the other hand, this way we can add extra security without breaking anything.

Jos

Partition type isn't available to fs driver. Think about mounting floppy/loopback/etc.

Seems you guys are discussing non-problem here. What really needs to be done is to add more sanity checks to FAT superblock detection/validation code:
* signatures like 55AA at end of 1st sector
* sane values for various superblock data (if you see "FAT copies: 146" it is more than enough to tell it's not a FAT, right?)

If anyone feels so inclined, please go to fs/fat/inode.c:fat_read_super() and hack on it. Send your patches to Alexander Viro <[email protected]> and tighten your seatbelt ;-)

--
vda

True. I never use floppys anymore :)

Will hack tonight and see if my seatbelt is strong enough :) It has never been used before, so maybe...

Jos

Hi Jos,

Stupid? No.
Fixing? Yes.

Whatever you throw at mount, you want it to fail_safe_. i.e. in the worst case, do nothing.

Without any help, mount (userspace) tries to determine the partition type. It understands the magics of a LOT of filesystems.

It looks for the NTFS magic before the DOS magic (or any of its variants).

Which is passed to the VFS, and the to the driver which performs some more rigid tests (hopefully :-)

Cheers,
FlatCap (Rich)
[email protected]

Agreed.

Well, the story: I reinstalled Win2k and updated from FAT32 to NTFS, the day before I tested 2.5.5pre-1. I forgot to update /etc/fstab. So mount was called with the -t vfat option.

Working on those tests, though it is hard to tell what limits are valid. I never saw more than 2 FATs, but can I assume there never will be more ? FAT has no real fingerprint, unfortunately. There are some bytes that can be checked, but they depend very much on the FAT version. FAT v 4.0 and higher have the fingerprint 'FAT' somewhere in the bootsector, but this doesn't hold anymore for some Win98/ME formatted partitions.

A test I'm thinking about is trying to match the FATs if there is more than one. You want to bail out if the FATs aren't the same anyway. The chance that the FAT-sectors are the same on non-FAT partitions is very small. Though this test can be rather slow, maybe we should only test the first sector(s).

Jos

It doesn't and it shouldn't. mount(8) does, so there's no reason to do that in kerenl.

Hi,

2.5.6 comes with the next error:

Failed to initialize the ICMP6 control socket (err -97).

I have not found any kernel settings that affect this error, it happens both with IPv6 compiled in kernel and loaded as module.

Jos

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

There was a similar problem with the -dj tree a while ago - I think the solution is probably the same.

Search the archives for the thread titled:

Subject: Linux 2.5.5-dj1 - IPv6 not loading correctly.

- --
Ben Clifford [email protected] GPG: 30F06950
http://www.hawaga.org.uk/ben/

-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.0.6 (GNU/Linux)
Comment: For info see http://www.gnupg.org

iD8DBQE8ina8sYXoezDwaVARAl5lAJ4wj6izPK+UhM12uz2axTxLbbVcrACfTEpQ WdCoZrOWKvWDHWqNI06w+sE=
=dmRK
-----END PGP SIGNATURE-----

This will be fixed shortly.