Search the usage output of "mkfs.fat" for
"--mbr[="; cache the result for
further invocations. If the option is supported, pass "--mbr=n" to
"mkfs.fat". This will prevent the creation of a bogus partition table
whose first (and only) entry describes a partition that contains the
partition table.
(Such a bogus partition table breaks "parted", which is a tool used by
libguestfs extensively, both internally and in public libguestfs APIs.)
Bugzilla:
https://bugzilla.redhat.com/show_bug.cgi?id=1931821
Signed-off-by: Laszlo Ersek <lersek(a)redhat.com>
---
Notes:
Testing (per <
https://bugzilla.redhat.com/show_bug.cgi?id=1931821#c0>):
$ virt-make-fs -vx --format=raw --size=200M --type=vfat --label=TEST \
website ~/test_vfat.img
> [...]
> creating vfat filesystem on /dev/sda ...
> libguestfs: trace: mkfs "vfat" "/dev/sda"
"label:TEST"
> [...]
> commandrvf: stdout=n stderr=y flags=0x0
> commandrvf: mkfs.fat
> Usage: mkfs.fat [OPTIONS] TARGET [BLOCKS]
> Create FAT filesystem in TARGET, which can be a block device or file. Use only
> up to BLOCKS 1024 byte blocks if specified. With the -C option, file TARGET will
be
> created with a size of 1024 bytes times BLOCKS, which must be specified.
>
> Options:
> -a Disable alignment of data structures
> -A Toggle Atari variant of the filesystem
> -b SECTOR Select SECTOR as location of the FAT32 backup boot sector
> -c Check device for bad blocks before creating the filesystem
> -C Create file TARGET then create filesystem in it
> -D NUMBER Write BIOS drive number NUMBER to boot sector
> -f COUNT Create COUNT file allocation tables
> -F SIZE Select FAT size SIZE (12, 16 or 32)
> -g GEOM Select disk geometry: heads/sectors_per_track
> -h NUMBER Write hidden sectors NUMBER to boot sector
> -i VOLID Set volume ID to VOLID (a 32 bit hexadecimal number)
> -I Ignore and disable safety checks
> -l FILENAME Read bad blocks list from FILENAME
> -m FILENAME Replace default error message in boot block with contents of
FILENAME
> -M TYPE Set media type in boot sector to TYPE
> --mbr[=y|n|a] Fill (fake) MBR table with one partition which spans whole
disk
> -n LABEL Set volume name to LABEL (up to 11 characters long)
> --codepage=N use DOS codepage N to encode label (default: 850)
> -r COUNT Make room for at least COUNT entries in the root directory
> -R COUNT Set minimal number of reserved sectors to COUNT
> -s COUNT Set number of sectors per cluster to COUNT
> -S SIZE Select a sector size of SIZE (a power of two, at least 512)
> -v Verbose execution
> --variant=TYPE Select variant TYPE of filesystem (standard or Atari)
>
> --invariant Use constants for randomly generated or time based values
> --offset=SECTOR Write the filesystem at a specific sector into the device
file.
> --help Show this help message and exit
> [...]
> commandrvf: stdout=n stderr=y flags=0x0
> commandrvf: mkfs -t vfat -I --mbr=n -n TEST /dev/sda
$ guestfish -a ~/test_vfat.img
> ><fs> run
> ><fs> list-filesystems
> /dev/sda: vfat
daemon/mkfs.c | 27 +++++++++++++++++++++++++++
1 file changed, 27 insertions(+)
diff --git a/daemon/mkfs.c b/daemon/mkfs.c
index ba90cef2111c..4cb216cabe8e 100644
--- a/daemon/mkfs.c
+++ b/daemon/mkfs.c
@@ -30,6 +30,14 @@
#define MAX_ARGS 64
+enum fat_mbr_option {
+ FMO_UNCHECKED,
+ FMO_DOESNT_EXIST,
+ FMO_EXISTS,
+};
+
+static enum fat_mbr_option fat_mbr_option = FMO_UNCHECKED;
+
/* Takes optional arguments, consult optargs_bitmask. */
int
do_mkfs (const char *fstype, const char *device, int blocksize,
@@ -101,6 +109,25 @@ do_mkfs (const char *fstype, const char *device, int blocksize,
STREQ (fstype, "msdos"))
ADD_ARG (argv, i, "-I");
+ /* Prevent mkfs.fat from creating a bogus partition table (RHBZ#1931821). */
+ if (STREQ (fstype, "fat") || STREQ (fstype, "vfat") ||
+ STREQ (fstype, "msdos")) {
+ if (fat_mbr_option == FMO_UNCHECKED) {
+ CLEANUP_FREE char *usage_err = NULL;
+
+ fat_mbr_option = FMO_DOESNT_EXIST;
+ /* Invoking either version 3 of version 4 of mkfs.fat without any options
+ * will make it (a) print a usage summary to stderr, (b) exit with status
+ * 1.
+ */
+ r = commandr (NULL, &usage_err, "mkfs.fat", (char *)NULL);
+ if (r == 1 && strstr (usage_err, "--mbr[=") != NULL)
+ fat_mbr_option = FMO_EXISTS;
+ }
+ if (fat_mbr_option == FMO_EXISTS)
+ ADD_ARG (argv, i, "--mbr=n");
+ }
+
/* Process blocksize parameter if set. */
if (optargs_bitmask & GUESTFS_MKFS_BLOCKSIZE_BITMASK) {
if (blocksize <= 0 || !is_power_of_2 (blocksize)) {
Yeah that looks like the right fix to me.
Acked-by: Richard W.M. Jones <rjones(a)redhat.com>
A side note, in case it wasn't obvious already: The daemon is single
threaded, so there's no need to lock access to that static variable.
We've discussed many times making the daemon be multi-threaded (eg. to
support a single library instance making several connections into the
daemon to support parallel operations) but it's such a substantial
change to the way that libguestfs works we've never done it, and
aren't likely to any time soon.
Rich.
--
Richard Jones, Virtualization Group, Red Hat
Fedora Windows cross-compiler. Compile Windows programs, test, and
build Windows installers. Over 100 libraries supported.