Bug#985974: grub-common: grub-probe fails to parse lvm metadata with lvm type "cache-pool+METADATA_FORMAT"

[Keyu Tao]

Hi Debian GRUB maintainers,

I have written a patch today trying to fix this bug. Idea is that when 
grub_lvm_detect() meets unknown lvm segment type, manually iterate all 
characters afterwards until we are sure this segment finishes parsed.

--- lvm.c    2021-03-28 09:07:30.000000000 +0000
+++ /root/grub2-2.02+dfsg1/grub-core/disk/lvm.c    2021-03-28 
09:01:21.988239003 +0000
@@ -778,6 +778,28 @@
                if (p2)
              *p2 ='"';
  #endif
+              /* currently p pointer is in a string */
+              int in_string = ~0;
+              int brackets = 0;
+              while (++p)
+            {
+              if (*p == '"')
+                in_string = ~in_string;
+              if (*p == '{' && in_string == 0)
+                brackets += 1;
+              if (*p == '}' && in_string == 0)
+                {
+                  if (brackets == 0)
+                {
+                  p--;
+                  break;
+                }
+                  else
+                {
+                  brackets -= 1;
+                }
+                }
+            }
                /* Found a non-supported type, give up and move on. */
                skip_lv = 1;
                break;

I modified on the grub2 buster package, and grub-probe can work probably 
now, although this patch probably needs to be rewritten to follow GNU's 
code style.


On Sat, 27 Mar 2021 16:08:33 +0800 Keyu Tao <taoky99 at outlook.com> wrote:

 > Package: grub-common
 > Version: 2.02+dfsg1-20+deb10u4
 > Severity: important
 >
 > In lvm.c (grub-core/disk/lvm.c) of grub2, grub_lvm_detect() will 
"skip" unknown LVM type.
 > However, the lv segment type "cache-pool+METADATA_FORMAT" has a 
policy_settings like this:
 >
 > ```
 > mcache {
 > id = "BJvm0E-9uCj-Ji6a-N37P-nEuL-evA4-ZDUxdt"
 > status = ["READ", "WRITE"]
 > flags = []
 > creation_time = 1594104910 # 2020-07-07 14:55:10 +0800
 > creation_host = "mirrors4"
 > segment_count = 1
 > segment1 {
 > start_extent = 0
 > extent_count = 393216 # 1.5 Terabytes
 > type = "cache-pool+METADATA_FORMAT"
 > data = "mcache_cdata"
 > metadata = "mcache_cmeta"
 > chunk_size = 2048
 > metadata_format = 2
 > cache_mode = "writethrough"
 > policy = "smq"
 > policy_settings {
 > migration_threshold=16384
 > }
 > }
 > }
 > ```
 >
 > The unexpected "}" of policy_settings will mislead grub_strchr (p, '}')
 > to "skip" the variable p to "}", and thus grub considers parsing done,
 > and ignores all lvm metadata after this segment.
 >
 > It can bring the system unbootable. In our example, rootfs is a
 > mirror-type lv, and the "mirrors" in its segment references 2 lvs:
 >
 > ```
 > root {
 > id = "QqUaB2-R6JG-ZCFG-6LO9-KfKp-FsQx-6jeFp2"
 > status = ["READ", "WRITE", "VISIBLE"]
 > flags = []
 > creation_time = 1586435631 # 2020-04-09 20:33:51 +0800
 > creation_host = "mirrors4"
 > segment_count = 1
 > segment1 {
 > start_extent = 0
 > extent_count = 8192 # 32 Gigabytes
 > type = "mirror"
 > mirror_count = 2
 > mirror_log = "root_mlog"
 > region_size = 4096
 > mirrors = [
 > "root_mimage_0", 0,
 > "root_mimage_1", 0
 > ]
 > }
 > }