fs: FAT: Add support for DOS 1.x formatted volumes

PATCH Superseded Conrad Meyer 2 participants 4 messages Mar 31, 2014 – Apr 13, 2014 VFAT/FAT/msdos filesystem

Share & tools

Conrad Meyer

author

2014-03-31 15:22 UTC #0 lore raw

PATCH v3 1/1 fs: FAT: Add support for DOS 1.x formatted volumes

Add dos1xfloppy mount option to infer DOS 2.x BIOS Parameter Block defaults from block device geometry for ancient floppies and floppy images.

Validate that the entire BPB is zero like we expect, and that the floppy has a DOS-style 8086 code bootstrapping header.

Fixes kernel.org bug #42617.

Values are inferred from media size and a table.[0] Media size is assumed to be static for archaic FAT volumes. See also [1].

[0]: https://en.wikipedia.org/wiki/File_Allocation_Table#Exceptions
[1]: http://www.win.tue.nl/~aeb/linux/fs/fat/fat-1.html

Signed-off-by Conrad Meyer <[email protected]>

---
Based on next-20140328.

Changes since v2:
* Only attempt to infer BPB values if mounted with -o dos1xfloppy
* Set sbi->* values directly instead of writing out BPB

Works fine in local tests. Can mount 1985 floppy image and explore files (with dos1xfloppy option). Without option, mount fails (expected).

Thanks.

---
 fs/fat/fat.h   |   3 +-
 fs/fat/inode.c | 154 ++++++++++++++++++++++++++++++++++++++++++++++++++-------
 2 files changed, 137 insertions(+), 20 deletions(-)

fs/fat/fat.h +2 -1

index 7270bdb..13b7202 100644
@@ -52,7 +52,8 @@ struct fat_mount_options {
 		 usefree:1,	   /* Use free_clusters for FAT32 */
 		 tz_set:1,	   /* Filesystem timestamps' offset set */
 		 rodir:1,	   /* allow ATTR_RO for directory */
-		 discard:1;	   /* Issue discard requests on deletions */
+		 discard:1,	   /* Issue discard requests on deletions */
+		 dos1xfloppy:1;	   /* Assume default BPB for DOS 1.x floppies */
 };
 
 #define FAT_HASH_BITS	8

fs/fat/inode.c +135 ~~-19~~

237 lines not on this page · raw message

Signature · 1 line

1.9.0

Conrad Meyer

author

2014-04-01 01:17 UTC #1 lore raw

PATCH v4 1/1 fs: FAT: Add support for DOS 1.x formatted volumes

Add dos1xfloppy mount option to infer DOS 2.x BIOS Parameter Block defaults from block device geometry for ancient floppies and floppy images.

Validate that the entire BPB is zero like we expect, and that the floppy has a DOS-style 8086 code bootstrapping header.

Fixes kernel.org bug #42617.

Values are inferred from media size and a table.[0] Media size is assumed to be static for archaic FAT volumes. See also [1].

[0]: https://en.wikipedia.org/wiki/File_Allocation_Table#Exceptions
[1]: http://www.win.tue.nl/~aeb/linux/fs/fat/fat-1.html

Signed-off-by Conrad Meyer <[email protected]>

---
Changes since v3!

* No EOF on floppy_defaults array (use ARRAY_SIZE instead)
* Use correct format string/types for sector_t in fat_msg()
* Pull out massive if () zero boolean expression into a hopefully expressively named helper function, fat_is_bpb_zero()

Thanks for the feedback.

---
 fs/fat/fat.h   |   3 +-
 fs/fat/inode.c | 175 ++++++++++++++++++++++++++++++++++++++++++++++++++-------
 2 files changed, 157 insertions(+), 21 deletions(-)

fs/fat/fat.h +2 -1

index 7270bdb..13b7202 100644
@@ -52,7 +52,8 @@ struct fat_mount_options {
 		 usefree:1,	   /* Use free_clusters for FAT32 */
 		 tz_set:1,	   /* Filesystem timestamps' offset set */
 		 rodir:1,	   /* allow ATTR_RO for directory */
-		 discard:1;	   /* Issue discard requests on deletions */
+		 discard:1,	   /* Issue discard requests on deletions */
+		 dos1xfloppy:1;	   /* Assume default BPB for DOS 1.x floppies */
 };
 
 #define FAT_HASH_BITS	8

