diff mbox series

[1/1] boot/grub2: add '--memdisk' option to grub-makeimage

Message ID 20200309095540.15448-1-skif@skif-web.ru
State Changes Requested
Headers show
Series [1/1] boot/grub2: add '--memdisk' option to grub-makeimage | expand

Commit Message

Alexey Lukyanchuk March 9, 2020, 9:55 a.m. UTC
The memdisk is a virtual disk device viewed by grub.
Memdisk can store some files(pictures, gpg key for verify module,etc), grub modules, into a core.img.
In UEFI platform, grub-mkimage does not pose a limit on the size of memdisk.
It's good solution for secure-boot or embedded systems

Signed-off-by: Alexey Lukyanchuk <skif@skif-web.ru>
---
 boot/grub2/Config.in |  6 ++++++
 boot/grub2/grub2.mk  | 11 +++++++++++
 2 files changed, 17 insertions(+)

Comments

Yann E. MORIN Oct. 24, 2021, 8:45 a.m. UTC | #1
Alexey, All,

Thank you for your contribution. I am so sorry that we did not look at
your patch earlier. Recent evolutions (by Köry, added in Cc) in grub
had me dig out the old patches we still had pending about grub. So if
you are still interested, here is a review of your patch.

On 2020-03-09 12:55 +0300, Alexey Lukyanchuk spake thusly:
> The memdisk is a virtual disk device viewed by grub.
> Memdisk can store some files(pictures, gpg key for verify module,etc), grub modules, into a core.img.

Commit log should be wrapped at 72 chars, as is standard practice with
git. I can fix that when applying, though.

> In UEFI platform, grub-mkimage does not pose a limit on the size of memdisk.

Does that mean there is a limit when booting in legacy BIOS? What is
that limit? Should we enforce/validate it when building the grub image?

> It's good solution for secure-boot or embedded systems

So, I was a bit curious about memdisk, and how one was supposed to use
it. But as [0] puts it, this features sorely lacks documentation. I was
only able to find a quick reference to memdisk in the manual [1] when
dealing about the naming of devices. And as usual, StackOverflow to the
rescue [2]. And then another example to setup LUKS on Debian [3].

It might be a good idea to add those 4 references to the commit log (or
the help text too, maybe):

[0] https://phip1611.de/blog/adding-and-using-files-to-from-the-grub-memdisk-grub-standalone/
[1] https://www.gnu.org/software/grub/manual/grub/grub.html#Device-syntax
[2] https://unix.stackexchange.com/questions/253657/actual-usage-of-grub-mkimage-config/273334#273334
[3] https://cryptsetup-team.pages.debian.net/cryptsetup/encrypted-boot.html

> Signed-off-by: Alexey Lukyanchuk <skif@skif-web.ru>
> ---
>  boot/grub2/Config.in |  6 ++++++
>  boot/grub2/grub2.mk  | 11 +++++++++++
>  2 files changed, 17 insertions(+)
> 
> diff --git a/boot/grub2/Config.in b/boot/grub2/Config.in
> index e45133999e..6ee2f351d9 100644
> --- a/boot/grub2/Config.in
> +++ b/boot/grub2/Config.in
> @@ -107,6 +107,12 @@ config BR2_TARGET_GRUB2_BUILTIN_CONFIG
>  	  device and other configuration parameters, but however menu
>  	  entries cannot be described in this embedded configuration.
>  
> +config BR2_TARGET_GRUB2_MEMDISK_DIR
> +	string "memdisk"
> +	help
> +	  Path to directory that will be a memdisk (-m option in 
> +	  grub2-mkimage)

It would be nice to have just a little blurb on how to use files from a
memdisk; maybe something like (if I understood all the above references
correctly):

    For a file BR2_TARGET_GRUB2_MEMDISK_DIR/boot/grub/foo, it can
    be accessed from grub with: (memdisk)/foo

So, to avoid any confusion or runtime issue, maybe we should enforce
that all files are in BR2_TARGET_GRUB2_MEMDISK_DIR/boot/grub/ (see
below [4]).

