Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

mkfs.vfat 4.2 on a whole block device adds bogus partition table #161

Closed
rwmjones opened this issue Feb 23, 2021 · 9 comments
Closed

mkfs.vfat 4.2 on a whole block device adds bogus partition table #161

rwmjones opened this issue Feb 23, 2021 · 9 comments

Comments

@rwmjones
Copy link

https://bugzilla.redhat.com/show_bug.cgi?id=1931866

You can no longer use this command on a whole device to create a filesystem:

mkfs -t vfat -I -n TEST /dev/sda

Previously this would do the obvious. Now it creates a bogus partition table which has a single partition that overlays the whole device:

# fdisk -l /dev/sda
Disk /dev/sda: 10 MiB, 10485760 bytes, 20480 sectors
Disk model: QEMU HARDDISK   
Units: sectors of 1 * 512 = 512 bytes
Sector size (logical/physical): 512 bytes / 512 bytes
I/O size (minimum/optimal): 512 bytes / 512 bytes
Disklabel type: dos
Disk identifier: 0x4fe3e4e4

Device     Boot Start   End Sectors Size Id Type
/dev/sda1  *        0 20479   20480  10M  4 FAT16 <32M

(Notice the partition start/end)

Diffing the filesystem that is created before and after shows the new partition table entry at 0x1be-0x1cd which was all zeroes before:

--- test1.img.hex       2021-02-23 12:29:49.815043947 +0000
+++ test1-bad.img.hex   2021-02-23 12:30:00.197159306 +0000
@@ -1,6 +1,6 @@
 00000000  eb 3c 90 6d 6b 66 73 2e  66 61 74 00 02 04 04 00  |.<.mkfs.fat.....|
 00000010  02 00 02 00 50 f8 14 00  14 00 01 00 00 00 00 00  |....P...........|
