GNOME Bugzilla – Bug 783997
GParted detecting LVM2 PV regardless whether second magic matches or not
Last modified: 2017-08-07 17:20:45 UTC
Quoting the fix --8<-- Internal file system detection is broken and matches LVM2 PV even when only the first magic is found. # lvm pvcreate /dev/sdb1 # hexdump -C /dev/sdb1 > hexdump-1.txt Clear the second magic # python f = open("/dev/sdb1","w") f.seek(0x218) f.write("\x00"*4) f.close() quit() # hexdump -C /dev/sdb1 > hexdump-2.txt # diff -u hexdump-[12].txt 00000200 4c 41 42 45 4c 4f 4e 45 01 00 00 00 00 00 00 00 |LABELONE........| -00000210 58 69 83 e1 20 00 00 00 4c 56 4d 32 20 30 30 31 |Xi.. ...LVM2 001| +00000210 58 69 83 e1 20 00 00 00 00 00 00 00 20 30 30 31 |Xi.. ....... 001| ^^ ^^ ^^ ^^ ^^^^ 00000220 52 4b 31 73 50 77 49 66 6a 72 55 4c 6a 4d 30 58 |RK1sPwIfjrULjM0X| GParted still detects this as an LVM2 PV even though lvm and blkid do not. --8<-- Fix to follow.
Created attachment 354102 [details] [review] Ensure internal detection requires second magic when needed (v1) Hi Curtis, Here's the patch for this. Thanks, Mike
As related information I started looking at GParted_Core::detect_filesystem_internal() because on Fedora 26 Beta with GCC 7.1.1 this new warning is produced: ../../../src/GParted_Core.cc: In static member function ‘static GParted::FILESYSTEM GParted::GParted_Core::detect_filesystem_internal(PedDevice*, PedPartition*)’: ../../../src/GParted_Core.cc:1383:17: warning: argument 2 null where non-null expected [-Wnonnull] memcmp( magic2, signatures[i].sig2, len2 ) == 0 ) ) ~~~~~~^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ In file included from /usr/include/glib-2.0/glib/gi18n.h:24:0, from /usr/include/glibmm-2.4/glibmm/i18n.h:27, from ../../../include/i18n.h:9, from ../../../include/Utils.h:27, from ../../../include/Partition.h:26, from ../../../include/Device.h:21, from ../../../include/Win_GParted.h:21, from ../../../src/GParted_Core.cc:18: /usr/include/string.h:66:12: note: in a call to function ‘int memcmp(const void*, const void*, size_t)’ declared here extern int memcmp (const void *__s1, const void *__s2, size_t __n) ^~~~~~ Turns out they put this fix into GCC 7.0 which means that it more extensively checks the nullability of marked arguments, as it is for memcmp()'s arguments in Glibc's <string.h> header. Bug 17308 - nonnull attribute not as useful as it could be https://gcc.gnu.org/bugzilla/show_bug.cgi?id=17308 gcc revision 243661 https://gcc.gnu.org/viewcvs/gcc?view=revision&revision=243661 /usr/include/string.h extern int memcmp (const void *__s1, const void *__s2, size_t __n) __THROW __attribute_pure__ __nonnull ((1, 2)); When reading the detect_filesystem_internal() code I noticed the bug in the code that it didn't correctly check the second magic. Nicely, fixing this also clears the warning. Mike
Good catch on this Mike. At least the old code did properly detect when both signatures were in place. Patch v1 from comment #1 has been committed to the git repository. The relevant git commit can be viewed at the following link: Ensure internal detection requires second magic when needed (#783997) https://git.gnome.org/browse/gparted/commit/?id=87ccf060ce4d5ed3a175a43346d37db8df68fe28
Hi Curtis, Just for the record, this bug would have been virtually impossible for users to hit because: 1) blkid recognises LVM2 PVs so only ever needed to fall back to internal detection on very old and recently desupported RHEL/CentOS 5 distro. 2) It would be very, very rare for the first LVM2 PV magic to exist and not the second. So the primary benefit is clearing the nullable parameter warning on memcmp() when compiled with GCC >= 7.0. Mike
This enhancement was included in the GParted 0.29.0 release on August 7, 2017.