From 1bcac6b2d11c75d024bde5012e7c82cc8b6d2555ff606fdb0cc24f10b025b643 Mon Sep 17 00:00:00 2001 From: Stephan Kulow Date: Mon, 29 Jul 2013 15:41:41 +0000 Subject: [PATCH] Accepting request 184482 from Base:System - add grub2-fix-parsing-of-short-LVM-PV-names.patch - fix PV detection in grub-probe when PV name is less than 10 charaters - add grub2-fix-descriptor-leak-in-grub_util_is_imsm.patch - fix decriptor leak which later caused LVM warnings during grub-probe invocation Both problem were introduced in current trunk, version in 12.3 did not call external lvm tools. - remove --enable-grub-emu-usb - it is not needed on physical platform (forwarded request 184477 from arvidjaar) OBS-URL: https://build.opensuse.org/request/show/184482 OBS-URL: https://build.opensuse.org/package/show/openSUSE:Factory/grub2?expand=0&rev=71 --- ...descriptor-leak-in-grub_util_is_imsm.patch | 92 +++++++++++++++++++ grub2-fix-parsing-of-short-LVM-PV-names.patch | 75 +++++++++++++++ grub2.changes | 9 ++ grub2.spec | 9 +- 4 files changed, 180 insertions(+), 5 deletions(-) create mode 100644 grub2-fix-descriptor-leak-in-grub_util_is_imsm.patch create mode 100644 grub2-fix-parsing-of-short-LVM-PV-names.patch diff --git a/grub2-fix-descriptor-leak-in-grub_util_is_imsm.patch b/grub2-fix-descriptor-leak-in-grub_util_is_imsm.patch new file mode 100644 index 0000000..79000c6 --- /dev/null +++ b/grub2-fix-descriptor-leak-in-grub_util_is_imsm.patch @@ -0,0 +1,92 @@ +From: Andrey Borzenkov +To: grub-devel@gnu.org +Subject: [PATCH] fix memory and descriptor leaks in grub_util_is_imsm + +Descriptor leak caused warning from later vgs invocation. Fix memory +leak (buffer was not always freed) while on it. + +Signed-off-by: Andrey Borzenkov + +--- + util/getroot.c | 29 +++++++++++++++-------------- + 1 file changed, 15 insertions(+), 14 deletions(-) + +diff --git a/util/getroot.c b/util/getroot.c +index 2ad8a55..b30a0d8 100644 +--- a/util/getroot.c ++++ b/util/getroot.c +@@ -1455,10 +1455,12 @@ out: + static int + grub_util_is_imsm (const char *os_dev) + { +- int try; ++ int retry; ++ int is_imsm = 0; ++ int container_seen = 0; + const char *dev = os_dev; + +- for (try = 0; try < 2; try++) ++ do + { + char *argv[5]; + int fd; +@@ -1467,6 +1469,8 @@ grub_util_is_imsm (const char *os_dev) + char *buf = NULL; + size_t len = 0; + ++ retry = 0; /* We'll do one more pass if device is part of container */ ++ + /* execvp has inconvenient types, hence the casts. None of these + strings will actually be modified. */ + argv[0] = (char *) "mdadm"; +@@ -1499,7 +1503,8 @@ grub_util_is_imsm (const char *os_dev) + + while (getline (&buf, &len, mdadm) > 0) + { +- if (strncmp (buf, "MD_CONTAINER=", sizeof ("MD_CONTAINER=") - 1) == 0) ++ if (strncmp (buf, "MD_CONTAINER=", sizeof ("MD_CONTAINER=") - 1) == 0 ++ && !container_seen) + { + char *newdev, *ptr; + newdev = xstrdup (buf + sizeof ("MD_CONTAINER=") - 1); +@@ -1508,31 +1513,27 @@ grub_util_is_imsm (const char *os_dev) + ptr[1] = 0; + grub_util_info ("Container of %s is %s", dev, newdev); + dev = newdev; +- goto out; ++ container_seen = retry = 1; ++ break; + } + if (strncmp (buf, "MD_METADATA=imsm", + sizeof ("MD_METADATA=imsm") - 1) == 0) + { +- close (fd); +- waitpid (pid, NULL, 0); ++ is_imsm = 1; + grub_util_info ("%s is imsm", dev); +- if (dev != os_dev) +- free ((void *) dev); +- return 1; ++ break; + } + } + + free (buf); +- +- return 0; +- +- out: + close (fd); + waitpid (pid, NULL, 0); + } ++ while (retry); ++ + if (dev != os_dev) + free ((void *) dev); +- return 0; ++ return is_imsm; + } + #endif /* __linux__ */ + +-- +tg: (e1a892d..) u/imsm_descriptor_leak (depends on: master) diff --git a/grub2-fix-parsing-of-short-LVM-PV-names.patch b/grub2-fix-parsing-of-short-LVM-PV-names.patch new file mode 100644 index 0000000..1a0b44c --- /dev/null +++ b/grub2-fix-parsing-of-short-LVM-PV-names.patch @@ -0,0 +1,75 @@ +From: Andrey Borzenkov +To: grub-devel@gnu.org +Subject: [PATCH] fix parsing of LVM PV names for short names + +Default format of vgs output is + +- two spaces as standard prefix +- PV name left aligned to field width which is 10 characters + +This means that if PV name has less than 10 chacaters it has some spaces at +the end. + +Example: + +linux-chxo:~ # vgs -o pv_name --noheadings | cat -E + /dev/md1 $ + /dev/md101$ +linux-chxo:~ # + +There is no explicit option to turn off alignment; it is implicitly +disabled if one of --separator or --nameprefixes option is used. + +--separator was added in 2007, --nameprefixes - in 2009. So let's use +--separator to extend range of versions we are compatible with. Note that +one or another must be used, current parsing is broken otherwise. + +Signed-off-by: Andrey Borzenkov + +--- + util/getroot.c | 12 +++++++++--- + 1 file changed, 9 insertions(+), 3 deletions(-) + +diff --git a/util/getroot.c b/util/getroot.c +index 2ad8a55..3afcf96 100644 +--- a/util/getroot.c ++++ b/util/getroot.c +@@ -1322,7 +1322,7 @@ grub_util_get_dev_abstraction (const char *os_dev) + static void + pull_lvm_by_command (const char *os_dev) + { +- char *argv[6]; ++ char *argv[8]; + int fd; + pid_t pid; + FILE *mdadm; +@@ -1351,12 +1351,17 @@ pull_lvm_by_command (const char *os_dev) + + /* execvp has inconvenient types, hence the casts. None of these + strings will actually be modified. */ ++ /* by default PV name is left aligned in 10 character field, meaning that ++ we do not know where name ends. Using dummy --separator disables ++ alignment. We have a single field, so separator itself is not output */ + argv[0] = (char *) "vgs"; + argv[1] = (char *) "--options"; + argv[2] = (char *) "pv_name"; + argv[3] = (char *) "--noheadings"; +- argv[4] = vgname; +- argv[5] = NULL; ++ argv[4] = (char *) "--separator"; ++ argv[5] = (char *) ":"; ++ argv[6] = vgname; ++ argv[7] = NULL; + + pid = exec_pipe (argv, &fd); + free (vgname); +@@ -1376,6 +1381,7 @@ pull_lvm_by_command (const char *os_dev) + while (getline (&buf, &len, mdadm) > 0) + { + char *ptr; ++ /* LVM adds two spaces as standard prefix */ + for (ptr = buf; ptr < buf + 2 && *ptr == ' '; ptr++); + if (*ptr == '\0') + continue; +-- +tg: (ebd40b6..) u/strip-pv-trailing-blanks (depends on: master) diff --git a/grub2.changes b/grub2.changes index b9f0b0a..70bb63d 100644 --- a/grub2.changes +++ b/grub2.changes @@ -1,3 +1,12 @@ +------------------------------------------------------------------- +Sat Jul 27 10:12:36 UTC 2013 - arvidjaar@gmail.com + +- add grub2-fix-parsing-of-short-LVM-PV-names.patch - fix PV detection in + grub-probe when PV name is less than 10 charaters +- add grub2-fix-descriptor-leak-in-grub_util_is_imsm.patch - fix decriptor + leak which later caused LVM warnings during grub-probe invocation +- remove --enable-grub-emu-usb - it is not needed on physical platform + ------------------------------------------------------------------- Tue Jul 9 10:54:41 UTC 2013 - mchang@suse.com diff --git a/grub2.spec b/grub2.spec index e0cd7f5..7fc0a32 100644 --- a/grub2.spec +++ b/grub2.spec @@ -130,6 +130,8 @@ Patch29: grub2-secureboot-chainloader.patch Patch30: grub2-cdpath.patch Patch34: grub2-secureboot-use-linuxefi-on-uefi-in-os-prober.patch Patch35: grub2-linguas.sh-no-rsync.patch +Patch36: grub2-fix-parsing-of-short-LVM-PV-names.patch +Patch37: grub2-fix-descriptor-leak-in-grub_util_is_imsm.patch Requires: gettext-runtime %if 0%{?suse_version} >= 1140 Requires: os-prober @@ -244,6 +246,8 @@ mv po/grub.pot po/%{name}.pot %patch30 -p1 %patch34 -p1 %patch35 -p1 +%patch36 -p1 +%patch37 -p1 # Generate po/LINGUAS for message catalogs ... ./linguas.sh @@ -345,10 +349,6 @@ cd build %define _target_platform i386-%{_vendor}-%{_target_os}%{?_gnu} %endif -%ifnarch ppc ppc64 -%define extraconfigure --enable-grub-emu-usb -%endif - # -static is needed so that autoconf script is able to link # test that looks for _start symbol on 64 bit platforms ../configure TARGET_LDFLAGS=-static \ @@ -356,7 +356,6 @@ cd build --sysconfdir=%{_sysconfdir} \ --target=%{_target_platform} \ --with-platform=%{platform} \ - %{?extraconfigure} \ --program-transform-name=s,grub,%{name}, make %{?_smp_mflags} %endif