>  config BR2_TARGET_GRUB2_INSTALL_TOOLS
>  	bool "install tools"
>  	help
> diff --git a/boot/grub2/grub2.mk b/boot/grub2/grub2.mk
> index a202525d71..0627c6857f 100644
> --- a/boot/grub2/grub2.mk
> +++ b/boot/grub2/grub2.mk
> @@ -22,6 +22,7 @@ endif
>  GRUB2_BUILTIN_MODULES = $(call qstrip,$(BR2_TARGET_GRUB2_BUILTIN_MODULES))
>  GRUB2_BUILTIN_CONFIG = $(call qstrip,$(BR2_TARGET_GRUB2_BUILTIN_CONFIG))
>  GRUB2_BOOT_PARTITION = $(call qstrip,$(BR2_TARGET_GRUB2_BOOT_PARTITION))
> +GRUB2_MEMDISK_DIR = $(call qstrip,$(BR2_TARGET_GRUB2_MEMDISK_DIR))
>  
>  ifeq ($(BR2_TARGET_GRUB2_I386_PC),y)
>  GRUB2_IMAGE = $(BINARIES_DIR)/grub.img
> @@ -87,6 +88,15 @@ GRUB2_CONF_ENV = \
>  	TARGET_OBJCOPY="$(TARGET_OBJCOPY)" \
>  	TARGET_STRIP="$(TARGET_CROSS)strip"
>  
> +ifneq ($(GRUB2_MEMDISK_DIR),)
> +GRUB2_POST_BUILD_HOOKS += GRUB2_MAKE_MEMDISK
> +define GRUB2_MAKE_MEMDISK
> +	mkdir -p $(@D)/grub2_memdisk
> +	cp -a $(GRUB2_MEMDISK_DIR)/* $(@D)/grub2_memdisk/
> +	tar -cvf $(@D)/memdisk.tar -C $(@D)/grub2_memdisk/ .

[4] here we would enforce the prefix:

    mkdir -p $(@D)/grub2_memdisk/boot/grub
    cp -a $(GRUB2_MEMDISK_DIR)/* $(@D)/grub2_memdisk/boot/grub/
    tar -cvf $(@D)/memdisk.tar -C $(@D)/grub2_memdisk/ .

(if we do that, then we must also propagate that top the help text).

Additionally, generating a tarball like that is not reproducible; we do
have a tar helper that generates reproducible archives, but it is a
shell function [5]... Maybe we should make that an actual tar wrapper...
But the memdisk undocumentation speaks about "GNU tar format", so not
sure our reproducible archives would be usable...

Also, this requires that both the 'memdisk' and 'tarfs' modules be built
into the grub image. Maybe we should validate that they are in the list?
Or that we should just append them if not?

[5] https://git.buildroot.org/buildroot/tree/support/download/helpers

> +endef
> +endif
> +
>  GRUB2_CONF_OPTS = \
>  	--target=$(GRUB2_TARGET) \
>  	--with-platform=$(GRUB2_PLATFORM) \
> @@ -121,6 +131,7 @@ define GRUB2_INSTALL_IMAGES_CMDS
>  		-O $(GRUB2_TUPLE) \
>  		-o $(GRUB2_IMAGE) \
>  		-p "$(GRUB2_PREFIX)" \
> +		$(if $(BR2_TARGET_GRUB2_MEMDISK_DIR),-m $(@D)/memdisk.tar) \

BR2_TARGET_GRUB2_MEMDISK_DIR is a Kconfig string, so it is
double-quoted, so it is never empty for make. So the test should be
against GRUB2_MEMDISK_DIR, like you did a few lines before.

However, as per grub-mkimage manpage, this overrides any previous -p
option (but later -p options can again override it). Not sure how that
would work with our passing of -p, just before...

Anyway, this looks like a very interesting feature which we would like
to support in Buildroot (I know of some people that would like to have
it to display a splashscreen, for example).

Again, sorry for the delay it took us to look at your patch, but as you
can see, there is quite some food on the table:
    https://patchwork.ozlabs.org/project/buildroot/list/

Regards,
Yann E. MORIN.

>  		$(if $(GRUB2_BUILTIN_CONFIG),-c $(GRUB2_BUILTIN_CONFIG)) \
>  		$(GRUB2_BUILTIN_MODULES)
>  	mkdir -p $(dir $(GRUB2_CFG))
Kory Maincent Nov. 30, 2021, 9:21 a.m. UTC | #2
Hello Alexey,

On Thu, 25 Nov 2021 15:01:19 +0300
Alexey Lukyanchuk <skif@skif-web.ru> wrote:

> Yann, All,
> 
> I got serious trouble with arch creation.  Buildroot can use only simple 
> tar without compression. So, we can not use buildroot helper. May be I 
> can write special function fo it?

Are you sure about that?
The host-tar package seems to be able to compress an archive. 

> 
> And where did you find "tarfs" module?

This module is build like the other module in the
build-$tuple/grub-core/tar.module

> 
> "Also, this requires that both the 'memdisk' and 'tarfs' modules be built
> into the grub image. Maybe we should validate that they are in the list?
> Or that we should just append them if not?"

Maybe just edit the default list of module selected:
default "... memdisk tar" if BR2_TARGET_GRUB2_MEMDISK

Or append these two modules to the list of installed modules when the MEMDISK
option is selected.

Regards,

Köry
diff mbox series

Patch

diff --git a/boot/grub2/Config.in b/boot/grub2/Config.in
index e45133999e..6ee2f351d9 100644
--- a/boot/grub2/Config.in
+++ b/boot/grub2/Config.in
@@ -107,6 +107,12 @@  config BR2_TARGET_GRUB2_BUILTIN_CONFIG
 	  device and other configuration parameters, but however menu
 	  entries cannot be described in this embedded configuration.
 
+config BR2_TARGET_GRUB2_MEMDISK_DIR
+	string "memdisk"
+	help
+	  Path to directory that will be a memdisk (-m option in 
+	  grub2-mkimage)
+
 config BR2_TARGET_GRUB2_INSTALL_TOOLS
 	bool "install tools"
 	help
diff --git a/boot/grub2/grub2.mk b/boot/grub2/grub2.mk
index a202525d71..0627c6857f 100644
--- a/boot/grub2/grub2.mk
+++ b/boot/grub2/grub2.mk
@@ -22,6 +22,7 @@  endif
 GRUB2_BUILTIN_MODULES = $(call qstrip,$(BR2_TARGET_GRUB2_BUILTIN_MODULES))
 GRUB2_BUILTIN_CONFIG = $(call qstrip,$(BR2_TARGET_GRUB2_BUILTIN_CONFIG))
 GRUB2_BOOT_PARTITION = $(call qstrip,$(BR2_TARGET_GRUB2_BOOT_PARTITION))
+GRUB2_MEMDISK_DIR = $(call qstrip,$(BR2_TARGET_GRUB2_MEMDISK_DIR))
 
 ifeq ($(BR2_TARGET_GRUB2_I386_PC),y)
 GRUB2_IMAGE = $(BINARIES_DIR)/grub.img
@@ -87,6 +88,15 @@  GRUB2_CONF_ENV = \
 	TARGET_OBJCOPY="$(TARGET_OBJCOPY)" \
 	TARGET_STRIP="$(TARGET_CROSS)strip"
 
+ifneq ($(GRUB2_MEMDISK_DIR),)
+GRUB2_POST_BUILD_HOOKS += GRUB2_MAKE_MEMDISK
+define GRUB2_MAKE_MEMDISK
+	mkdir -p $(@D)/grub2_memdisk
+	cp -a $(GRUB2_MEMDISK_DIR)/* $(@D)/grub2_memdisk/
+	tar -cvf $(@D)/memdisk.tar -C $(@D)/grub2_memdisk/ .
+endef
+endif
+
 GRUB2_CONF_OPTS = \
 	--target=$(GRUB2_TARGET) \
 	--with-platform=$(GRUB2_PLATFORM) \
@@ -121,6 +131,7 @@  define GRUB2_INSTALL_IMAGES_CMDS
 		-O $(GRUB2_TUPLE) \
 		-o $(GRUB2_IMAGE) \
 		-p "$(GRUB2_PREFIX)" \
+		$(if $(BR2_TARGET_GRUB2_MEMDISK_DIR),-m $(@D)/memdisk.tar) \
 		$(if $(GRUB2_BUILTIN_CONFIG),-c $(GRUB2_BUILTIN_CONFIG)) \
 		$(GRUB2_BUILTIN_MODULES)
 	mkdir -p $(dir $(GRUB2_CFG))