fs/fat/inode.c +155 ~~-20~~

265 lines not on this page · raw message

Signature · 1 line

1.9.0

OGAWA Hirofumi

maintainer

2014-04-13 03:57 UTC #2 lore raw

Conrad Meyer <[email protected]> writes:

Hi,

Sorry for late reply.

fs/fat/inode.c int fat_fill_super(struct super_block *sb, void *data, int silent, int isvfat, · line 1452

Quoted 2 lines

fs/fat/inode.c +2 -0

+
+	bd_sects = part_nr_sects_read(sb->s_bdev->bd_part);

This would be better to use i_read_size(&sb->s_bdev->bd_inode->i_size)

fs/fat/inode.c int fat_fill_super(struct super_block *sb, void *data, int silent, int isvfat, · line 1471

Quoted 6 lines

fs/fat/inode.c +6 -0

+		if (!fat_bpb_is_zero(b)) {
+			fat_msg(sb, KERN_ERR, "%s; DOS 2.x BPB is non-zero",
+				notdos1x);
+			brelse(bh);
+			goto out_invalid;
+		}

Please don't message if silent. While auto detection, user don't want to see message from kernel.

fs/fat/inode.c int fat_fill_super(struct super_block *sb, void *data, int silent, int isvfat, · line 1460

Quoted 8 lines

fs/fat/inode.c +8 -0

+	if (sbi->options.dos1xfloppy) {
+		/* 16-bit DOS 1.x reliably wrote bootstrap short-jmp code */
+		if (b->ignored[0] != 0xeb || b->ignored[2] != 0x90) {
+			fat_msg(sb, KERN_ERR, "%s; no bootstrapping code",
+				notdos1x);
+			brelse(bh);
+			goto out_invalid;
+		}

[...]

Looks like I was not explain my suggestion correctly. I was intended to allow dos1xfloppy by option, not allow dos1xfloppy only.

I.e.

	err = read-from-bpb();
	if (err == -ENOTFATFS)
		if (dos1xfloppy)
			err = read-from-static-bpb();
	if (err)
		goto error;

fs/fat/inode.c int fat_fill_super(struct super_block *sb, void *data, int silent, int isvfat, · line 1667

Quoted 6 lines

fs/fat/inode.c +4 -0

+	if (sbi->options.dos1xfloppy)
+		total_sectors = fdefaults->nr_sectors;
+	else
+		total_sectors = get_unaligned_le16(&b->sectors);
 	if (total_sectors == 0)
 		total_sectors = le32_to_cpu(b->total_sect);

Can we make 2 helpers to read from BPB and static BPB? If possible, we would not be bothered by if (dos1xfloppy), like above crappy pseudo.

Thanks.

Signature · 1 line

OGAWA Hirofumi <[email protected]>

Quoted 114 lines

fs/fat/inode.c +62 -7

+
+		for (i = 0; i < ARRAY_SIZE(floppy_defaults); i++) {
+			if (floppy_defaults[i].nr_sectors == bd_sects) {
+				fdefaults = &floppy_defaults[i];
+				break;
+			}
+		}
+
+		if (fdefaults == NULL) {
+			fat_msg(sb, KERN_WARNING,
+				"This looks like a DOS 1.x volume, but isn't a recognized floppy size (%llu sectors)",
+				(u64)bd_sects);
+			brelse(bh);
+			goto out_invalid;
+		}
+
+		fat_msg(sb, KERN_INFO,
+			"This looks like a DOS 1.x volume; assuming default BPB values");
+	} else {
+		if (!b->reserved) {
+			if (!silent)
+				fat_msg(sb, KERN_ERR,
+					"bogus number of reserved sectors");
+			brelse(bh);
+			goto out_invalid;
+		}
+		if (!b->fats) {
+			if (!silent)
+				fat_msg(sb, KERN_ERR,
+					"bogus number of FAT structure");
+			brelse(bh);
+			goto out_invalid;
+		}
 	}
 
 	/*

@@ -1397,6 +1510,8 @@ int fat_fill_super(struct super_block *sb, void *data, int silent, int isvfat,
 	 */
 
 	media = b->media;
+	if (sbi->options.dos1xfloppy)
+		media = fdefaults->media;
 	if (!fat_valid_media(media)) {
 		if (!silent)
 			fat_msg(sb, KERN_ERR, "invalid media value (0x%02x)",

@@ -1404,7 +1519,10 @@ int fat_fill_super(struct super_block *sb, void *data, int silent, int isvfat,
 		brelse(bh);
 		goto out_invalid;
 	}
-	logical_sector_size = get_unaligned_le16(&b->sector_size);
+	if (sbi->options.dos1xfloppy)
+		logical_sector_size = SECTOR_SIZE;
+	else
+		logical_sector_size = get_unaligned_le16(&b->sector_size);
 	if (!is_power_of_2(logical_sector_size)
 	    || (logical_sector_size < 512)
 	    || (logical_sector_size > 4096)) {

@@ -1414,7 +1532,10 @@ int fat_fill_super(struct super_block *sb, void *data, int silent, int isvfat,
 		brelse(bh);
 		goto out_invalid;
 	}
-	sbi->sec_per_clus = b->sec_per_clus;
+	if (sbi->options.dos1xfloppy)
+		sbi->sec_per_clus = fdefaults->sec_per_clus;
+	else
+		sbi->sec_per_clus = b->sec_per_clus;
 	if (!is_power_of_2(sbi->sec_per_clus)) {
 		if (!silent)
 			fat_msg(sb, KERN_ERR, "bogus sectors per cluster %u",

@@ -1450,10 +1571,18 @@ int fat_fill_super(struct super_block *sb, void *data, int silent, int isvfat,
 	mutex_init(&sbi->s_lock);
 	sbi->cluster_size = sb->s_blocksize * sbi->sec_per_clus;
 	sbi->cluster_bits = ffs(sbi->cluster_size) - 1;
-	sbi->fats = b->fats;
+	if (sbi->options.dos1xfloppy)
+		sbi->fats = 2;
+	else
+		sbi->fats = b->fats;
 	sbi->fat_bits = 0;		/* Don't know yet */
-	sbi->fat_start = le16_to_cpu(b->reserved);
-	sbi->fat_length = le16_to_cpu(b->fat_length);
+	if (sbi->options.dos1xfloppy) {
+		sbi->fat_start = 1;
+		sbi->fat_length = fdefaults->fat_length;
+	} else {
+		sbi->fat_start = le16_to_cpu(b->reserved);
+		sbi->fat_length = le16_to_cpu(b->fat_length);
+	}
 	sbi->root_cluster = 0;
 	sbi->free_clusters = -1;	/* Don't know yet */
 	sbi->free_clus_valid = 0;

@@ -1515,7 +1644,10 @@ int fat_fill_super(struct super_block *sb, void *data, int silent, int isvfat,
 	sbi->dir_per_block_bits = ffs(sbi->dir_per_block) - 1;
 
 	sbi->dir_start = sbi->fat_start + sbi->fats * sbi->fat_length;
-	sbi->dir_entries = get_unaligned_le16(&b->dir_entries);
+	if (sbi->options.dos1xfloppy)
+		sbi->dir_entries = fdefaults->dir_entries;
+	else
+		sbi->dir_entries = get_unaligned_le16(&b->dir_entries);
 	if (sbi->dir_entries & (sbi->dir_per_block - 1)) {
 		if (!silent)
 			fat_msg(sb, KERN_ERR, "bogus directory-entries per block"

@@ -1527,7 +1659,10 @@ int fat_fill_super(struct super_block *sb, void *data, int silent, int isvfat,
 	rootdir_sectors = sbi->dir_entries
 		* sizeof(struct msdos_dir_entry) / sb->s_blocksize;
 	sbi->data_start = sbi->dir_start + rootdir_sectors;
-	total_sectors = get_unaligned_le16(&b->sectors);
+	if (sbi->options.dos1xfloppy)
+		total_sectors = fdefaults->nr_sectors;
+	else
+		total_sectors = get_unaligned_le16(&b->sectors);
 	if (total_sectors == 0)
 		total_sectors = le32_to_cpu(b->total_sect);

Signature · 1 line

OGAWA Hirofumi <[email protected]>

Conrad Meyer

author

2014-04-13 12:35 UTC #3 lore raw

Quoted 4 lines · On Sun, 13 Apr 2014 12:57:42 +0900 OGAWA Hirofumi…

Conrad Meyer <[email protected]> writes:
Hi,

Quoted 8 lines

Sorry for late reply.
Quoted 3 lines
(quoted diff) +2 -0
+
+	bd_sects =
part_nr_sects_read(sb->s_bdev->bd_part);
This would be better to use
i_read_size(&sb->s_bdev->bd_inode->i_size)

Ok, easy to fix.

Quoted 11 lines

fs/fat/inode.c int fat_fill_super(struct super_block *sb, void *data, int silent, int isvfat, · line 1460
Quoted 8 lines
(quoted diff) +2 -0
+		if (!fat_bpb_is_zero(b)) {
+			fat_msg(sb, KERN_ERR, "%s; DOS
2.x BPB is non-zero",
fs/fat/inode.c +4 -0
+				notdos1x);
+			brelse(bh);
+			goto out_invalid;
+		}
Please don't message if silent. While auto detection, user don't want to see message from kernel.

Okay, fixed.

Quoted 40 lines

fs/fat/inode.c int fat_fill_super(struct super_block *sb, void *data, int silent, int isvfat, · line 1460
Quoted 12 lines
(quoted diff) +2 -0
+	if (sbi->options.dos1xfloppy) {
+		/* 16-bit DOS 1.x reliably wrote
bootstrap short-jmp code */
(quoted diff) +1 -0
+		if (b->ignored[0] != 0xeb ||
b->ignored[2] != 0x90) {
(quoted diff) +1 -0
+			fat_msg(sb, KERN_ERR, "%s; no
bootstrapping code",
fs/fat/inode.c +4 -0
+				notdos1x);
+			brelse(bh);
+			goto out_invalid;
+		}
[...]
Looks like I was not explain my suggestion correctly. I was intended to allow dos1xfloppy by option, not allow dos1xfloppy only.
I.e.
	err = read-from-bpb();
	if (err == -ENOTFATFS)
		if (dos1xfloppy)
			err = read-from-static-bpb();
	if (err)
		goto error;
Quoted 7 lines
fs/fat/inode.c +4 -0
+	if (sbi->options.dos1xfloppy)
+		total_sectors = fdefaults->nr_sectors;
+	else
+		total_sectors =
get_unaligned_le16(&b->sectors); if (total_sectors == 0)
total_sectors =
le32_to_cpu(b->total_sect);
Can we make 2 helpers to read from BPB and static BPB? If possible, we would not be bothered by if (dos1xfloppy), like above crappy pseudo.

Quoted 1 line

Thanks.

We would have to duplicate, replace, or skip large parts of fat_fill_super(). This is why my initial approach was to just fill in BPB values in the SB copy in memory. We read from BPB over the course of 215 lines of code, ending here:

Quoted 11 lines

(quoted diff) +0 -0
@@ -1527,7 +1659,10 @@ int fat_fill_super(struct
 super_block *sb, void *data, int silent, int isvfat,
rootdir_sectors = sbi->dir_entries
* sizeof(struct msdos_dir_entry) /
sb->s_blocksize; sbi->data_start = sbi->dir_start + rootdir_sectors;
fs/fat/inode.c +4 -1
-	total_sectors = get_unaligned_le16(&b->sectors);
+	if (sbi->options.dos1xfloppy)
+		total_sectors = fdefaults->nr_sectors;
+	else
+		total_sectors =

Quoted 3 lines

get_unaligned_le16(&b->sectors); if (total_sectors == 0)
total_sectors =
le32_to_cpu(b->total_sect);

I will make an attempt at it.

Thanks,
Conrad