-00000020  00 00 00 00 80 00 29 c1  0d 23 51 54 45 53 54 20  |......)..#QTEST |
+00000020  00 00 00 00 80 00 29 65  e1 e3 4f 54 45 53 54 20  |......)e..OTEST |
 00000030  20 20 20 20 20 20 46 41  54 31 36 20 20 20 0e 1f  |      FAT16   ..|
 00000040  be 5b 7c ac 22 c0 74 0b  56 b4 0e bb 07 00 cd 10  |.[|.".t.V.......|
 00000050  5e eb f0 32 e4 cd 16 cd  19 eb fe 54 68 69 73 20  |^..2.......This |
@@ -12,6 +12,10 @@
 000000b0  72 79 20 61 67 61 69 6e  20 2e 2e 2e 20 0d 0a 00  |ry again ... ...|
 000000c0  00 00 00 00 00 00 00 00  00 00 00 00 00 00 00 00  |................|
 *
+000001b0  00 00 00 00 00 00 00 00  e4 e4 e3 4f 00 00 80 00  |...........O....|
+000001c0  01 00 04 fe ff ff 00 00  00 00 00 50 00 00 00 00  |...........P....|
+000001d0  00 00 00 00 00 00 00 00  00 00 00 00 00 00 00 00  |................|
+*
 000001f0  00 00 00 00 00 00 00 00  00 00 00 00 00 00 55 aa  |..............U.|
 00000200  00 00 00 00 00 00 00 00  00 00 00 00 00 00 00 00  |................|
 *
@@ -21,8 +25,8 @@
 00003000  f8 ff ff ff 00 00 00 00  00 00 00 00 00 00 00 00  |................|
 00003010  00 00 00 00 00 00 00 00  00 00 00 00 00 00 00 00  |................|
 *
-00005800  54 45 53 54 20 20 20 20  20 20 20 08 00 00 a3 63  |TEST       ....c|
-00005810  57 52 57 52 00 00 a3 63  57 52 00 00 00 00 00 00  |WRWR...cWR......|
+00005800  54 45 53 54 20 20 20 20  20 20 20 08 00 00 97 63  |TEST       ....c|
+00005810  57 52 57 52 00 00 97 63  57 52 00 00 00 00 00 00  |WRWR...cWR......|
 00005820  00 00 00 00 00 00 00 00  00 00 00 00 00 00 00 00  |................|
 *
 00a00000

Curiously it still works fine if you run mkfs on a local file.

This is a regression somewhere between versions 4.1 and 4.2.

@rwmjones
Copy link
Author

I bisected this to commit 5e936c9. The change seems quite complicated. I wasn't able to revert it to be completely sure that the bisection is correct since the code in src/device_info.c has changed a lot.

@rwmjones
Copy link
Author

A simple and safe reproducer is:

# modprobe nbd
# nbdkit memory 10M
# nbd-client -b 512 localhost /dev/nbd0
# mkfs.vfat -I -n TEST /dev/nbd0
# hexdump -C /dev/nbd0

Check if the hexdump output contains the bogus PTE at 0x1b0-0x1c0.

@pali
Copy link
Member

pali commented Feb 23, 2021

And what is the issue? Are you unable to mount fat formatted /dev/sda disk? I have done lot of tests and mounting /dev/sda is working fine. If you do not have loaded vfat.ko it is needed either to run mount with -t vfat option or call modprobe vfat.

Also partition table on /dev/sda is valid and you should be able to mount /dev/sda1 without any special option.

@rwmjones
Copy link
Author

rwmjones commented Feb 23, 2021

The issue is the partiition table is wrong, it's a partition table containing a partition which references itself. This causes trouble with some tools we use for creating filesystems which don't expect incorrect PTs. The second issue is it's inconsistent - creating a filesystem in a file does the right thing.

@rwmjones
Copy link
Author

My colleague noticed the --mbr=no option (added in commit 5199d68) which suppresses the broken PT.

@pali
Copy link
Member

pali commented Feb 23, 2021

The issue is the partiition table is wrong, it's a partition table containing a partition which references itself.

Well, I do not see nothing wrong with it. Kernel correctly handles it and also other standard tools.

This was added in 4.2 for compatibility with Windows kernel as it cannot access FAT filesystem on non-removable disk without MBR/GPT partition table.

Same behavior is already implemented in mformat utility (alternative FAT format utility) from Linux mtools project for a long time. And also in mkudffs (UDF format tool) from udftools project.

This causes trouble with some tools we use for create filesystems which don't expect incorrect PTs.

So I think these tools needs to be fixed as they are incompatible also with mtools and udftools.

Anyway, this behavior is documented in mkfs.fat manual page and can be changed by --mbr option.

The second issue is it's inconsistent - creating a filesystem in a file does the right thing.

This is also documented. That MBR table is created only on non-removable disk block devices and via --mbr option you can change it. It is because when formatting filesystem in a file, it is not known if you are going to copy this image into partition or to a whole disk.

Also this change is mentioned in 4.2 release notes.

If you think that documentation in manpage is not enough or can be extended or better explained, please let me know.

@pali
Copy link
Member

pali commented Feb 25, 2021

So, are you unable to mount fat formatted /dev/sda disk or /dev/sda1 device? Or is not kernel correctly parsing this partition table? Or is documentation incomplete and needs to be extended? Please provide more information as I do not see nothing bogus or broken on this partition table. Existing tools like fdisk can correctly handle / read it. Also same behavior has for a long time mformat and is present already in mkudffs.

@rwmjones
Copy link
Author

We're going to use --mbr=no to suppress the behaviour.

@lersek
Copy link

lersek commented Nov 23, 2021

In response to #161 (comment) @pali

The issue is the partiition table is wrong, it's a partition table containing a partition which references itself.

Well, I do not see nothing wrong with it. Kernel correctly handles it and also other standard tools.

parted is arguably a standard tool, and it does not handle the bogus partition table created by recent mkfs.fat.

# parted -m -s -- /dev/loop0 unit b print
Error: Invalid partition table - recursive partition on /dev/loop0.
BYT;
/dev/loop0:1048576B:loopback:512:512:unknown:Loopback device:;

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants