From 0efccea84456760fb34dad1d4c01ea1fb7832162 Mon Sep 17 00:00:00 2001 From: Laszlo Ersek Date: Thu, 25 Nov 2021 10:49:53 +0100 Subject: [PATCH] daemon/parted: work around part table type misreporting by "parted" "parted" incorrectly reports "loop" rather than "msdos" for the partition table type, when the (fake) partition table comes from the "--mbr" option of "mkfs.fat" (in dosfstools-4.2+), and the FAT variant in question is FAT16 or FAT32. (See RHBZ#2026224.) Work this around by - parsing the partition table ourselves, and - overriding "loop" with "msdos" when appropriate. Note that when the FAT variant is FAT12, "parted" fails to parse the fake MBR partition table completely (see RHBZ#2026220), which we cannot work around. However, FAT12 should be a rare corner case in libguestfs usage -- "mkfs.fat" auto-chooses FAT12 only below 9MB disk size, and even "-F 12" can only be forced up to and including 255MB disk size. Add the helper function "has_bogus_mbr" to the Utils module; we'll use it elsewhere too. Bugzilla: https://bugzilla.redhat.com/show_bug.cgi?id=1931821 Signed-off-by: Laszlo Ersek Acked-by: Richard W.M. Jones Message-Id: <20211125094954.9713-5-lersek@redhat.com> [lersek@redhat.com: drop "fun" keyword, and use partial application, in the definition of "sec0at" [Rich]] --- daemon/parted.ml | 7 ++++++- daemon/utils.ml | 45 +++++++++++++++++++++++++++++++++++++++++++++ daemon/utils.mli | 10 ++++++++++ 3 files changed, 61 insertions(+), 1 deletion(-) diff --git a/daemon/parted.ml b/daemon/parted.ml index 29b98c480..53077d91a 100644 --- a/daemon/parted.ml +++ b/daemon/parted.ml @@ -118,7 +118,12 @@ let part_get_parttype device = let fields = String.nsplit ":" device_line in match fields with | _::_::_::_::_::"loop"::_ -> (* If "loop" return an error (RHBZ#634246). *) - failwithf "%s: not a partitioned device" device + (* ... Unless parted failed to recognize the fake MBR that mkfs.fat from + * dosfstools-4.2+ created. In that case, return "msdos" for MBR + * (RHBZ#1931821). + *) + if Utils.has_bogus_mbr device then "msdos" + else failwithf "%s: not a partitioned device" device | _::_::_::_::_::ret::_ -> ret | _ -> failwithf "%s: cannot parse the output of parted" device diff --git a/daemon/utils.ml b/daemon/utils.ml index b56306055..adcda72aa 100644 --- a/daemon/utils.ml +++ b/daemon/utils.ml @@ -187,6 +187,51 @@ and compare_device_names a b = ) ) +let has_bogus_mbr device = + try + with_openfile device [O_RDONLY; O_CLOEXEC] 0 (fun fd -> + let sec0size = 0x200 + and sigofs = 0x1FE + and sysidofs = 0x003 and sysidsize = 0x008 + and pte1ofs = 0x1BE + and parttypes = [0x01; (* FAT12 *) + 0x04; (* FAT16 *) + 0x06; (* FAT12, FAT16, FAT16B *) + 0x0C; (* FAT32 LBA *) + 0x0E (* FAT16B LBA *)] in + let sec0 = Bytes.create sec0size in + let sec0read = read fd sec0 0 sec0size in + let sec0at = Bytes.get_uint8 sec0 in + + (* sector read completely *) + sec0read = sec0size && + + (* boot signature present *) + sec0at (sigofs ) = 0x55 && + sec0at (sigofs + 0x1) = 0xAA && + + (* mkfs.fat signature present *) + Bytes.sub_string sec0 sysidofs sysidsize = "mkfs.fat" && + + (* partition bootable *) + sec0at (pte1ofs ) = 0x80 && + + (* partition starts at C/H/S 0/0/1 *) + sec0at (pte1ofs + 0x1) = 0x00 && + sec0at (pte1ofs + 0x2) = 0x01 && + sec0at (pte1ofs + 0x3) = 0x00 && + + (* partition type is a FAT variant that mkfs.fat is known to create *) + List.mem (sec0at (pte1ofs + 0x4)) parttypes && + + (* partition starts at LBA 0 *) + sec0at (pte1ofs + 0x8) = 0x00 && + sec0at (pte1ofs + 0x9) = 0x00 && + sec0at (pte1ofs + 0xA) = 0x00 && + sec0at (pte1ofs + 0xB) = 0x00 + ) + with _ -> false + let proc_unmangle_path path = let n = String.length path in let b = Buffer.create n in diff --git a/daemon/utils.mli b/daemon/utils.mli index ec1127f72..d7b7aeba0 100644 --- a/daemon/utils.mli +++ b/daemon/utils.mli @@ -56,6 +56,16 @@ val sort_device_names : string list -> string list This also deals with partition numbers, and works whether or not [/dev/] is present. *) +val has_bogus_mbr : string -> bool +(** Check whether the first sector of the device contains a bogus MBR partition + table; namely one where the first partition table entry describes a + partition that starts at absolute sector 0, thereby overlapping the + partition table itself. + + dosfstools-4.2+ creates bogus partition tables like this by default when + formatting non-removable, non-partitioned block devices. Refer to + RHBZ#1931821. *) + val proc_unmangle_path : string -> string (** Reverse kernel path escaping done in fs/seq_file.c:mangle_path. This is inconsistently used for /proc fields. *)