Re: Strange failure during de_CH.po generation with make -j
Hi! On Sun, Jan 14, 2024 at 03:58:42PM +0800, Xinhui Yang via Grub-devel wrote: > >Recently we observed a strange failure while packaging GRUB 2.12 for >our distro. The translation file generation process might fail if >parallelism was enabled (e.g. `make -j', or with `-j16' and larger >numbers). > >We found that the build always failed whilst generating de_CH >translation data from de.po, complaining about syntax errors or a >premature EOF whilst processing the strings. The exact line numbers >msgfilter complained about were random. If we remove de_CH from >linguas.sh, the build passes every time, even with parallelism >enabled - of course, with parallelism disabled, we did not observe >this issue. > >This issue has been reproduced on: > >- AOSC OS, the aforementioned distro that I work on. >- Debian Bookworm. > >I propose the following solutions: > >- Tell the distro maintainers not to use -j. >- Patch po/gettext-patches to add a .NOTPARALLEL directive in Makefile.in.in. > >We have opted to temporarily disable parallelism in po/Makefile, since we >could not find the culprit to this issue. I've seen this before when hacking on the Debian packaging too. IIRC it's because the po build does not use separated-out build dirs like the binary builds, and that would the right way to fix this. -- Steve McIntyre, Cambridge, UK.st...@einval.com "The problem with defending the purity of the English language is that English is about as pure as a cribhouse whore. We don't just borrow words; on occasion, English has pursued other languages down alleyways to beat them unconscious and rifle their pockets for new vocabulary." -- James D. Nicoll ___ Grub-devel mailing list Grub-devel@gnu.org https://lists.gnu.org/mailman/listinfo/grub-devel
Re: [PATCH v2 1/6] ia64: Remove support
On Thu, May 11, 2023 at 02:17:57PM +0200, Ard Biesheuvel wrote: >On Thu, 11 May 2023 at 14:14, John Paul Adrian Glaubitz > wrote: >> >> On Thu, 2023-05-11 at 14:06 +0200, Ard Biesheuvel wrote: >> > Itanium IA-64 support is obsolete, and implements its own flavor of EFI >> > boot that deviates from other architectures. Given that IA64 is unused >> > and unmaintained, it makes no sense to pretend that the EFI changes we >> > are making are tested or supported on IA64, so let's just get rid of it. >> >> But I just recently tested GRUB from git on IA64 and it worked without >> any problems. We're using GRUB to boot Debian on IA64. >> > >IA-64 is a dead platform, and a waste of electricity. > >Feel free to keep using it, but please stop demanding that our people >keep wasting their time on it. If you want to support it in Debian, >you can carry it as a downstream patch and shoulder the maintenance >burden. And I'm not convinced that as the Debian maintainer I care enough. At some point old machines and old architectures die, it's a fact of life. If people insist on keeping their old machines alive for the sake of it, then at some point they also will have to accept staying on old software too. -- Steve McIntyre, Cambridge, UK.st...@einval.com “Changing random stuff until your program works is bad coding practice, but if you do it fast enough it’s Machine Learning.” -- https://twitter.com/manisha72617183 ___ Grub-devel mailing list Grub-devel@gnu.org https://lists.gnu.org/mailman/listinfo/grub-devel
Re: [PATCH] font: Try opening fonts from the bundled memdisk
We're running with this patch in Debian builds right now and it's
working well...
Reviewed-by: Steve McIntyre <93...@debian.org>
Tested-by: Steve McIntyre <93...@debian.org>
(whichever you prefer!)
On Wed, Apr 26, 2023 at 12:06:52PM +0200, Chris Coulson wrote:
>Grub since 93a786a00163e50c29f0394df198518617e1c9a5 has enforced
>verification of font files in secure boot mode. In order to continue to
>be able to load some default fonts, vendors may bundle them with their
>signed EFI image by adding them to the built-in memdisk.
>
>This change makes the font loader try loading fonts from the memdisk
>before the prefix path when attempting to load a font file by specifying
>its filename, which avoids having to make changes to grub configurations
>in order to accommodate memdisk bundled fonts. It expects the directory
>structure to be the same as fonts stored in the prefix path
>(ie, /fonts/.pf2).
>
>Signed-off-by: Chris Coulson
>---
> grub-core/font/font.c | 48 ---
> 1 file changed, 31 insertions(+), 17 deletions(-)
>
>diff --git a/grub-core/font/font.c b/grub-core/font/font.c
>index 24adcb35a..7c83467a3 100644
>--- a/grub-core/font/font.c
>+++ b/grub-core/font/font.c
>@@ -415,6 +415,27 @@ read_section_as_short (struct font_file_section *section,
> return 0;
> }
>
>+static grub_file_t
>+try_open_from_prefix (const char *prefix, const char *filename)
>+{
>+ grub_file_t file;
>+ char *fullname, *ptr;
>+
>+ fullname = grub_malloc (grub_strlen (prefix) + grub_strlen (filename) + 1
>++ sizeof ("/fonts/") + sizeof (".pf2"));
>+ if (!fullname)
>+return 0;
>+ ptr = grub_stpcpy (fullname, prefix);
>+ ptr = grub_stpcpy (ptr, "/fonts/");
>+ ptr = grub_stpcpy (ptr, filename);
>+ ptr = grub_stpcpy (ptr, ".pf2");
>+ *ptr = 0;
>+
>+ file = grub_buffile_open (fullname, GRUB_FILE_TYPE_FONT, 1024);
>+ grub_free (fullname);
>+ return file;
>+}
>+
> /* Load a font and add it to the beginning of the global font list.
>Returns 0 upon success, nonzero upon failure. */
> grub_font_t
>@@ -433,25 +454,18 @@ grub_font_load (const char *filename)
> file = grub_buffile_open (filename, GRUB_FILE_TYPE_FONT, 1024);
> else
> {
>- const char *prefix = grub_env_get ("prefix");
>- char *fullname, *ptr;
>- if (!prefix)
>+ file = try_open_from_prefix ("(memdisk)", filename);
>+ if (!file)
> {
>-grub_error (GRUB_ERR_FILE_NOT_FOUND, N_("variable `%s' isn't set"),
>-"prefix");
>-goto fail;
>+const char *prefix = grub_env_get ("prefix");
>+if (!prefix)
>+ {
>+grub_error (GRUB_ERR_FILE_NOT_FOUND, N_("variable `%s' isn't
>set"),
>+"prefix");
>+goto fail;
>+ }
>+file = try_open_from_prefix (prefix, filename);
> }
>- fullname = grub_malloc (grub_strlen (prefix) + grub_strlen (filename) +
>1
>-+ sizeof ("/fonts/") + sizeof (".pf2"));
>- if (!fullname)
>- goto fail;
>- ptr = grub_stpcpy (fullname, prefix);
>- ptr = grub_stpcpy (ptr, "/fonts/");
>- ptr = grub_stpcpy (ptr, filename);
>- ptr = grub_stpcpy (ptr, ".pf2");
>- *ptr = 0;
>- file = grub_buffile_open (fullname, GRUB_FILE_TYPE_FONT, 1024);
>- grub_free (fullname);
> }
> if (!file)
> goto fail;
>--
>2.39.2
>
>
>___
>Grub-devel mailing list
>Grub-devel@gnu.org
>https://lists.gnu.org/mailman/listinfo/grub-devel
--
Steve McIntyre, Cambridge, UK.st...@einval.com
'There is some grim amusement in watching Pence try to run the typical
"politician in the middle of a natural disaster" playbook, however
incompetently, while Trump scribbles all over it in crayon and eats some
of the pages.' -- Russ Allbery
___
Grub-devel mailing list
Grub-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/grub-devel
Re: [programmer11...@programist.ru: Bug#1021846: grub-install is broken since 2.06-3: error: unknown filesystem]
On Sat, Dec 10, 2022 at 07:40:47AM +0300, программист некто wrote: >Hello. Sorry for long wait. > >>программист некто: could you please try these changes and report back? > >I tried the first patch with grub 2.06-7. Result: grub-install works without >error. Cool, thanks for confirming! -- Steve McIntyre, Cambridge, UK.st...@einval.com "Since phone messaging became popular, the young generation has lost the ability to read or write anything that is longer than one hundred and sixty characters." -- Ignatios Souvatzis ___ Grub-devel mailing list Grub-devel@gnu.org https://lists.gnu.org/mailman/listinfo/grub-devel
Re: [RFC PATCH 4/4] kern/efi/sb: Use shim to verify font files
On Tue, Dec 06, 2022 at 11:09:57AM -0500, Robbie Harwood wrote: >Zhang Boyang writes: > >> Since font files can be wrapped as PE images by grub-wrap, use shim to >> verify font files if Secure Boot is enabled. To prevent other PE files >> (e.g. kernel images) used as wrappers, it only allows files marked as >> Windows GUI used as wrappers. > >Thanks for writing this; it's helpful to have something concrete to look >at. Definitely! >This approach is very font-focused, and while I understand that given >the discussion, I do still wonder if it wouldn't be better to make fonts >an instance of modules. If fonts become instances of modules, and >modules are wrapped into PE files, that not only seems cleaner but also >gives us signed module support without baking those into the image. > >What do you think? Nod, that probably makes more sense if we want to go this way. I'm not sure we do personally, but... -- Steve McIntyre, Cambridge, UK.st...@einval.com Dance like no one's watching. Encrypt like everyone is. - @torproject ___ Grub-devel mailing list Grub-devel@gnu.org https://lists.gnu.org/mailman/listinfo/grub-devel
[PATCH] kern/file: Fix error handling in grub_file_open()
grub_file_open() calls grub_file_get_device_name(), but doesn't check the return. Instead, it checks if grub_errno is set. However, nothing initialises grub_errno here when grub_file_open() starts. This means that trying to open one file that doesn't exist and then trying to open another file that does will (incorrectly) also fail to open that second file. Let's fix that. Signed-off-by: Steve McIntyre --- grub-core/kern/file.c | 3 +++ 1 file changed, 3 insertions(+) diff --git a/grub-core/kern/file.c b/grub-core/kern/file.c index 8d48fd50d..668b149c3 100644 --- a/grub-core/kern/file.c +++ b/grub-core/kern/file.c @@ -66,6 +66,9 @@ grub_file_open (const char *name, enum grub_file_type type) const char *file_name; grub_file_filter_id_t filter; + /* Reset grub_errno before we start */ + grub_errno = GRUB_ERR_NONE; + device_name = grub_file_get_device_name (name); if (grub_errno) goto fail; -- 2.30.2 ___ Grub-devel mailing list Grub-devel@gnu.org https://lists.gnu.org/mailman/listinfo/grub-devel
Re: [programmer11...@programist.ru: Bug#1021846: grub-install is broken since 2.06-3: error: unknown filesystem]
Hi Daniel! On Sat, Dec 03, 2022 at 01:41:51AM +1100, Daniel Axtens wrote: >Steve McIntyre writes: >> >> программист некто (in CC) reported this bug a few weeks back in >> Debian. Since I applied the bundle of filesystem bounds-checking fixes >> a few months back, he can't run grub-install. He's done the work to >> determine that the patch that breaks things for him is >> >> 2d014248d540c7e087934a94b6e7a2aa7fc2c704 fs/f2fs: Do not read past the end >> of nat journal entries >> >> The full thread of our discussion is at https://bugs.debian.org/1021846 >> >> I don't have any knowledge of f2fs to go any further here. Help please! :-) > >Ergh, apologies for the regression. > >[somewhat off-topic: The fix came from a crash derived from fuzzing. I >am not really knowledgeable about f2fs either - I was just trying to do >my best based on what we could derive from the existing driver. In >general, filesystems are a nightmare for fuzzing fixes because testing >beyond the (quite decent!) tests that the grub test-suite runs is very >challenging. There is usually no-one who is both involved in grub >security and an expert on any given file system either. We do the best >we can. Sadly our regression rate has been climbing, so we may need to >come up with some other way to secure file systems or get access to >sufficient expertise in the future.] ACK. I used to develop amd maintain filesystems as a day job, I understand the issue! Writing good and comprehensive tests is hard, and therefore quite rare! >I had a massive, massive work-in-progress spiel where I looked at this >code and compared the linux code and counted sizes and so on and so >forth. I was getting nowhere. But eventually I realised I had just made >an off-by-one error in the test. You're allowed to have up to n = >NAT_JOURNAL_ENTRIES entries _inclusive_, because the loop below uses i < >n, not i <= n. D'oh. Doh indeed! :-) >Please try the following: > >diff --git a/grub-core/fs/f2fs.c b/grub-core/fs/f2fs.c >index df6beb544cbd..855e24618c2b 100644 >--- a/grub-core/fs/f2fs.c >+++ b/grub-core/fs/f2fs.c >@@ -650,7 +650,7 @@ get_blkaddr_from_nat_journal (struct grub_f2fs_data *data, >grub_uint32_t nid, > grub_uint16_t n = grub_le_to_cpu16 (data->nat_j.n_nats); > grub_uint16_t i; > >- if (n >= NAT_JOURNAL_ENTRIES) >+ if (n > NAT_JOURNAL_ENTRIES) > return grub_error (GRUB_ERR_BAD_FS, >"invalid number of nat journal entries"); > > >If for some reason that doesn't work, please add the following debug >code and report the results: > >diff --git a/grub-core/fs/f2fs.c b/grub-core/fs/f2fs.c >index 855e24618c2b..6e49a6d17b7a 100644 >--- a/grub-core/fs/f2fs.c >+++ b/grub-core/fs/f2fs.c >@@ -643,6 +643,10 @@ get_nat_journal (struct grub_f2fs_data *data) > return err; > } > >+#ifdef GRUB_UTIL >+#include >+#endif >+ > static grub_err_t > get_blkaddr_from_nat_journal (struct grub_f2fs_data *data, grub_uint32_t nid, > grub_uint32_t *blkaddr) >@@ -650,6 +654,10 @@ get_blkaddr_from_nat_journal (struct grub_f2fs_data >*data, grub_uint32_t nid, > grub_uint16_t n = grub_le_to_cpu16 (data->nat_j.n_nats); > grub_uint16_t i; > >+#ifdef GRUB_UTIL >+ fprintf(stderr, "%s: n = %hu\n", __func__, n); >+#endif >+ > if (n > NAT_JOURNAL_ENTRIES) > return grub_error (GRUB_ERR_BAD_FS, >"invalid number of nat journal entries"); > программист некто: could you please try these changes and report back? >Amusingly the debug code shows that the grub-fs-tester tests always have >n = 0, which makes sense for a test that doesn't really stress the >file-system, and also explains why we didn't catch the bug when it was >introduced. Right. -- Steve McIntyre, Cambridge, UK.st...@einval.com Armed with "Valor": "Centurion" represents quality of Discipline, Honor, Integrity and Loyalty. Now you don't have to be a Caesar to concord the digital world while feeling safe and proud. ___ Grub-devel mailing list Grub-devel@gnu.org https://lists.gnu.org/mailman/listinfo/grub-devel
Fonts and theming and what to do in future with SB
Hey folks! So, with the latest set of GRUB CVE patches we've fixed up a bunch of potential crashes in font-handling code that could lead to Secure Boot holes. These are good and useful fixes, and thanks to Zhang Boyang and everyone else involved! There were also a few other changes: * In SB mode, refuse to load fonts from outside of the signed GRUB image * Restrictions to image dimensions * Fix integer overflow in fbutil Locking down fonts here has caused some issues that I've seen. We didn't update the config generation code in util/grub.d, so we're still generating grub.cfg files that will try (and fail!) to load fonts from other locations at runtime in SB mode. This causes ugly errors, and also causes GRUB to fail to set up video as normal. We can fix this, but it would be nice to agree on something upstream rather than as diverging distro patches. AFAIK Chris Coulson has a patch for the font loader to cause it to try loading fonts from the embedded memdisk first. Is that the best approach? If so, what fonts should we be embedding in the signed image? It's a tradeoff between size and functionality, of course - some people are happy with just "unicode" while others may want a wider choice for added theming options. Is the size an issue for most people? Or... Could/should we look at options to sign fonts separately? I've heard suggestions to embed them into faked-up modules that we could load with insmod, but of course we don't support signing modules yet anyway... :-) Thoughts?? -- Steve McIntyre, Cambridge, UK.st...@einval.com "I used to be the first kid on the block wanting a cranial implant, now I want to be the first with a cranial firewall. " -- Charlie Stross ___ Grub-devel mailing list Grub-devel@gnu.org https://lists.gnu.org/mailman/listinfo/grub-devel
Re: [programmer11...@programist.ru: Bug#1021846: grub-install is broken since 2.06-3: error: unknown filesystem]
Hi Sudhakar! On Fri, Nov 25, 2022 at 10:52:39AM +0530, sudhakar wrote: >Hi Steve, > >It seems invalid Commit id which you reported. It should be >4bd9877f62166b7e369773ab92fe24a39f6515f8 >did you applied below patch and tested? Could you please confirm that. > >fs/f2fs: Do not read past the end of nat journal entries > >https://git.savannah.gnu.org/cgit/grub.git/patch/?id=4bd9877f62166b7e369773ab92fe24a39f6515f8 It's exactly the same patch, just the commit hash is different when pulled into our 2.06 tree. Cheers, Steve -- Steve McIntyre, Cambridge, UK.st...@einval.com Welcome my son, welcome to the machine. ___ Grub-devel mailing list Grub-devel@gnu.org https://lists.gnu.org/mailman/listinfo/grub-devel
[programmer11...@programist.ru: Bug#1021846: grub-install is broken since 2.06-3: error: unknown filesystem]
Hi all! программист некто (in CC) reported this bug a few weeks back in Debian. Since I applied the bundle of filesystem bounds-checking fixes a few months back, he can't run grub-install. He's done the work to determine that the patch that breaks things for him is 2d014248d540c7e087934a94b6e7a2aa7fc2c704 fs/f2fs: Do not read past the end of nat journal entries The full thread of our discussion is at https://bugs.debian.org/1021846 I don't have any knowledge of f2fs to go any further here. Help please! :-) - Forwarded message from программист некто - From: программист некто To: sub...@bugs.debian.org Date: Sat, 15 Oct 2022 23:54:36 +0300 Subject: Bug#1021846: grub-install is broken since 2.06-3: error: unknown filesystem Message-Id: <3168731665867...@wf4nrjvtssjecb53.iva.yp-c.yandex.net> Package: grub-pc Version: 2.06-3~deb11u1 Severity: critical Hello. Since version 2.06-3, grub-install is broken: it fails with "error: unknown filesystem". I test command /usr/sbin/grub-install -v /dev/sda in some versions. Results in mail attachments. Versions older than 2.06-3 works without error (2.06-2 and lower). Tested versions: 2.04-20, 2.06-1, 2.06-2, 2.06-3~deb10u1, 2.06-3~deb11u1, 2.06-4. Disk partitions: # fdisk --list-details Disk /dev/sda: 29,82 GiB, 32017047552 bytes, 62533296 sectors Disk model: TS32GSSD370S Units: sectors of 1 * 512 = 512 bytes Sector size (logical/physical): 512 bytes / 512 bytes I/O size (minimum/optimal): 512 bytes / 512 bytes Disklabel type: dos Disk identifier: 0xc7177f7e Device Boot Start End Sectors Id Type Start-C/H/S End-C/H/S Attrs /dev/sda1 2048 22763519 22761472 83 Linux 4/4/1 1023/254/2 /dev/sda2 * 25866240 62531583 36665344 7 HPFS/ 1023/254/2 1023/254/2 80 $ disktype /dev/sda1 --- /dev/sda1 Block device, size 10.85 GiB (11653873664 bytes) F2FS file system (version 1.14) $ disktype /dev/sda2 --- /dev/sda2 Block device, size 17.48 GiB (18772656128 bytes) NTFS file system Volume size 17.48 GiB (18772652032 bytes, 36665336 sectors) - End forwarded message - -- Steve McIntyre, Cambridge, UK.st...@einval.com Mature Sporty Personal More Innovation More Adult A Man in Dandism Powered Midship Specialty ___ Grub-devel mailing list Grub-devel@gnu.org https://lists.gnu.org/mailman/listinfo/grub-devel
Re: [PATCH] Explicitly unset SOURCE_DATE_EPOCH before running fs tests
On Thu, Sep 22, 2022 at 07:18:50PM +0200, Daniel Kiper wrote: >On Sun, Sep 18, 2022 at 11:12:02PM +0100, Steve McIntyre wrote: >> In some filesystem utils like mksquashfs, they will silently change >> behaviour and cause timestamps to unexpectedly change. Build >> environments like Debian's set SOURCE_DATE_EPOCH in the environment, >> so remove it. Reproducible builds are good and useful for shipped >> artifacts, but this causes build-time tests to fail. >> >> Signed-off-by: Steve McIntyre > >Reviewed-by: Daniel Kiper but... > >> --- >> tests/util/grub-fs-tester.in | 3 +++ >> 1 file changed, 3 insertions(+) >> >> diff --git a/tests/util/grub-fs-tester.in b/tests/util/grub-fs-tester.in >> index 43f6175c3..6d70967e6 100644 >> --- a/tests/util/grub-fs-tester.in >> +++ b/tests/util/grub-fs-tester.in >> @@ -5,6 +5,9 @@ export BLKID_FILE=/dev/null >> >> fs="$1" >> >> +# We can't have this set, or filesystem tests will fail >> +unset SOURCE_DATE_EPOCH >> + > >... I would put this change before fs assignment above. It would be next >to export and look more logically. At least for me... :-) If you do not >object I will make this change before applying the patch. Sure, go for it. :-) -- Steve McIntyre, Cambridge, UK.st...@einval.com You raise the blade, you make the change... You re-arrange me 'til I'm sane... ___ Grub-devel mailing list Grub-devel@gnu.org https://lists.gnu.org/mailman/listinfo/grub-devel
[PATCH] Explicitly unset SOURCE_DATE_EPOCH before running fs tests
In some filesystem utils like mksquashfs, they will silently change
behaviour and cause timestamps to unexpectedly change. Build
environments like Debian's set SOURCE_DATE_EPOCH in the environment,
so remove it. Reproducible builds are good and useful for shipped
artifacts, but this causes build-time tests to fail.
Signed-off-by: Steve McIntyre
---
tests/util/grub-fs-tester.in | 3 +++
1 file changed, 3 insertions(+)
diff --git a/tests/util/grub-fs-tester.in b/tests/util/grub-fs-tester.in
index 43f6175c3..6d70967e6 100644
--- a/tests/util/grub-fs-tester.in
+++ b/tests/util/grub-fs-tester.in
@@ -5,6 +5,9 @@ export BLKID_FILE=/dev/null
fs="$1"
+# We can't have this set, or filesystem tests will fail
+unset SOURCE_DATE_EPOCH
+
GRUBFSTEST="@builddir@/grub-fstest"
tempdir=`mktemp -d "${TMPDIR:-/tmp}/${0##*/}.$(date
'+%Y%m%d%H%M%S%N').${fs}.XXX"` ||
--
2.30.2
___
Grub-devel mailing list
Grub-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/grub-devel
Re: [PATCH] Remove HFS support
On Fri, Aug 19, 2022 at 04:03:38PM +0200, John Paul Adrian Glaubitz wrote: >> On Aug 19, 2022, at 3:59 PM, Daniel Kiper wrote: >> >> If I do not hear any major objections in the following weeks I will >> merge this patch or a variant of it in the second half of September. > >We’re still formatting our /boot partitions for Debian PowerPC for >PowerMacs using HFS, so this change would be a breaking change for >us. > >So, that would be a no from Debian’s side. Not so fast please, Adrian. At the risk of sounding harsh, non-release old ports like powerpc *really* don't get to dictate things in Debian terms. As Daniel Axtens has been finding out, the HFS code is terrible in terms of security. If you still need it for old/semi-dead machines, maybe you should fork an older grub release and stay with that? -- Steve McIntyre, Cambridge, UK.st...@einval.com Getting a SCSI chain working is perfectly simple if you remember that there must be exactly three terminations: one on one end of the cable, one on the far end, and the goat, terminated over the SCSI chain with a silver-handled knife whilst burning *black* candles. --- Anthony DeBoer ___ Grub-devel mailing list Grub-devel@gnu.org https://lists.gnu.org/mailman/listinfo/grub-devel
Re: [ANNOUNCEMENT] GRUB2 minisumit starts in November
On Wed, Oct 14, 2020 at 03:37:32PM +0200, Daniel Kiper wrote: >Hey, > >GRUB2 minisumit starts in November. CfP is open. More you can find here: > https://twitter.com/3mdeb_com/status/1316057910816976899?s=20 Cool! Do you have a wiki page or similar to track things? :-) -- Steve McIntyre, Cambridge, UK.st...@einval.com You lock the door And throw away the key There's someone in my head but it's not me ___ Grub-devel mailing list Grub-devel@gnu.org https://lists.gnu.org/mailman/listinfo/grub-devel
Re: [ANNOUNCEMENT] FOSDEM 2020 Open Source Firmware, BMC and Bootloader Devroom Call for Participation
On Fri, Jan 10, 2020 at 02:50:59PM +0100, Daniel Kiper wrote: >Hi Steve, > >On Fri, Jan 03, 2020 at 06:23:34PM +0000, Steve McIntyre wrote: >> Hi Daniel! >> >> On Mon, Nov 25, 2019 at 11:19:52AM +0100, Daniel Kiper wrote: >> > >> >Some of you may already know but we think that it is worth posting this >> >thing >> >here too. Piotr Król from 3mdeb and I are looking for papers for FOSDEM 2020 >> >Open Source Firmware, BMC and Bootloader Devroom. If you want to help us >> >to make this room interesting please send us your proposal until Saturday, >> >1st December 2019 23:59:59 UTC. You can find full CfP here: >> > https://lists.fosdem.org/pipermail/fosdem/2019q4/002933.html >> > >> >We will be more than happy if you forward this email to interesting >> >parties too. >> > >> >If you have any questions please do not hesitate to ask us here or >> >in private. >> >> Are you planning a meetup to talk about Grub development again this >> year, please? > >Yes, "GRUB upstream and distros cooperation" session will be held at >Distributions devroom on Sunday at 14:30 LMT [1]. I decided to stick >with distro guys because it seems to me that people used to get GRUB >session there. At least for time being... Cool. See you there! -- Steve McIntyre, Cambridge, UK.st...@einval.com "Managing a volunteer open source project is a lot like herding kittens, except the kittens randomly appear and disappear because they have day jobs." -- Matt Mackall ___ Grub-devel mailing list Grub-devel@gnu.org https://lists.gnu.org/mailman/listinfo/grub-devel
Re: [ANNOUNCEMENT] FOSDEM 2020 Open Source Firmware, BMC and Bootloader Devroom Call for Participation
Hi Daniel! On Mon, Nov 25, 2019 at 11:19:52AM +0100, Daniel Kiper wrote: > >Some of you may already know but we think that it is worth posting this thing >here too. Piotr Król from 3mdeb and I are looking for papers for FOSDEM 2020 >Open Source Firmware, BMC and Bootloader Devroom. If you want to help us >to make this room interesting please send us your proposal until Saturday, >1st December 2019 23:59:59 UTC. You can find full CfP here: > https://lists.fosdem.org/pipermail/fosdem/2019q4/002933.html > >We will be more than happy if you forward this email to interesting >parties too. > >If you have any questions please do not hesitate to ask us here or >in private. Are you planning a meetup to talk about Grub development again this year, please? -- Steve McIntyre, Cambridge, UK.st...@einval.com Is there anybody out there? ___ Grub-devel mailing list Grub-devel@gnu.org https://lists.gnu.org/mailman/listinfo/grub-devel
Re: GRUB failed to install on Fujitsu M10-4
On Tue, Jun 04, 2019 at 09:56:37AM +0200, John Paul Adrian Glaubitz wrote: >On 6/4/19 2:56 AM, Sonnie Hook wrote: >> Now I re-installed from cdrom. When installing GRUB on disk, I got this >> message: >> It seems that this computer is configured to boot via EFI, but maybe that >> configuration will not work for booting from the hard drive. Some EFI >> firmware implementations do not meet the EFI specification ... blabla > >This message is generated by grub-installer, i.e. the component of >debian-installer which installs GRUB during installation. The >relevant code part can found here [1], the relevant message text here >[2]. > >I haven't analyzed the code in detail yet so I'm not sure why it >triggers in your case. I will perform a test installation inside a >SPARC T5 LDOM to find out. That's code I added into grub-installer. It's clearly a bug to see it firing here! -- Steve McIntyre, Cambridge, UK.st...@einval.com "... the premise [is] that privacy is about hiding a wrong. It's not. Privacy is an inherent human right, and a requirement for maintaining the human condition with dignity and respect." -- Bruce Schneier ___ Grub-devel mailing list Grub-devel@gnu.org https://lists.gnu.org/mailman/listinfo/grub-devel
Re: [RFC 1/1] mkimage: revert "Align efi sections on 4k boundary"
On Thu, Apr 18, 2019 at 05:14:58PM +0200, Heinrich Schuchardt wrote:
>On 4/18/19 12:41 PM, Daniel Kiper wrote:
>> On Wed, Apr 17, 2019 at 07:50:09AM +0200, Heinrich Schuchardt wrote:
>> > Since this commit a51f953f4ee8 ("mkimage: Align efi sections on 4k
>> > boundary") grubarm.efi crashes. Let's revert it.
>>
>> Everywhere or on a specific machines?
>
>I observed the issue on a TinkerBoard (Rockchip RK3288 CPU) and on
>qemu-arm-system -machine virt with the current Debian Buster GRUB.
Nod. I've seen similar in a qemu VM and on a Beaglebone Black. Leif's
aware and promised me he'd look when he's back from his travels.
--
Steve McIntyre, Cambridge, UK.st...@einval.com
"This dress doesn't reverse." -- Alden Spiess
___
Grub-devel mailing list
Grub-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/grub-devel
Re: [PATCH v3 2/2] Minimise writes to EFI variable storage
;
>+ rc = set_efi_variable (entry->name, entry);
>+ if (rc < 0)
>+goto err;
>+
>+ add_to_boot_order (order, (uint16_t) entry_num);
>+
>+ grub_util_info ("setting EFI variable BootOrder");
>+ rc = set_efi_variable ("BootOrder", order);
>+ if (rc < 0)
>+goto err;
>+
>+ return 0;
>+
>+err:
>+ show_efi_errors ();
>+ return errno;
>+}
>+
>+#endif /* HAVE_EFIVAR */
>diff --git a/grub-core/osdep/unix/platform.c b/grub-core/osdep/unix/platform.c
>index 55b8f4016..9d4530a38 100644
>--- a/grub-core/osdep/unix/platform.c
>+++ b/grub-core/osdep/unix/platform.c
>@@ -19,15 +19,12 @@
> #include
>
> #include
>-#include
> #include
> #include
> #include
> #include
> #include
>-#include
> #include
>-#include
>
> static char *
> get_ofpathname (const char *dev)
>@@ -78,102 +75,19 @@ get_ofpathname (const char *dev)
> dev);
> }
>
>-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 },
>);
>- char *line = NULL;
>- size_t len = 0;
>- int rc = 0;
>-
>- if (!pid)
>-{
>- grub_util_warn (_("Unable to open stream from %s: %s"),
>-"efibootmgr", strerror (errno));
>- return errno;
>-}
>-
>- FILE *fp = fdopen (fd, "r");
>- if (!fp)
>-{
>- grub_util_warn (_("Unable to open stream from %s: %s"),
>-"efibootmgr", strerror (errno));
>- return errno;
>-}
>-
>- line = xmalloc (80);
>- len = 80;
>- while (1)
>-{
>- int ret;
>- char *bootnum;
>- ret = getline (, , fp);
>- if (ret == -1)
>- break;
>- if (grub_memcmp (line, "Boot", sizeof ("Boot") - 1) != 0
>-|| line[sizeof ("Boot") - 1] < '0'
>-|| line[sizeof ("Boot") - 1] > '9')
>- continue;
>- if (!strcasestr (line, efi_distributor))
>- continue;
>- bootnum = line + sizeof ("Boot") - 1;
>- bootnum[4] = '\0';
>- if (!verbosity)
>- rc = grub_util_exec ((const char * []){ "efibootmgr", "-q",
>-"-b", bootnum, "-B", NULL });
>- else
>- rc = grub_util_exec ((const char * []){ "efibootmgr",
>-"-b", bootnum, "-B", NULL });
>-}
>-
>- free (line);
>- return rc;
>-}
>-
> 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;
>-
>- if (grub_util_exec_redirect_null ((const char * []){ "efibootmgr",
>"--version", NULL }))
>-{
>- /* TRANSLATORS: This message is shown when required executable `%s'
>- isn't found. */
>- grub_util_error (_("%s: not found"), "efibootmgr");
>-}
>-
>- /* On Linux, we need the efivars kernel modules. */
>-#ifdef __linux__
>- grub_util_exec ((const char * []){ "modprobe", "-q", "efivars", NULL });
>+#ifdef HAVE_EFIVAR
>+ return grub_install_efivar_register_efi (efidir_grub_dev, efifile_path,
>+ efi_distributor);
>+#else
>+ grub_util_error ("%s",
>+ _("GRUB was not built with efivar support; "
>+ "cannot register EFI boot entry"));
> #endif
>- /* Delete old entries from the same 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)
>-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
>-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 2631b1074..dcad16ac5 100644
>--- a/include/grub/util/install.h
>+++ b/include/grub/util/install.h
>@@ -216,6 +216,11 @@ grub_install_get_default_arm_platform (void);
> const char *
> grub_install_get_default_x86_platform (void);
>
>+int
>+grub_install_efivar_register_efi (grub_device_t efidir_grub_dev,
>+const char *efifile_path,
>+const char *efi_distributor);
>+
> int
> grub_install_register_efi (grub_device_t efidir_grub_dev,
> const char *efifile_path,
>diff --git a/util/grub-install.c b/util/grub-install.c
>index 264f9ecdc..a61df056e 100644
>--- a/util/grub-install.c
>+++ b/util/grub-install.c
>@@ -1885,7 +1885,7 @@ main (int argc, char *argv[])
>
> "\\System\\Library\\CoreServices",
> efi_distributor);
> if (ret)
>- grub_util_error (_("efibootmgr failed to register the boot
>entry: %s"),
>+ grub_util_error (_("failed to register the EFI boot entry: %s"),
>strerror (ret));
> }
>
>@@ -1929,7 +1929,7 @@ main (int argc, char *argv[])
> 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"),
>+ grub_util_error (_("failed to register the EFI boot entry: %s"),
>strerror (ret));
> }
> break;
Reviewed-by: Steve McIntyre <93...@debian.org>
--
Steve McIntyre, Cambridge, UK.st...@einval.com
< Aardvark> I dislike C++ to start with. C++11 just seems to be
handing rope-creating factories for users to hang multiple
instances of themselves.
___
Grub-devel mailing list
Grub-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/grub-devel
Re: [PATCH v3 1/2] Add %X to grub_vsnprintf_real and friends
Apologies for the delayed review - lots of travel at the moment... :-/
On Sat, Mar 23, 2019 at 01:42:08PM +, Colin Watson wrote:
>This is needed for UEFI Boot* variables, which the standard says are
>named using upper-case hexadecimal.
>
>Signed-off-by: Colin Watson
>---
> grub-core/kern/misc.c | 7 +--
> 1 file changed, 5 insertions(+), 2 deletions(-)
>
>diff --git a/grub-core/kern/misc.c b/grub-core/kern/misc.c
>index 3b633d51f..18cad5803 100644
>--- a/grub-core/kern/misc.c
>+++ b/grub-core/kern/misc.c
>@@ -588,7 +588,7 @@ grub_divmod64 (grub_uint64_t n, grub_uint64_t d,
>grub_uint64_t *r)
> static inline char *
> grub_lltoa (char *str, int c, unsigned long long n)
> {
>- unsigned base = (c == 'x') ? 16 : 10;
>+ unsigned base = (c == 'x' || c == 'X') ? 16 : 10;
> char *p;
>
> if ((long long) n < 0 && c == 'd')
>@@ -603,7 +603,7 @@ grub_lltoa (char *str, int c, unsigned long long n)
> do
> {
> unsigned d = (unsigned) (n & 0xf);
>- *p++ = (d > 9) ? d + 'a' - 10 : d + '0';
>+ *p++ = (d > 9) ? d + ((c == 'x') ? 'a' : 'A') - 10 : d + '0';
Yay, nested ternaries! Hardly the most readable code, but meh.
> }
> while (n >>= 4);
> else
>@@ -676,6 +676,7 @@ parse_printf_args (const char *fmt0, struct printf_args
>*args,
> {
> case 'p':
> case 'x':
>+ case 'X':
> case 'u':
> case 'd':
> case 'c':
>@@ -762,6 +763,7 @@ parse_printf_args (const char *fmt0, struct printf_args
>*args,
> switch (c)
> {
> case 'x':
>+ case 'X':
> case 'u':
> args->ptr[curn].type = UNSIGNED_INT + longfmt;
> break;
>@@ -900,6 +902,7 @@ grub_vsnprintf_real (char *str, grub_size_t max_len, const
>char *fmt0,
> c = 'x';
> /* Fall through. */
> case 'x':
>+ case 'X':
> case 'u':
> case 'd':
> {
Reviewed-by: Steve McIntyre <93...@debian.org>
--
Steve McIntyre, Cambridge, UK.st...@einval.com
"I suspect most samba developers are already technically insane... Of
course, since many of them are Australians, you can't tell." -- Linus Torvalds
___
Grub-devel mailing list
Grub-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/grub-devel
Re: [PATCH 2/2] Minimise writes to EFI variable storage
On Fri, Mar 22, 2019 at 11:29:15PM +, Colin Watson wrote: >On Wed, Mar 13, 2019 at 01:07:20AM +0000, Steve McIntyre wrote: >> On Mon, Mar 11, 2019 at 03:05:46PM +, Colin Watson wrote: >> >+/* Boot option attributes. */ >> >+#define LOAD_OPTION_ACTIVE 0x0001 >> >+ >> >+/* GUIDs. */ >> >+#define BLKX_UNKNOWN_GUID \ >> >+ EFI_GUID (0x47c7b225, 0xc42a, 0x11d2, 0x8e57, 0x00, 0xa0, 0xc9, 0x69, \ >> >+ 0x72, 0x3b) >> >> Ugh. I'm assuming the mahic numbers here are not exposed usefully by >> efivar or efiboot? > >Sadly not, as far as I can see. :-( That sounds like a clear bug, then. IMHO it's clearly part of the interface that they should be providing. >> So new_efi_variable() is using xmalloc() so it's safe if the >> allocation fails. But what happens if efi_get_variable() fails - do >> you need to free all the members by calling free_efi_variable() rather >> than simply free() here? > >Quite right, and there was a similar bug in another place. I'll send a >v2 fixing this and your comment about my grub_lltoa change. ACK, and then a v3. Will look again shortly... -- Steve McIntyre, Cambridge, UK.st...@einval.com Welcome my son, welcome to the machine. ___ Grub-devel mailing list Grub-devel@gnu.org https://lists.gnu.org/mailman/listinfo/grub-devel
Re: [PATCH 2/2] Minimise writes to EFI variable storage
fidir_disk, efidir_part,
>+efifile_path, efi_distributor);
>+ if (!entry)
>+goto err;
>+
>+ grub_util_info ("setting EFI variable %s", entry->name);
>+ rc = set_efi_variable (entry->name, entry);
>+ if (rc < 0)
>+goto err;
>+
>+ add_to_boot_order (order, (uint16_t) entry_num);
>+
>+ grub_util_info ("setting EFI variable BootOrder");
>+ rc = set_efi_variable ("BootOrder", order);
>+ if (rc < 0)
>+goto err;
>+
>+ return 0;
>+
>+err:
>+ show_efi_errors ();
>+ return errno;
>+}
>+
>+#endif /* HAVE_EFIVAR */
>diff --git a/grub-core/osdep/unix/platform.c b/grub-core/osdep/unix/platform.c
>index 55b8f4016..9d4530a38 100644
>--- a/grub-core/osdep/unix/platform.c
>+++ b/grub-core/osdep/unix/platform.c
>@@ -19,15 +19,12 @@
> #include
>
> #include
>-#include
> #include
> #include
> #include
> #include
> #include
>-#include
> #include
>-#include
>
> static char *
> get_ofpathname (const char *dev)
>@@ -78,102 +75,19 @@ get_ofpathname (const char *dev)
> dev);
> }
>
>-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 },
>);
>- char *line = NULL;
>- size_t len = 0;
>- int rc = 0;
>-
>- if (!pid)
>-{
>- grub_util_warn (_("Unable to open stream from %s: %s"),
>-"efibootmgr", strerror (errno));
>- return errno;
>-}
>-
>- FILE *fp = fdopen (fd, "r");
>- if (!fp)
>-{
>- grub_util_warn (_("Unable to open stream from %s: %s"),
>-"efibootmgr", strerror (errno));
>- return errno;
>-}
>-
>- line = xmalloc (80);
>- len = 80;
>- while (1)
>-{
>- int ret;
>- char *bootnum;
>- ret = getline (, , fp);
>- if (ret == -1)
>- break;
>- if (grub_memcmp (line, "Boot", sizeof ("Boot") - 1) != 0
>-|| line[sizeof ("Boot") - 1] < '0'
>-|| line[sizeof ("Boot") - 1] > '9')
>- continue;
>- if (!strcasestr (line, efi_distributor))
>- continue;
>- bootnum = line + sizeof ("Boot") - 1;
>- bootnum[4] = '\0';
>- if (!verbosity)
>- rc = grub_util_exec ((const char * []){ "efibootmgr", "-q",
>-"-b", bootnum, "-B", NULL });
>- else
>- rc = grub_util_exec ((const char * []){ "efibootmgr",
>-"-b", bootnum, "-B", NULL });
>-}
>-
>- free (line);
>- return rc;
>-}
>-
> 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;
>-
>- if (grub_util_exec_redirect_null ((const char * []){ "efibootmgr",
>"--version", NULL }))
>-{
>- /* TRANSLATORS: This message is shown when required executable `%s'
>- isn't found. */
>- grub_util_error (_("%s: not found"), "efibootmgr");
>-}
>-
>- /* On Linux, we need the efivars kernel modules. */
>-#ifdef __linux__
>- grub_util_exec ((const char * []){ "modprobe", "-q", "efivars", NULL });
>+#ifdef HAVE_EFIVAR
>+ return grub_install_efivar_register_efi (efidir_grub_dev, efifile_path,
>+ efi_distributor);
>+#else
>+ grub_util_error ("%s",
>+ _("GRUB was not built with efivar support; "
>+ "cannot register EFI boot entry"));
> #endif
>- /* Delete old entries from the same 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)
>-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
>-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 2631b1074..dcad16ac5 100644
>--- a/include/grub/util/install.h
>+++ b/include/grub/util/install.h
>@@ -216,6 +216,11 @@ grub_install_get_default_arm_platform (void);
> const char *
> grub_install_get_default_x86_platform (void);
>
>+int
>+grub_install_efivar_register_efi (grub_device_t efidir_grub_dev,
>+const char *efifile_path,
>+const char *efi_distributor);
>+
> int
> grub_install_register_efi (grub_device_t efidir_grub_dev,
> const char *efifile_path,
>diff --git a/util/grub-install.c b/util/grub-install.c
>index 264f9ecdc..a61df056e 100644
>--- a/util/grub-install.c
>+++ b/util/grub-install.c
>@@ -1885,7 +1885,7 @@ main (int argc, char *argv[])
>
> "\\System\\Library\\CoreServices",
> efi_distributor);
> if (ret)
>- grub_util_error (_("efibootmgr failed to register the boot
>entry: %s"),
>+ grub_util_error (_("failed to register the EFI boot entry: %s"),
>strerror (ret));
> }
>
>@@ -1929,7 +1929,7 @@ main (int argc, char *argv[])
> 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"),
>+ grub_util_error (_("failed to register the EFI boot entry: %s"),
>strerror (ret));
> }
> break;
>--
>2.17.1
>
--
Steve McIntyre, Cambridge, UK.st...@einval.com
"This dress doesn't reverse." -- Alden Spiess
___
Grub-devel mailing list
Grub-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/grub-devel
Re: [PATCH 1/2] Add %X to grub_vsnprintf_real and friends
On Mon, Mar 11, 2019 at 03:05:19PM +, Colin Watson wrote:
>This is needed for UEFI Boot* variables, which the standard says are
>named using upper-case hexadecimal.
>---
> grub-core/kern/misc.c | 10 --
> 1 file changed, 8 insertions(+), 2 deletions(-)
>
>diff --git a/grub-core/kern/misc.c b/grub-core/kern/misc.c
>index 3b633d51f..73f8e0e9e 100644
>--- a/grub-core/kern/misc.c
>+++ b/grub-core/kern/misc.c
>@@ -588,7 +588,7 @@ grub_divmod64 (grub_uint64_t n, grub_uint64_t d,
>grub_uint64_t *r)
> static inline char *
> grub_lltoa (char *str, int c, unsigned long long n)
> {
>- unsigned base = (c == 'x') ? 16 : 10;
>+ unsigned base = (c == 'x' || c == 'X') ? 16 : 10;
> char *p;
>
> if ((long long) n < 0 && c == 'd')
>@@ -603,7 +603,10 @@ grub_lltoa (char *str, int c, unsigned long long n)
> do
> {
> unsigned d = (unsigned) (n & 0xf);
>- *p++ = (d > 9) ? d + 'a' - 10 : d + '0';
>+ *p = (d > 9) ? d + 'a' - 10 : d + '0';
>+ if (c == 'X')
>+*p = grub_toupper (*p);
>+ p++;
I'd be more tempted to simply do the upper-case stuff using
if (c == 'x')
*p++ = (d > 9) ? d + 'a' - 10 : d + '0';
else
*p++ = (d > 9) ? d + 'A' - 10 : d + '0';
fi
rather than the call out to grub_toupper(), but that's just minor
nit-picking on style.
> }
> while (n >>= 4);
> else
>@@ -676,6 +679,7 @@ parse_printf_args (const char *fmt0, struct printf_args
>*args,
> {
> case 'p':
> case 'x':
>+ case 'X':
> case 'u':
> case 'd':
> case 'c':
>@@ -762,6 +766,7 @@ parse_printf_args (const char *fmt0, struct printf_args
>*args,
> switch (c)
> {
> case 'x':
>+ case 'X':
> case 'u':
> args->ptr[curn].type = UNSIGNED_INT + longfmt;
> break;
>@@ -900,6 +905,7 @@ grub_vsnprintf_real (char *str, grub_size_t max_len, const
>char *fmt0,
> c = 'x';
> /* Fall through. */
> case 'x':
>+ case 'X':
> case 'u':
> case 'd':
> {
>--
>2.17.1
Reviewed-by: Steve McIntyre <93...@debian.org>
--
Steve McIntyre, Cambridge, UK.st...@einval.com
"Because heaters aren't purple!" -- Catherine Pitt
___
Grub-devel mailing list
Grub-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/grub-devel
Re: [PATCH] util: Detect more I/O errors
On Wed, Feb 27, 2019 at 09:10:08AM +, Colin Watson wrote:
>Many of GRUB's utilities don't check anywhere near all the possible
>write errors. For example, if grub-install runs out of space when
>copying a file, it won't notice. There were missing checks for the
>return values of write, fflush, fsync, and close (or the equivalents on
>other OSes), all of which must be checked.
>
>I tried to be consistent with the existing logging practices of the
>various hostdisk implementations, but they weren't entirely consistent
>to start with so I used my judgement. The result at least looks
>reasonable on GNU/Linux when I provoke a write error:
>
> Installing for x86_64-efi platform.
> grub-install: error: cannot copy
> `/usr/lib/grub/x86_64-efi-signed/grubx64.efi.signed' to
> `/boot/efi/EFI/debian/grubx64.efi': No space left on device.
>
>There are more missing checks in other utilities, but this should fix
>the most critical ones.
>
>Fixes Debian bug #922741.
>---
> grub-core/osdep/aros/hostdisk.c| 38 ++
> grub-core/osdep/unix/hostdisk.c| 18 +++---
> grub-core/osdep/windows/hostdisk.c | 37 ++---
> include/grub/emu/hostfile.h| 4 ++--
> include/grub/emu/misc.h| 2 +-
> util/editenv.c | 3 ++-
> util/grub-editenv.c| 3 ++-
> util/grub-install-common.c | 16 +
> util/grub-mkimage.c| 8 +--
> util/setup.c | 6 +++--
> 10 files changed, 90 insertions(+), 45 deletions(-)
LGTM
Reviewed-by: Steve McIntyre <93...@debian.org>
>
>diff --git a/grub-core/osdep/aros/hostdisk.c b/grub-core/osdep/aros/hostdisk.c
>index 7d99b54b8..2be654ca3 100644
>--- a/grub-core/osdep/aros/hostdisk.c
>+++ b/grub-core/osdep/aros/hostdisk.c
>@@ -439,36 +439,44 @@ grub_util_get_fd_size (grub_util_fd_t fd,
> return -1;
> }
>
>-void
>+int
> grub_util_fd_close (grub_util_fd_t fd)
> {
> switch (fd->type)
> {
> case GRUB_UTIL_FD_FILE:
>- close (fd->fd);
>- return;
>+ return close (fd->fd);
> case GRUB_UTIL_FD_DISK:
> CloseDevice ((struct IORequest *) fd->ioreq);
> DeleteIORequest((struct IORequest *) fd->ioreq);
> DeleteMsgPort (fd->mp);
>- return;
>+ return 0;
> }
>+ return 0;
> }
>
> static int allow_fd_syncs = 1;
>
>-static void
>+static int
> grub_util_fd_sync_volume (grub_util_fd_t fd)
> {
>+ LONG err;
>+
> fd->ioreq->iotd_Req.io_Command = CMD_UPDATE;
> fd->ioreq->iotd_Req.io_Length = 0;
> fd->ioreq->iotd_Req.io_Data = 0;
> fd->ioreq->iotd_Req.io_Offset = 0;
> fd->ioreq->iotd_Req.io_Actual = 0;
>- DoIO ((struct IORequest *) fd->ioreq);
>+ err = DoIO ((struct IORequest *) fd->ioreq);
>+ if (err)
>+{
>+ grub_util_info ("I/O failed with error %d, IoErr=%d", (int)err, (int)
>IoErr ());
>+ return -1;
>+}
>+ return 0;
> }
>
>-void
>+int
> grub_util_fd_sync (grub_util_fd_t fd)
> {
> if (allow_fd_syncs)
>@@ -476,22 +484,22 @@ grub_util_fd_sync (grub_util_fd_t fd)
> switch (fd->type)
> {
> case GRUB_UTIL_FD_FILE:
>-fsync (fd->fd);
>-return;
>+return fsync (fd->fd);
> case GRUB_UTIL_FD_DISK:
>-grub_util_fd_sync_volume (fd);
>-return;
>+return grub_util_fd_sync_volume (fd);
> }
> }
>+ return 0;
> }
>
>-void
>+int
> grub_util_file_sync (FILE *f)
> {
>- fflush (f);
>+ if (fflush (f) != 0)
>+return -1;
> if (!allow_fd_syncs)
>-return;
>- fsync (fileno (f));
>+return 0;
>+ return fsync (fileno (f));
> }
>
> void
>diff --git a/grub-core/osdep/unix/hostdisk.c b/grub-core/osdep/unix/hostdisk.c
>index 5450cf416..91150969b 100644
>--- a/grub-core/osdep/unix/hostdisk.c
>+++ b/grub-core/osdep/unix/hostdisk.c
>@@ -177,20 +177,22 @@ grub_util_fd_strerror (void)
>
> static int allow_fd_syncs = 1;
>
>-void
>+int
> grub_util_fd_sync (grub_util_fd_t fd)
> {
> if (allow_fd_syncs)
>-fsync (fd);
>+return fsync (fd);
>+ return 0;
> }
>
>-void
>+int
> grub_util_file_sync (FILE *f)
> {
>- fflush (f);
>+ if (fflush (f) != 0)
>+return -1;
> if (!allow_fd_syncs)
>-return;
>- fsync (fileno (f));
>+return 0;
>+ return fsync (fileno (f));
> }
>
> void
>@@ -199,10 +201,10 @@ grub_util_disable_fd_syncs (void)
> allow_fd_syncs = 0;
> }
>
>-void
>+int
> grub_util_fd_close (grub_u
Re: [PATCH] grub-install: check for arm-efi as a default target
On Tue, Feb 26, 2019 at 03:55:47PM +0100, Daniel Kiper wrote: >On Tue, Feb 26, 2019 at 01:27:21PM +, Leif Lindholm wrote: >> >> As pointed out by Colin on IRC - it appears you have pushed Steves >> original submission rather than the revised one which I gave my R-b >> for. >> >> Could you address? > >Argh... Addressed. Sorry for the confusion. I took first email from >the thread by mistake. Folks, next time please send new patches as >not a reply to. Just create new threads. ACK, will do! -- Steve McIntyre, Cambridge, UK.st...@einval.com "You can't barbecue lettuce!" -- Ellie Crane ___ Grub-devel mailing list Grub-devel@gnu.org https://lists.gnu.org/mailman/listinfo/grub-devel
[PATCH] grub-install: check for arm-efi as a default target
Much like on x86, we can work out if the system is running on top of
EFI firmware. If so, return "arm-efi". If not, fall back to
"arm-uboot" as previously.
Split out the code to (maybe) load the efivar module and check for
/sys/firmware/efi into a common helper routine is_efi_system()
Signed-off-by: Steve McIntyre <93...@debian.org>
---
grub-core/osdep/basic/platform.c | 6 ++
grub-core/osdep/linux/platform.c | 46 +++-
include/grub/util/install.h | 3 +++
util/grub-install.c | 2 +-
4 files changed, 46 insertions(+), 11 deletions(-)
diff --git a/grub-core/osdep/basic/platform.c b/grub-core/osdep/basic/platform.c
index 4b5502aeb..a7dafd85a 100644
--- a/grub-core/osdep/basic/platform.c
+++ b/grub-core/osdep/basic/platform.c
@@ -19,6 +19,12 @@
#include
const char *
+grub_install_get_default_arm_platform (void)
+{
+ return "arm-uboot";
+}
+
+const char *
grub_install_get_default_x86_platform (void)
{
return "i386-pc";
diff --git a/grub-core/osdep/linux/platform.c b/grub-core/osdep/linux/platform.c
index 775b6c031..5cb7bf923 100644
--- a/grub-core/osdep/linux/platform.c
+++ b/grub-core/osdep/linux/platform.c
@@ -97,16 +97,19 @@ read_platform_size (void)
return ret;
}
-const char *
-grub_install_get_default_x86_platform (void)
-{
+/* Are we running on an EFI-based system? */
+static int
+is_efi_system (void)
+{
/*
- On Linux, we need the efivars kernel modules.
- If no EFI is available this module just does nothing
- besides a small hello and if we detect efi we'll load it
- anyway later. So it should be safe to
- try to load it here.
- */
+ Linux uses efivarfs (mounted on /sys/firmware/efi/efivars)
+ to access the EFI variable store.
+ Some legacy systems may still use the deprecated efivars
+ interface (accessed through /sys/firmware/efi/vars).
+ Where both are present, libefivar will use the former in
+ preference, so attempting to load efivars will not interfere
+ with later operations.
+ */
grub_util_exec_redirect_all ((const char * []){ "modprobe", "efivars", NULL
},
NULL, NULL, "/dev/null");
@@ -114,13 +117,36 @@ grub_install_get_default_x86_platform (void)
if (is_not_empty_directory ("/sys/firmware/efi"))
{
grub_util_info ("...found");
+ return 1;
+}
+ else
+{
+ grub_util_info ("... not found");
+ return 0;
+}
+}
+
+const char *
+grub_install_get_default_arm_platform (void)
+{
+ if (is_efi_system())
+return "arm-efi";
+ else
+return "arm-uboot";
+}
+
+const char *
+grub_install_get_default_x86_platform (void)
+{
+ if (is_efi_system())
+{
if (read_platform_size() == 64)
return "x86_64-efi";
else
return "i386-efi";
}
- grub_util_info ("... not found. Looking for /proc/device-tree ..");
+ grub_util_info ("Looking for /proc/device-tree ..");
if (is_not_empty_directory ("/proc/device-tree"))
{
grub_util_info ("...found");
diff --git a/include/grub/util/install.h b/include/grub/util/install.h
index af2bf65d7..80a51fcb1 100644
--- a/include/grub/util/install.h
+++ b/include/grub/util/install.h
@@ -209,6 +209,9 @@ void
grub_install_create_envblk_file (const char *name);
const char *
+grub_install_get_default_arm_platform (void);
+
+const char *
grub_install_get_default_x86_platform (void);
int
diff --git a/util/grub-install.c b/util/grub-install.c
index 4a0a66168..1d68cc5bb 100644
--- a/util/grub-install.c
+++ b/util/grub-install.c
@@ -319,7 +319,7 @@ get_default_platform (void)
#elif defined (__ia64__)
return "ia64-efi";
#elif defined (__arm__)
- return "arm-uboot";
+ return grub_install_get_default_arm_platform ();
#elif defined (__aarch64__)
return "arm64-efi";
#elif defined (__amd64__) || defined (__x86_64__) || defined (__i386__)
--
2.11.0
___
Grub-devel mailing list
Grub-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/grub-devel
Re: [PATCH] grub-install: check for arm-efi as a default target
Hey Leif,
On Thu, Feb 21, 2019 at 11:41:10AM +, Leif Lindholm wrote:
>On Mon, Feb 11, 2019 at 02:42:34AM +0000, Steve McIntyre wrote:
>> Much like on x86, we can work out if the system is running on top of
>> EFI firmware. If so, return "arm-efi". If not, fall back to
>> "arm-uboot" as previously.
>
>Right, this clearly needs a fix.
Yup!
>> Heavily inspired by the existing code for x86.
>
>Mmm. I would much prefer if we could break out the efi test in a
>separate helper function. And clean it up while we're at it.
ACK. I was looking for a quick change, but I'll be less lazy. :-)
...
>> const char *
>> +grub_install_get_default_arm_platform (void)
>> +{
>> + /*
>> + On Linux, we need the efivars kernel modules.
>> + If no EFI is available this module just does nothing
>> + besides a small hello and if we detect efi we'll load it
>> + anyway later. So it should be safe to
>> + try to load it here.
>> + */
>> + grub_util_exec_redirect_all ((const char * []){ "modprobe", "efivars",
>> NULL },
>> + NULL, NULL, "/dev/null");
>
>So, I guess this is a "safe" operation. But efivars isn't the
>interface that should be used these days - efivarfs is.
>The efivars library will always use efivarfs
>(mounted on /sys/firmware/efi/efivars) in preference to efivars
>(available through /sys/firmware/efi/vars).
>
>Since it's safe, we may leave it in for someone running bleeding edge
>grub on an ancient system, or just using a misconfigured kernel.
>But the comment is misleading. I would suggest changing it to
>something like:
>
> /*
> Linux uses efivarfs (mounted on /sys/firmware/efi/efivars)
> to access the EFI variable store.
> Some legacy systems may still use the deprecated efivars
> interface (accessed through /sys/firmware/efi/vars).
> Where both are present, libefivar will use the former in
> preference, so attempting to load efivars will not interfere
> with later operations.
> */
ACK. Using your text. :-)
Patch v2 on its way in a mo.
--
Steve McIntyre, Cambridge, UK.st...@einval.com
We don't need no education.
We don't need no thought control.
___
Grub-devel mailing list
Grub-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/grub-devel
Re: [PATCH] arm64/efi: fix grub_efi_get_ram_base()
On Thu, Feb 21, 2019 at 10:15:08AM +, Leif Lindholm wrote: >grub_efi_get_ram_base() looks for the lowest available RAM address by >traversing the memory map, comparing lowest address found so far. >Due to a brain glitch, that "so far" was initialized to GRUB_UINT_MAX - >completely preventing boot on systems without RAM below 4GB. > >Change the initial value to GRUB_EFI_MAX_USABLE_ADDRESS, as originally >intended. > >Reported-by: Steve McIntyre <93...@debian.org> >Signed-off-by: Leif Lindholm Tested-by: Steve McIntyre <93...@debian.org> >--- > grub-core/kern/efi/mm.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > >diff --git a/grub-core/kern/efi/mm.c b/grub-core/kern/efi/mm.c >index 42ad7c570..cfe9764b7 100644 >--- a/grub-core/kern/efi/mm.c >+++ b/grub-core/kern/efi/mm.c >@@ -653,7 +653,7 @@ grub_efi_get_ram_base(grub_addr_t *base_addr) > if (ret < 1) > return GRUB_ERR_BUG; > >- for (desc = memory_map, *base_addr = GRUB_UINT_MAX; >+ for (desc = memory_map, *base_addr = GRUB_EFI_MAX_USABLE_ADDRESS; >(grub_addr_t) desc < ((grub_addr_t) memory_map + memory_map_size); >desc = NEXT_MEMORY_DESCRIPTOR (desc, desc_size)) > if (desc->attribute & GRUB_EFI_MEMORY_WB) >-- >2.11.0 > > -- Steve McIntyre, Cambridge, UK.st...@einval.com "I've only once written 'SQL is my bitch' in a comment. But that code is in use on a military site..." -- Simon Booth ___ Grub-devel mailing list Grub-devel@gnu.org https://lists.gnu.org/mailman/listinfo/grub-devel
Re: [PATCH] grub-install: check for arm-efi as a default target
Ping?
On Mon, Feb 11, 2019 at 02:42:34AM +, Steve McIntyre wrote:
>Much like on x86, we can work out if the system is running on top of
>EFI firmware. If so, return "arm-efi". If not, fall back to
>"arm-uboot" as previously.
>
>Heavily inspired by the existing code for x86.
>
>Signed-off-by: Steve McIntyre <93...@debian.org>
>---
> grub-core/osdep/basic/platform.c | 6 ++
> grub-core/osdep/linux/platform.c | 24
> include/grub/util/install.h | 3 +++
> util/grub-install.c | 2 +-
> 4 files changed, 34 insertions(+), 1 deletion(-)
>
>diff --git a/grub-core/osdep/basic/platform.c
>b/grub-core/osdep/basic/platform.c
>index 4b5502aeb..a7dafd85a 100644
>--- a/grub-core/osdep/basic/platform.c
>+++ b/grub-core/osdep/basic/platform.c
>@@ -19,6 +19,12 @@
> #include
>
> const char *
>+grub_install_get_default_arm_platform (void)
>+{
>+ return "arm-uboot";
>+}
>+
>+const char *
> grub_install_get_default_x86_platform (void)
> {
> return "i386-pc";
>diff --git a/grub-core/osdep/linux/platform.c
>b/grub-core/osdep/linux/platform.c
>index 775b6c031..a3f9e9d28 100644
>--- a/grub-core/osdep/linux/platform.c
>+++ b/grub-core/osdep/linux/platform.c
>@@ -98,6 +98,30 @@ read_platform_size (void)
> }
>
> const char *
>+grub_install_get_default_arm_platform (void)
>+{
>+ /*
>+ On Linux, we need the efivars kernel modules.
>+ If no EFI is available this module just does nothing
>+ besides a small hello and if we detect efi we'll load it
>+ anyway later. So it should be safe to
>+ try to load it here.
>+ */
>+ grub_util_exec_redirect_all ((const char * []){ "modprobe", "efivars", NULL
>},
>+ NULL, NULL, "/dev/null");
>+
>+ grub_util_info ("Looking for /sys/firmware/efi ..");
>+ if (is_not_empty_directory ("/sys/firmware/efi"))
>+{
>+ grub_util_info ("...found");
>+ return "arm-efi";
>+}
>+
>+ grub_util_info ("... not found");
>+ return "arm-uboot";
>+}
>+
>+const char *
> grub_install_get_default_x86_platform (void)
> {
> /*
>diff --git a/include/grub/util/install.h b/include/grub/util/install.h
>index af2bf65d7..80a51fcb1 100644
>--- a/include/grub/util/install.h
>+++ b/include/grub/util/install.h
>@@ -209,6 +209,9 @@ void
> grub_install_create_envblk_file (const char *name);
>
> const char *
>+grub_install_get_default_arm_platform (void);
>+
>+const char *
> grub_install_get_default_x86_platform (void);
>
> int
>diff --git a/util/grub-install.c b/util/grub-install.c
>index 4a0a66168..1d68cc5bb 100644
>--- a/util/grub-install.c
>+++ b/util/grub-install.c
>@@ -319,7 +319,7 @@ get_default_platform (void)
> #elif defined (__ia64__)
>return "ia64-efi";
> #elif defined (__arm__)
>- return "arm-uboot";
>+ return grub_install_get_default_arm_platform ();
> #elif defined (__aarch64__)
>return "arm64-efi";
> #elif defined (__amd64__) || defined (__x86_64__) || defined (__i386__)
>--
>2.11.0
>
>
--
Steve McIntyre, Cambridge, UK.st...@einval.com
"I used to be the first kid on the block wanting a cranial implant,
now I want to be the first with a cranial firewall. " -- Charlie Stross
___
Grub-devel mailing list
Grub-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/grub-devel
[PATCH] grub-install: check for arm-efi as a default target
Much like on x86, we can work out if the system is running on top of
EFI firmware. If so, return "arm-efi". If not, fall back to
"arm-uboot" as previously.
Heavily inspired by the existing code for x86.
Signed-off-by: Steve McIntyre <93...@debian.org>
---
grub-core/osdep/basic/platform.c | 6 ++
grub-core/osdep/linux/platform.c | 24
include/grub/util/install.h | 3 +++
util/grub-install.c | 2 +-
4 files changed, 34 insertions(+), 1 deletion(-)
diff --git a/grub-core/osdep/basic/platform.c b/grub-core/osdep/basic/platform.c
index 4b5502aeb..a7dafd85a 100644
--- a/grub-core/osdep/basic/platform.c
+++ b/grub-core/osdep/basic/platform.c
@@ -19,6 +19,12 @@
#include
const char *
+grub_install_get_default_arm_platform (void)
+{
+ return "arm-uboot";
+}
+
+const char *
grub_install_get_default_x86_platform (void)
{
return "i386-pc";
diff --git a/grub-core/osdep/linux/platform.c b/grub-core/osdep/linux/platform.c
index 775b6c031..a3f9e9d28 100644
--- a/grub-core/osdep/linux/platform.c
+++ b/grub-core/osdep/linux/platform.c
@@ -98,6 +98,30 @@ read_platform_size (void)
}
const char *
+grub_install_get_default_arm_platform (void)
+{
+ /*
+ On Linux, we need the efivars kernel modules.
+ If no EFI is available this module just does nothing
+ besides a small hello and if we detect efi we'll load it
+ anyway later. So it should be safe to
+ try to load it here.
+ */
+ grub_util_exec_redirect_all ((const char * []){ "modprobe", "efivars", NULL
},
+ NULL, NULL, "/dev/null");
+
+ grub_util_info ("Looking for /sys/firmware/efi ..");
+ if (is_not_empty_directory ("/sys/firmware/efi"))
+{
+ grub_util_info ("...found");
+ return "arm-efi";
+}
+
+ grub_util_info ("... not found");
+ return "arm-uboot";
+}
+
+const char *
grub_install_get_default_x86_platform (void)
{
/*
diff --git a/include/grub/util/install.h b/include/grub/util/install.h
index af2bf65d7..80a51fcb1 100644
--- a/include/grub/util/install.h
+++ b/include/grub/util/install.h
@@ -209,6 +209,9 @@ void
grub_install_create_envblk_file (const char *name);
const char *
+grub_install_get_default_arm_platform (void);
+
+const char *
grub_install_get_default_x86_platform (void);
int
diff --git a/util/grub-install.c b/util/grub-install.c
index 4a0a66168..1d68cc5bb 100644
--- a/util/grub-install.c
+++ b/util/grub-install.c
@@ -319,7 +319,7 @@ get_default_platform (void)
#elif defined (__ia64__)
return "ia64-efi";
#elif defined (__arm__)
- return "arm-uboot";
+ return grub_install_get_default_arm_platform ();
#elif defined (__aarch64__)
return "arm64-efi";
#elif defined (__amd64__) || defined (__x86_64__) || defined (__i386__)
--
2.11.0
___
Grub-devel mailing list
Grub-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/grub-devel
Re: [PATCH] Make grub-install check for errors from efibootmgr
Apologies for the delayed response - just found this in the archives but for some reason I never got it in my inbox. Weird... :-/ On Tue, Feb 06, 2018 at 04:29:23PM +0100, Daniel Kiper wrote: >On Wed, Jan 31, 2018 at 09:49:36PM +0000, Steve McIntyre wrote: >> 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 this patch in Debian now for some time, with no ill > >s/this/similar/? If you are OK I will change that during commit. Yup, perfect. :-) >If there are no objections I will commit this in a week or so. Please... -- Steve McIntyre, Cambridge, UK.st...@einval.com We don't need no education. We don't need no thought control. ___ Grub-devel mailing list Grub-devel@gnu.org https://lists.gnu.org/mailman/listinfo/grub-devel
[PATCH] 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 this patch in Debian now for some time, with no ill
effects.
Signed-off-by: Steve McIntyre <93...@debian.org>
---
grub-core/osdep/unix/platform.c | 24 +++-
include/grub/util/install.h | 2 +-
util/grub-install.c | 18 +-
3 files changed, 29 insertions(+), 15 deletions(-)
diff --git a/grub-core/osdep/unix/platform.c b/grub-core/osdep/unix/platform.c
index a3fcfcaca..ca448bc11 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 },
);
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 5910b0c09..0dba8b67f 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 5e4cdfd2b..690f180c5 100644
--- a/util/grub-install.c
+++ b/util/grub-install.c
@@ -1848,9 +1848,13 @@ m
Re: [PATCH] Make grub-install check for errors from efibootmgr
On Tue, Jan 30, 2018 at 06:44:05PM +0100, Daniel Kiper wrote:
>On Mon, Jan 29, 2018 at 06:54:23PM +0000, Steve McIntyre wrote:
>>
>> diff --git a/grub-core/osdep/unix/platform.c
>> b/grub-core/osdep/unix/platform.c
>> index a3fcfcaca..b3a617e44 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 },
>> );
>>char *line = NULL;
>>size_t len = 0;
>> + int ret;
>>
>>if (!pid)
>> {
>>grub_util_warn (_("Unable to open stream from %s: %s"),
>>"efibootmgr", strerror (errno));
>> - return;
>> + return errno;
>> }
>>
>>FILE *fp = fdopen (fd, "r");
>> @@ -98,14 +99,13 @@ 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);
>>len = 80;
>>while (1)
>> {
>> - int ret;
>
>Oh, no, please do not do that here. If my first proposal conflicts with
>existing variables use second one. If second is bad please choose third.
>The rest of the patch LGTM.
OK, no problem. New version on its way in a sec.
--
Steve McIntyre, Cambridge, UK.st...@einval.com
You raise the blade, you make the change... You re-arrange me 'til I'm sane...
___
Grub-devel mailing list
Grub-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/grub-devel
[PATCH] 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 this patch in Debian now for some time, with no ill
effects.
Signed-off-by: Steve McIntyre <93...@debian.org>
---
grub-core/osdep/unix/platform.c | 25 +++--
include/grub/util/install.h | 2 +-
util/grub-install.c | 18 +-
3 files changed, 29 insertions(+), 16 deletions(-)
diff --git a/grub-core/osdep/unix/platform.c b/grub-core/osdep/unix/platform.c
index a3fcfcaca..b3a617e44 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 },
);
char *line = NULL;
size_t len = 0;
+ int ret;
if (!pid)
{
grub_util_warn (_("Unable to open stream from %s: %s"),
"efibootmgr", strerror (errno));
- return;
+ return errno;
}
FILE *fp = fdopen (fd, "r");
@@ -98,14 +99,13 @@ 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);
len = 80;
while (1)
{
- int ret;
char *bootnum;
ret = getline (, , fp);
if (ret == -1)
@@ -119,23 +119,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",
+ ret = grub_util_exec ((const char * []){ "efibootmgr", "-q",
"-b", bootnum, "-B", NULL });
else
- grub_util_exec ((const char * []){ "efibootmgr",
+ ret = grub_util_exec ((const char * []){ "efibootmgr",
"-b", bootnum, "-B", NULL });
}
free (line);
+ return ret;
}
-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 +153,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 5910b0c09..0dba8b67f 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/
Re: [PATCH] Make grub-install check for errors from efibootmgr
On Mon, Jan 29, 2018 at 06:57:51PM +0100, Daniel Kiper wrote:
>On Mon, Jan 29, 2018 at 02:04:54PM +0000, Steve McIntyre wrote:
>>
>> diff --git a/grub-core/osdep/unix/platform.c
>> b/grub-core/osdep/unix/platform.c
>> index a3fcfcaca..fdd57a027 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 },
>> );
>>char *line = NULL;
>>size_t len = 0;
>> + int error = 0;
>
>s/error/ret/ or s/error/rc/ or s/error/rv/ here and below please...
>In that order of preference...
ACK.
...
>> diff --git a/include/grub/util/install.h b/include/grub/util/install.h
>> index 5910b0c09..0dba8b67f 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 5e4cdfd2b..e783824ba 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,
>> + int error = 0;
>
>I think that you do not need this initialization.
ACK. Updated patch coming shortly.
--
Steve McIntyre, Cambridge, UK.st...@einval.com
The two hard things in computing:
* naming things
* cache invalidation
* off-by-one errors -- Stig Sandbeck Mathisen
___
Grub-devel mailing list
Grub-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/grub-devel
[PATCH] 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 this patch in Debian now for some time, with no ill
effects.
Signed-off-by: Steve McIntyre <93...@debian.org>
---
grub-core/osdep/unix/platform.c | 24 +++-
include/grub/util/install.h | 2 +-
util/grub-install.c | 14 +++---
3 files changed, 27 insertions(+), 13 deletions(-)
diff --git a/grub-core/osdep/unix/platform.c b/grub-core/osdep/unix/platform.c
index a3fcfcaca..fdd57a027 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 },
);
char *line = NULL;
size_t len = 0;
+ int error = 0;
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",
+ error = grub_util_exec ((const char * []){ "efibootmgr", "-q",
"-b", bootnum, "-B", NULL });
else
- grub_util_exec ((const char * []){ "efibootmgr",
+ error = grub_util_exec ((const char * []){ "efibootmgr",
"-b", bootnum, "-B", NULL });
}
free (line);
+ return error;
}
-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 error = 0;
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);
+ error = grub_install_remove_efi_entries_by_distributor (efi_distributor);
+ if (error)
+return error;
char *efidir_part_str = xasprintf ("%d", efidir_part);
if (!verbosity)
-grub_util_exec ((const char * []){ "efibootmgr", "-q",
+error = 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",
+error = 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 error;
}
void
diff --git a/include/grub/util/install.h b/include/grub/util/install.h
index 5910b0c09..0dba8b67f 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 5e4cdfd2b..e783824ba 100644
--- a/util/grub-install.c
+++ b/util/grub-inst
Re: [wea...@debian.org: Bug#858832: calls efibootmgr with invalid options]
On Tue, Mar 28, 2017 at 08:59:30PM +0300, Andrei Borzenkov wrote: >28.03.2017 09:34, Peter Palfrader пишет: >> } /dev/md2953M 176K 953M 1% /boot/efi > >Sorry, that's not going to work. Even assuming that grub can map from >Linux MD to underlying physical device + partition (as that is what >efibootmgr needs, we cannot simply pass /dev/md2 to it), this will >obviously fail unless metadata is 0.90 or 1.0 (and current mdadm default >is 1.2). And even if you are careful enough - EFI application is free to >write to ESP, so unless you have EFI driver for Linux MD it is no go to >try to mirror it, as you are never sure whether mirrors are identical >after boot. And if you have EFI driver for Linux MD you need to modify >efibootmgr to understand it. ACK, good catch. >We need to return better error message here to explain what happens >though, that's the actual bug. > >The (IMHO) correct way to handle ESP redundancy is to call "grub-install >/path/to/first/ESP /path/to/second/ESP". I have half-finished patch >allowing this, but it will need distributions cooperation to actually be >useful. More than happy to help with that - it's something I've been wanting for a while. -- Steve McIntyre, Cambridge, UK.st...@einval.com "I used to be the first kid on the block wanting a cranial implant, now I want to be the first with a cranial firewall. " -- Charlie Stross ___ Grub-devel mailing list Grub-devel@gnu.org https://lists.gnu.org/mailman/listinfo/grub-devel
Re: [PATCH] Make grub-install check for errors from efibootmgr
Hi folks, It would be nice to see this clear bug fixed for the upcoming release - any chance of getting it reviewed please? -- Steve McIntyre, Cambridge, UK.st...@einval.com You raise the blade, you make the change... You re-arrange me 'til I'm sane... ___ Grub-devel mailing list Grub-devel@gnu.org https://lists.gnu.org/mailman/listinfo/grub-devel
Re: [PATCH] Make grub-install check for errors from efibootmgr
On Thu, Feb 09, 2017 at 09:52:40PM +0300, Andrei Borzenkov wrote: >30.01.2017 22:04, Steve McIntyre пишет: >> 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! > >This looks more or less cosmetic to me. First, errors are displayed to >user so it is not that user is not aware. Maybe, maybe not - if this occurs in the middle of a set of package installations or upgrades on a system, text like this will get lost. >Second, efibootmgr is more or less optional. This is convenient but >by far not the only one possibility to use newly installed >grub. If you're running on a UEFI system, this is anything *but* optional. >Third, even successful execution of efibootmgr does not mean it will >actually boot grub - there are enough systems out there that will >simply ditch grub entry and replace it with hard coded Windows one. That's not an excuse for not catching errors on systems that *are* working. >So I'm fine with changing "no error reported" to "efibootmgr invocation >failed; check your firmware settings" or similar, but I am not sure we >need to abort grub-install in this case. What exact problem do you solve >by aborting? You pick up an error correctly, and report it upwards so that other programs calling grub-install can reliably check for errors, and maybe deal with those errors. I don't see why it's a problem to actually handle errors properly? In Debian we're seeing quite a few people reporting problems in this area. It would be better to catch and handle errors better here. See https://bugs.debian.org/852513 for an example. -- Steve McIntyre, Cambridge, UK.st...@einval.com You raise the blade, you make the change... You re-arrange me 'til I'm sane... ___ Grub-devel mailing list Grub-devel@gnu.org https://lists.gnu.org/mailman/listinfo/grub-devel
Re: [PATCH] Make grub-install check for errors from efibootmgr
Anybody interested in reviewing this?
On Mon, Jan 30, 2017 at 07:04:51PM +, Steve McIntyre wrote:
>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!
>
>Signed-off-by: Steve McIntyre <93...@debian.org>
>---
> grub-core/osdep/unix/platform.c | 24 +++-
> include/grub/util/install.h | 2 +-
> util/grub-install.c | 14 +++---
> 3 files changed, 27 insertions(+), 13 deletions(-)
>
>diff --git a/grub-core/osdep/unix/platform.c b/grub-core/osdep/unix/platform.c
>index 28cb37e..f9c376c 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 },
> );
> char *line = NULL;
> size_t len = 0;
>+ int error = 0;
>
> 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",
>+ error = grub_util_exec ((const char * []){ "efibootmgr", "-q",
> "-b", bootnum, "-B", NULL });
> else
>- grub_util_exec ((const char * []){ "efibootmgr",
>+ error = grub_util_exec ((const char * []){ "efibootmgr",
> "-b", bootnum, "-B", NULL });
> }
>
> free (line);
>+ return error;
> }
>
>-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 error = 0;
> 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);
>+ error = grub_install_remove_efi_entries_by_distributor (efi_distributor);
>+ if (error)
>+return error;
>
> char *efidir_part_str = xasprintf ("%d", efidir_part);
>
> if (!verbosity)
>-grub_util_exec ((const char * []){ "efibootmgr", "-q",
>+error = 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",
>+error = 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 error;
> }
>
> void
>diff --git a/include/grub/util/install.h b/include/grub/util/install.h
>index 9f517a1..58648e2 100644
>--- a/include/grub/util/install.h
>+++
[PATCH] 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!
Signed-off-by: Steve McIntyre <93...@debian.org>
---
grub-core/osdep/unix/platform.c | 24 +++-
include/grub/util/install.h | 2 +-
util/grub-install.c | 14 +++---
3 files changed, 27 insertions(+), 13 deletions(-)
diff --git a/grub-core/osdep/unix/platform.c b/grub-core/osdep/unix/platform.c
index 28cb37e..f9c376c 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 },
);
char *line = NULL;
size_t len = 0;
+ int error = 0;
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",
+ error = grub_util_exec ((const char * []){ "efibootmgr", "-q",
"-b", bootnum, "-B", NULL });
else
- grub_util_exec ((const char * []){ "efibootmgr",
+ error = grub_util_exec ((const char * []){ "efibootmgr",
"-b", bootnum, "-B", NULL });
}
free (line);
+ return error;
}
-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 error = 0;
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);
+ error = grub_install_remove_efi_entries_by_distributor (efi_distributor);
+ if (error)
+return error;
char *efidir_part_str = xasprintf ("%d", efidir_part);
if (!verbosity)
-grub_util_exec ((const char * []){ "efibootmgr", "-q",
+error = 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",
+error = 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 error;
}
void
diff --git a/include/grub/util/install.h b/include/grub/util/install.h
index 9f517a1..58648e2 100644
--- a/include/grub/util/install.h
+++ b/include/grub/util/install.h
@@ -209,7 +209,7 @@ grub_install_get_default_x86_platform (void);
const char *
grub_install_get_default_powerpc_machtype (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 d87d228..7a7e67e 100644
--- a/util/grub-install.c
+++ b/util/grub-install.c
@@ -2002,9 +2002,13 @@ main (int argc, char *argv[])
if (!removabl
Re: [PATCH] Add support for running a 64-bit Linux kernel on a 32-bit EFI
On Wed, Jan 28, 2015 at 11:42:14PM +, Steve McIntyre wrote: v2 coming shortly. Hi! Any comments on v2? -- Steve McIntyre, Cambridge, UK.st...@einval.com Google-bait: http://www.debian.org/CD/free-linux-cd Debian does NOT ship free CDs. Please do NOT contact the mailing lists asking us to send them to you. ___ Grub-devel mailing list Grub-devel@gnu.org https://lists.gnu.org/mailman/listinfo/grub-devel
Re: [PATCH] Add support for running a 64-bit Linux kernel on a 32-bit EFI
On Wed, Jan 28, 2015 at 06:42:01AM +0300, Andrei Borzenkov wrote:
В Wed, 28 Jan 2015 00:56:30 +
Steve McIntyre st...@einval.com пишет:
=
Some platforms might be capable of running a 64-bit Linux kernel but
only use a 32-bit EFI. To support such systems, it is necessary to
work out the size of the firmware rather than just the size of the
kernel. To enable that, there is now an extra EFI sysfs file to
describe the underlying firmware. Read that if possible, otherwise
fall back to the kernel type as before.
Signed-off-by: Steve McIntyre st...@einval.com
---
grub-core/osdep/linux/platform.c | 38
+-
1 file changed, 37 insertions(+), 1 deletion(-)
diff --git a/grub-core/osdep/linux/platform.c
b/grub-core/osdep/linux/platform.c
index 4b9f6ef..5668ae5 100644
--- a/grub-core/osdep/linux/platform.c
+++ b/grub-core/osdep/linux/platform.c
@@ -60,6 +60,42 @@ is_64_kernel (void)
return strcmp (un.machine, x86_64) == 0;
}
+static int
+read_platform_size (void)
+{
+ FILE *fp;
+ char *buf = NULL;
+ size_t len = 0;
+ int ret = 0;
+
+ /* Newer kernels can tell us directly about the size of the
+ * underlying firmware - let's see if that interface is there. */
+ fp = grub_util_fopen (/sys/firmware/efi/fw_platform_size, r);
+ if (fp != NULL)
+ {
+if (getline (buf, len, fp) 0)
size = 2
Yup.
+ {
+if (strncmp (buf, 32, 2) == 0)
+ ret = 32;
+else if (strncmp (buf, 64, 2) == 0)
+ ret = 64;
+ }
+free (buf);
+fclose (fp);
+ }
+
+ if (ret == 0)
+/* Unrecognised - fall back to matching the kernel size instead */
+{
I usually prefer comments inside braces and indented accordingly.
OK, cool.
Could you send it suitable for git am? Mentioning platforms in commit
message would be useful for reference. BTW, re subject - this is
about installing 32 bit EFI grub on 64 bit Linux, not running, right?
It already should be able to run kernel if installed appropriately
manually.
Correct, yes.
v2 coming shortly.
--
Steve McIntyre, Cambridge, UK.st...@einval.com
Welcome my son, welcome to the machine.
___
Grub-devel mailing list
Grub-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/grub-devel
[PATCH] Recognise a 64-bit Linux kernel on a 32-bit EFI firmware
Some x86 systems might be capable of running a 64-bit Linux kernel but
only use a 32-bit EFI (e.g. Intel Bay Trail systems). It's useful for
grub-install to be able to recognise such systems, to set the default
x86 platform correctly.
To allow grub-install to know the size of the firmware rather than
just the size of the kernel, there is now an extra EFI sysfs file to
describe the underlying firmware. Read that if possible, otherwise
fall back to the kernel type as before.
Signed-off-by: Steve McIntyre st...@einval.com
---
grub-core/osdep/linux/platform.c | 39 +-
1 file changed, 38 insertions(+), 1 deletion(-)
diff --git a/grub-core/osdep/linux/platform.c b/grub-core/osdep/linux/platform.c
index 4b9f6ef..775b6c0 100644
--- a/grub-core/osdep/linux/platform.c
+++ b/grub-core/osdep/linux/platform.c
@@ -60,6 +60,43 @@ is_64_kernel (void)
return strcmp (un.machine, x86_64) == 0;
}
+static int
+read_platform_size (void)
+{
+ FILE *fp;
+ char *buf = NULL;
+ size_t len = 0;
+ int ret = 0;
+
+ /* Newer kernels can tell us directly about the size of the
+ * underlying firmware - let's see if that interface is there. */
+ fp = grub_util_fopen (/sys/firmware/efi/fw_platform_size, r);
+ if (fp != NULL)
+ {
+if (getline (buf, len, fp) = 3) /* 2 digits plus newline */
+ {
+ if (strncmp (buf, 32, 2) == 0)
+ ret = 32;
+ else if (strncmp (buf, 64, 2) == 0)
+ ret = 64;
+ }
+free (buf);
+fclose (fp);
+ }
+
+ if (ret == 0)
+{
+ /* Unrecognised - fall back to matching the kernel size
+ * instead */
+ if (is_64_kernel ())
+ ret = 64;
+ else
+ ret = 32;
+}
+
+ return ret;
+}
+
const char *
grub_install_get_default_x86_platform (void)
{
@@ -77,7 +114,7 @@ grub_install_get_default_x86_platform (void)
if (is_not_empty_directory (/sys/firmware/efi))
{
grub_util_info (...found);
- if (is_64_kernel ())
+ if (read_platform_size() == 64)
return x86_64-efi;
else
return i386-efi;
--
1.7.10.4
___
Grub-devel mailing list
Grub-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/grub-devel
[PATCH] Add support for running a 64-bit Linux kernel on a 32-bit EFI
Hi folks,
I've been working in Debian on adding support for amd64 platforms
which are shipped with 32-bit UEFI firmware, such as the Asus X205TA
and other Bay Trail machines. As part of that, I've had a patch
accepted by the Linux EFI maintainers [1] to expose the size of the
underlying firmware to userland via a new sysfs file. It should make
mainline Linux very soon.
Using that patch as a basis, I've patched grub to use the new sysfs
interface to work out the correct default x86 platform to match the
firmware. Colin's happy with the patch and has just accepted it for
Debian's grub package for now; I promised to submit it upstream for
review, so here it is. :-)
Feedback welcome please!
[1] http://article.gmane.org/gmane.linux.kernel.efi/5229
=
Some platforms might be capable of running a 64-bit Linux kernel but
only use a 32-bit EFI. To support such systems, it is necessary to
work out the size of the firmware rather than just the size of the
kernel. To enable that, there is now an extra EFI sysfs file to
describe the underlying firmware. Read that if possible, otherwise
fall back to the kernel type as before.
Signed-off-by: Steve McIntyre st...@einval.com
---
grub-core/osdep/linux/platform.c | 38 +-
1 file changed, 37 insertions(+), 1 deletion(-)
diff --git a/grub-core/osdep/linux/platform.c b/grub-core/osdep/linux/platform.c
index 4b9f6ef..5668ae5 100644
--- a/grub-core/osdep/linux/platform.c
+++ b/grub-core/osdep/linux/platform.c
@@ -60,6 +60,42 @@ is_64_kernel (void)
return strcmp (un.machine, x86_64) == 0;
}
+static int
+read_platform_size (void)
+{
+ FILE *fp;
+ char *buf = NULL;
+ size_t len = 0;
+ int ret = 0;
+
+ /* Newer kernels can tell us directly about the size of the
+ * underlying firmware - let's see if that interface is there. */
+ fp = grub_util_fopen (/sys/firmware/efi/fw_platform_size, r);
+ if (fp != NULL)
+ {
+if (getline (buf, len, fp) 0)
+ {
+ if (strncmp (buf, 32, 2) == 0)
+ ret = 32;
+ else if (strncmp (buf, 64, 2) == 0)
+ ret = 64;
+ }
+free (buf);
+fclose (fp);
+ }
+
+ if (ret == 0)
+/* Unrecognised - fall back to matching the kernel size instead */
+{
+ if (is_64_kernel ())
+ ret = 64;
+ else
+ ret = 32;
+}
+
+ return ret;
+}
+
const char *
grub_install_get_default_x86_platform (void)
{
@@ -77,7 +113,7 @@ grub_install_get_default_x86_platform (void)
if (is_not_empty_directory (/sys/firmware/efi))
{
grub_util_info (...found);
- if (is_64_kernel ())
+ if (read_platform_size() == 64)
return x86_64-efi;
else
return i386-efi;
--
1.7.10.4
--
Steve McIntyre, Cambridge, UK.st...@einval.com
You can't barbecue lettuce! -- Ellie Crane
___
Grub-devel mailing list
Grub-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/grub-devel
Re: Bug#594967: Bug #594967: [poulsbo] grub-pc Hangs After Welcome to GRUB!
On Mon, Jan 03, 2011 at 12:13:01AM +, Colin Watson wrote: Whoops, I forgot to right-shift the header word. Can you try 4.iso instead, at the same location? I also made it handle PCI-to-CardBus bridges the same way as PCI-to-PCI bridges since that's what pciutils does. (In addition to 'set debug=pci', I'd recommend also doing 'set pager=1' so that lspci's output will be paged.) 4.iso: grub set debug=pci grub set pager=1 grub lspci bus/pci.c:92: bus 0x0 00:00.0 8086:8100 [0600] Host Bridge 00:02.0 8086:8108 [0300] VGA Controller 00:1b.0 8086:811b [0403] Multimedia device bus/pci.c:143: bridge range 0x2-0x2 00:1c.0 8086:8110 [0604] PCI-PCI Bridge bus/pci.c:143: bridge range 0x3-0x3 00:1c.1 8086:8112 [0604] PCI-PCI Bridge 00:1d.0 8086:8114 [0c03] USB Controller 00:1d.1 8086:8115 [0c03] USB Controller 00:1d.2 8086:8116 [0c03] USB Controller 00:1d.7 8086:8117 [0c03] USB Controller [PI 20] 00:1f.0 8086:8119 [0601] ISA Bridge 00:1f.1 8086:811a [0101] IDE Controller [PI 80] bus/pci.c:92: bus 0x2 02:00.0 10ec:8136 [0200] Ethernet Controller bus/pci.c:92: bus 0x3 03:00.0 168c:001c [0200] Ethernet Controller grub -- Steve McIntyre, Cambridge, UK.st...@einval.com Because heaters aren't purple! -- Catherine Pitt ___ Grub-devel mailing list Grub-devel@gnu.org http://lists.gnu.org/mailman/listinfo/grub-devel
Re: Bug#594967: Bug #594967: [poulsbo] grub-pc Hangs After Welcome to GRUB!
On Tue, Jan 04, 2011 at 12:46:54PM +, Colin Watson wrote: On Tue, Jan 04, 2011 at 12:06:54AM +, Colin Watson wrote: On Mon, Jan 03, 2011 at 12:13:01AM +, Colin Watson wrote: 2011-01-02 Colin Watson cjwat...@ubuntu.com * grub-core/bus/pci.c (grub_pci_iterate): Skip remaining functions on devices that do not implement function 0. I've applied this patch to trunk following an ack from Vladimir on IRC. I'll prepare an updated package for unstable shortly. Uploading now. I'd appreciate confirmation from affected folks that this is enough to make things boot. If it is, we can perhaps avoid worrying about the rest; otherwise, we'll have to get more creative. Just tested on Jo's laptop now, and the new version works exactly as hoped. Thanks for the quick fix! Jo says many thankyous too. :-) -- Steve McIntyre, Cambridge, UK.st...@einval.com Since phone messaging became popular, the young generation has lost the ability to read or write anything that is longer than one hundred and sixty characters. -- Ignatios Souvatzis ___ Grub-devel mailing list Grub-devel@gnu.org http://lists.gnu.org/mailman/listinfo/grub-devel
