[jackhill/guix/guix.git] / gnu / packages / patches / grub-check-error-efibootmgr.patch

Without this patch, GRUB may proceed to wipe all firmware boot entries
and report a successful installation, even if efibootmgr hit an error.

Origin URL:
https://git.sv.gnu.org/cgit/grub.git/commit/?id=6400613ad0b463abc93362086a491cd2a5e99b0d

From 6400613ad0b463abc93362086a491cd2a5e99b0d Mon Sep 17 00:00:00 2001
From: Steve McIntyre <steve@einval.com>
Date: Wed, 31 Jan 2018 21:49:36 +0000
Subject: Make grub-install check for errors from efibootmgr

Code is currently ignoring errors from efibootmgr, giving users
clearly bogus output like:

        Setting up grub-efi-amd64 (2.02~beta3-4) ...
        Installing for x86_64-efi platform.
        Could not delete variable: No space left on device
        Could not prepare Boot variable: No space left on device
        Installation finished. No error reported.

and then potentially unbootable systems. If efibootmgr fails, grub-install
should know that and report it!

We've been using similar patch in Debian now for some time, with no ill effects.

diff --git a/grub-core/osdep/unix/platform.c b/grub-core/osdep/unix/platform.c
index a3fcfca..ca448bc 100644
--- a/grub-core/osdep/unix/platform.c
+++ b/grub-core/osdep/unix/platform.c
@@ -78,19 +78,20 @@ get_ofpathname (const char *dev)
 		   dev);
 }
 
-static void
+static int
 grub_install_remove_efi_entries_by_distributor (const char *efi_distributor)
 {
   int fd;
   pid_t pid = grub_util_exec_pipe ((const char * []){ "efibootmgr", NULL }, &fd);
   char *line = NULL;
   size_t len = 0;
+  int rc;
 
   if (!pid)
     {
       grub_util_warn (_("Unable to open stream from %s: %s"),
 		      "efibootmgr", strerror (errno));
-      return;
+      return errno;
     }
 
   FILE *fp = fdopen (fd, "r");
@@ -98,7 +99,7 @@ grub_install_remove_efi_entries_by_distributor (const char *efi_distributor)
     {
       grub_util_warn (_("Unable to open stream from %s: %s"),
 		      "efibootmgr", strerror (errno));
-      return;
+      return errno;
     }
 
   line = xmalloc (80);
@@ -119,23 +120,25 @@ grub_install_remove_efi_entries_by_distributor (const char *efi_distributor)
       bootnum = line + sizeof ("Boot") - 1;
       bootnum[4] = '\0';
       if (!verbosity)
-	grub_util_exec ((const char * []){ "efibootmgr", "-q",
+	rc = grub_util_exec ((const char * []){ "efibootmgr", "-q",
 	      "-b", bootnum,  "-B", NULL });
       else
-	grub_util_exec ((const char * []){ "efibootmgr",
+	rc = grub_util_exec ((const char * []){ "efibootmgr",
 	      "-b", bootnum, "-B", NULL });
     }
 
   free (line);
+  return rc;
 }
 
-void
+int
 grub_install_register_efi (grub_device_t efidir_grub_dev,
 			   const char *efifile_path,
 			   const char *efi_distributor)
 {
   const char * efidir_disk;
   int efidir_part;
+  int ret;
   efidir_disk = grub_util_biosdisk_get_osdev (efidir_grub_dev->disk);
   efidir_part = efidir_grub_dev->disk->partition ? efidir_grub_dev->disk->partition->number + 1 : 1;
 
@@ -151,23 +154,26 @@ grub_install_register_efi (grub_device_t efidir_grub_dev,
   grub_util_exec ((const char * []){ "modprobe", "-q", "efivars", NULL });
 #endif
   /* Delete old entries from the same distributor.  */
-  grub_install_remove_efi_entries_by_distributor (efi_distributor);
+  ret = grub_install_remove_efi_entries_by_distributor (efi_distributor);
+  if (ret)
+    return ret;
 
   char *efidir_part_str = xasprintf ("%d", efidir_part);
 
   if (!verbosity)
-    grub_util_exec ((const char * []){ "efibootmgr", "-q",
+    ret = grub_util_exec ((const char * []){ "efibootmgr", "-q",
 	  "-c", "-d", efidir_disk,
 	  "-p", efidir_part_str, "-w",
 	  "-L", efi_distributor, "-l", 
 	  efifile_path, NULL });
   else
-    grub_util_exec ((const char * []){ "efibootmgr",
+    ret = grub_util_exec ((const char * []){ "efibootmgr",
 	  "-c", "-d", efidir_disk,
 	  "-p", efidir_part_str, "-w",
 	  "-L", efi_distributor, "-l", 
 	  efifile_path, NULL });
   free (efidir_part_str);
+  return ret;
 }
 
 void
diff --git a/include/grub/util/install.h b/include/grub/util/install.h
index 5910b0c..0dba8b6 100644
--- a/include/grub/util/install.h
+++ b/include/grub/util/install.h
@@ -210,7 +210,7 @@ grub_install_create_envblk_file (const char *name);
 const char *
 grub_install_get_default_x86_platform (void);
 
-void
+int
 grub_install_register_efi (grub_device_t efidir_grub_dev,
 			   const char *efifile_path,
 			   const char *efi_distributor);
diff --git a/util/grub-install.c b/util/grub-install.c
index 5e4cdfd..690f180 100644
--- a/util/grub-install.c
+++ b/util/grub-install.c
@@ -1848,9 +1848,13 @@ main (int argc, char *argv[])
 	  if (!removable && update_nvram)
 	    {
 	      /* Try to make this image bootable using the EFI Boot Manager, if available.  */
-	      grub_install_register_efi (efidir_grub_dev,
-					 "\\System\\Library\\CoreServices",
-					 efi_distributor);
+	      int ret;
+	      ret = grub_install_register_efi (efidir_grub_dev,
+					       "\\System\\Library\\CoreServices",
+					       efi_distributor);
+	      if (ret)
+	        grub_util_error (_("efibootmgr failed to register the boot entry: %s"),
+				 strerror (ret));
 	    }
 
 	  grub_device_close (ins_dev);
@@ -1871,6 +1875,7 @@ main (int argc, char *argv[])
 	{
 	  char * efifile_path;
 	  char * part;
+	  int ret;
 
 	  /* Try to make this image bootable using the EFI Boot Manager, if available.  */
 	  if (!efi_distributor || efi_distributor[0] == '\0')
@@ -1887,7 +1892,10 @@ main (int argc, char *argv[])
 			  efidir_grub_dev->disk->name,
 			  (part ? ",": ""), (part ? : ""));
 	  grub_free (part);
-	  grub_install_register_efi (efidir_grub_dev,
-				     efifile_path, efi_distributor);
+	  ret = grub_install_register_efi (efidir_grub_dev,
+					   efifile_path, efi_distributor);
+	  if (ret)
+	    grub_util_error (_("efibootmgr failed to register the boot entry: %s"),
+			     strerror (ret));
 	}
       break;


Below is a followup to the patch above: the uninitialized variable could lead
‘grub-install’ to error out when it shouldn’t (seen on an AArch64 box where
‘grub_install_remove_efi_entries_by_distributor’ didn't have any entry to
remove):

  grub-install: error: efibootmgr failed to register the boot entry: Unknown error 65535.

See <http://lists.gnu.org/archive/html/bug-grub/2018-10/msg00006.html>.

--- grub-2.02/grub-core/osdep/unix/platform.c	2018-10-17 22:21:53.015284846 +0200
+++ grub-2.02/grub-core/osdep/unix/platform.c	2018-10-17 22:21:55.595271222 +0200
@@ -85,7 +85,7 @@ grub_install_remove_efi_entries_by_distr
   pid_t pid = grub_util_exec_pipe ((const char * []){ "efibootmgr", NULL }, &fd);
   char *line = NULL;
   size_t len = 0;
-  int rc;
+  int rc = 0;
 
   if (!pid)
     {
Commit	Line	Data
526ce419 MB	1	Without this patch, GRUB may proceed to wipe all firmware boot entries
	2	and report a successful installation, even if efibootmgr hit an error.
	3
	4	Origin URL:
	5	https://git.sv.gnu.org/cgit/grub.git/commit/?id=6400613ad0b463abc93362086a491cd2a5e99b0d
	6
	7	From 6400613ad0b463abc93362086a491cd2a5e99b0d Mon Sep 17 00:00:00 2001
	8	From: Steve McIntyre <steve@einval.com>
	9	Date: Wed, 31 Jan 2018 21:49:36 +0000
	10	Subject: Make grub-install check for errors from efibootmgr
	11
	12	Code is currently ignoring errors from efibootmgr, giving users
	13	clearly bogus output like:
	14
	15	Setting up grub-efi-amd64 (2.02~beta3-4) ...
	16	Installing for x86_64-efi platform.
	17	Could not delete variable: No space left on device
	18	Could not prepare Boot variable: No space left on device
	19	Installation finished. No error reported.
	20
	21	and then potentially unbootable systems. If efibootmgr fails, grub-install
	22	should know that and report it!
	23
	24	We've been using similar patch in Debian now for some time, with no ill effects.
	25
	26	diff --git a/grub-core/osdep/unix/platform.c b/grub-core/osdep/unix/platform.c
	27	index a3fcfca..ca448bc 100644
	28	--- a/grub-core/osdep/unix/platform.c
	29	+++ b/grub-core/osdep/unix/platform.c
	30	@@ -78,19 +78,20 @@ get_ofpathname (const char *dev)
	31	dev);
	32	}
	33
	34	-static void
	35	+static int
	36	grub_install_remove_efi_entries_by_distributor (const char *efi_distributor)
	37	{
	38	int fd;
	39	pid_t pid = grub_util_exec_pipe ((const char * []){ "efibootmgr", NULL }, &fd);
	40	char *line = NULL;
	41	size_t len = 0;
	42	+ int rc;
	43
	44	if (!pid)
	45	{
	46	grub_util_warn (_("Unable to open stream from %s: %s"),
	47	"efibootmgr", strerror (errno));
	48	- return;
	49	+ return errno;
	50	}
	51
	52	FILE *fp = fdopen (fd, "r");
	53	@@ -98,7 +99,7 @@ grub_install_remove_efi_entries_by_distributor (const char *efi_distributor)
	54	{
	55	grub_util_warn (_("Unable to open stream from %s: %s"),
	56	"efibootmgr", strerror (errno));
	57	- return;
	58	+ return errno;
	59	}
	60
	61	line = xmalloc (80);
	62	@@ -119,23 +120,25 @@ grub_install_remove_efi_entries_by_distributor (const char *efi_distributor)
	63	bootnum = line + sizeof ("Boot") - 1;
	64	bootnum[4] = '\0';
65	if (!verbosity)
66	- grub_util_exec ((const char * []){ "efibootmgr", "-q",
67	+ rc = grub_util_exec ((const char * []){ "efibootmgr", "-q",
68	"-b", bootnum, "-B", NULL });
69	else
70	- grub_util_exec ((const char * []){ "efibootmgr",
71	+ rc = grub_util_exec ((const char * []){ "efibootmgr",
72	"-b", bootnum, "-B", NULL });
73	}
74
75	free (line);
76	+ return rc;
77	}
78
79	-void
80	+int
81	grub_install_register_efi (grub_device_t efidir_grub_dev,
82	const char *efifile_path,
83	const char *efi_distributor)
84	{
85	const char * efidir_disk;
86	int efidir_part;
87	+ int ret;
88	efidir_disk = grub_util_biosdisk_get_osdev (efidir_grub_dev->disk);
89	efidir_part = efidir_grub_dev->disk->partition ? efidir_grub_dev->disk->partition->number + 1 : 1;
90
91	@@ -151,23 +154,26 @@ grub_install_register_efi (grub_device_t efidir_grub_dev,
92	grub_util_exec ((const char * []){ "modprobe", "-q", "efivars", NULL });
93	#endif
94	/* Delete old entries from the same distributor. */
95	- grub_install_remove_efi_entries_by_distributor (efi_distributor);
96	+ ret = grub_install_remove_efi_entries_by_distributor (efi_distributor);
97	+ if (ret)
98	+ return ret;
99
100	char *efidir_part_str = xasprintf ("%d", efidir_part);
101
102	if (!verbosity)
103	- grub_util_exec ((const char * []){ "efibootmgr", "-q",
104	+ ret = grub_util_exec ((const char * []){ "efibootmgr", "-q",
105	"-c", "-d", efidir_disk,
106	"-p", efidir_part_str, "-w",
107	"-L", efi_distributor, "-l",
108	efifile_path, NULL });
109	else
110	- grub_util_exec ((const char * []){ "efibootmgr",
111	+ ret = grub_util_exec ((const char * []){ "efibootmgr",
112	"-c", "-d", efidir_disk,
113	"-p", efidir_part_str, "-w",
114	"-L", efi_distributor, "-l",
115	efifile_path, NULL });
116	free (efidir_part_str);
117	+ return ret;
118	}
119
120	void
121	diff --git a/include/grub/util/install.h b/include/grub/util/install.h
122	index 5910b0c..0dba8b6 100644
123	--- a/include/grub/util/install.h
124	+++ b/include/grub/util/install.h
125	@@ -210,7 +210,7 @@ grub_install_create_envblk_file (const char *name);
126	const char *
127	grub_install_get_default_x86_platform (void);
128
129	-void
130	+int
131	grub_install_register_efi (grub_device_t efidir_grub_dev,
132	const char *efifile_path,
133	const char *efi_distributor);
134	diff --git a/util/grub-install.c b/util/grub-install.c
135	index 5e4cdfd..690f180 100644
136	--- a/util/grub-install.c
137	+++ b/util/grub-install.c
138	@@ -1848,9 +1848,13 @@ main (int argc, char *argv[])
139	if (!removable && update_nvram)
140	{
141	/* Try to make this image bootable using the EFI Boot Manager, if available. */
142	- grub_install_register_efi (efidir_grub_dev,
143	- "\\System\\Library\\CoreServices",
144	- efi_distributor);
145	+ int ret;
146	+ ret = grub_install_register_efi (efidir_grub_dev,
147	+ "\\System\\Library\\CoreServices",
148	+ efi_distributor);
149	+ if (ret)
150	+ grub_util_error (_("efibootmgr failed to register the boot entry: %s"),
151	+ strerror (ret));
152	}
153
154	grub_device_close (ins_dev);
155	@@ -1871,6 +1875,7 @@ main (int argc, char *argv[])
156	{
157	char * efifile_path;
158	char * part;
159	+ int ret;
160
161	/* Try to make this image bootable using the EFI Boot Manager, if available. */
162	if (!efi_distributor \|\| efi_distributor[0] == '\0')
d36cd888	163	@@ -1887,7 +1892,10 @@ main (int argc, char *argv[])
526ce419 MB	164	efidir_grub_dev->disk->name,
	165	(part ? ",": ""), (part ? : ""));
	166	grub_free (part);
	167	- grub_install_register_efi (efidir_grub_dev,
	168	- efifile_path, efi_distributor);
	169	+ ret = grub_install_register_efi (efidir_grub_dev,
	170	+ efifile_path, efi_distributor);
	171	+ if (ret)
	172	+ grub_util_error (_("efibootmgr failed to register the boot entry: %s"),
	173	+ strerror (ret));
	174	}
	175	break;
d36cd888 LC	176
	177
	178	Below is a followup to the patch above: the uninitialized variable could lead
	179	‘grub-install’ to error out when it shouldn’t (seen on an AArch64 box where
	180	‘grub_install_remove_efi_entries_by_distributor’ didn't have any entry to
	181	remove):
	182
	183	grub-install: error: efibootmgr failed to register the boot entry: Unknown error 65535.
	184
	185	See <http://lists.gnu.org/archive/html/bug-grub/2018-10/msg00006.html>.
	186
	187	--- grub-2.02/grub-core/osdep/unix/platform.c 2018-10-17 22:21:53.015284846 +0200
	188	+++ grub-2.02/grub-core/osdep/unix/platform.c 2018-10-17 22:21:55.595271222 +0200
	189	@@ -85,7 +85,7 @@ grub_install_remove_efi_entries_by_distr
	190	pid_t pid = grub_util_exec_pipe ((const char * []){ "efibootmgr", NULL }, &fd);
	191	char *line = NULL;
	192	size_t len = 0;
	193	- int rc;
	194	+ int rc = 0;
526ce419	195
d36cd888 LC	196	if (!pid)
d36cd888 LC	197	{