[PATCH] grub-install: force journal draining to ensure data integrity

2020-04-29 Thread Michael Chang 
In XFS, the system would end up in unbootable state if an abrupt power
off after grub-install is occuring. It can be easily reproduced with.

  grub-install /dev/vda; reboot -f

The grub error would show many different kinds of corruption in
filesystem and the problem boils down to incompleted journal transaction
which would leave pending writes behind in the on-disk journal. It is
therefore necessary to recover the system via re-mounting the filesystem
from linux system that all pending journal log can be replayed.

On the other hand if journal draining can be enforced by grub-install
then it can bring more resilience to such abrupt power loss. The fsync
is not enough here for XFS, because that only writes in-memory log to
on-disk (ie makes sure broken state can be repaired). Unfortunately
there's no designated system call to serve solely for the journal
draining, so it can only be achieved via fsfreeze system call that the
journal draining can happen as a byproduct during the process.

This patch adds fsfreeze/unfreeze at the end of grub-install to induce
journal draining on journaled file system. However btrfs is excluded
from the list as it is using fsync to drain journal and also is not
desired as reportedly having negative side effect. With this patch
applied, the boot falilure can no longer be reproduced with above
procedure.

Signed-off-by: Michael Chang 
---
 Makefile.util.def|  1 +
 grub-core/osdep/basic/journaled_fs.c | 26 +++
 grub-core/osdep/journaled_fs.c   |  5 
 grub-core/osdep/linux/journaled_fs.c | 48 
 include/grub/util/install.h  |  2 ++
 util/grub-install.c  | 20 +++
 6 files changed, 102 insertions(+)
 create mode 100644 grub-core/osdep/basic/journaled_fs.c
 create mode 100644 grub-core/osdep/journaled_fs.c
 create mode 100644 grub-core/osdep/linux/journaled_fs.c

diff --git a/Makefile.util.def b/Makefile.util.def
index d9e2bd84d..8461f7b20 100644
--- a/Makefile.util.def
+++ b/Makefile.util.def
@@ -652,6 +652,7 @@ program = {
   common = util/resolve.c;
   common = grub-core/kern/emu/argp_common.c;
   common = grub-core/osdep/init.c;
+  common = grub-core/osdep/journaled_fs.c;
 
   ldadd = '$(LIBLZMA)';
   ldadd = libgrubmods.a;
diff --git a/grub-core/osdep/basic/journaled_fs.c 
b/grub-core/osdep/basic/journaled_fs.c
new file mode 100644
index 0..9d8245704
--- /dev/null
+++ b/grub-core/osdep/basic/journaled_fs.c
@@ -0,0 +1,26 @@
+/*
+ *  GRUB  --  GRand Unified Bootloader
+ *  Copyright (C) 2010,2011,2012,2013  Free Software Foundation, Inc.
+ *
+ *  GRUB is free software: you can redistribute it and/or modify
+ *  it under the terms of the GNU General Public License as published by
+ *  the Free Software Foundation, either version 3 of the License, or
+ *  (at your option) any later version.
+ *
+ *  GRUB is distributed in the hope that it will be useful,
+ *  but WITHOUT ANY WARRANTY; without even the implied warranty of
+ *  MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ *  GNU General Public License for more details.
+ *
+ *  You should have received a copy of the GNU General Public License
+ *  along with GRUB.  If not, see .
+ */
+
+#include 
+
+int
+grub_install_sync_fs_journal (const char *path)
+{
+  return 1;
+}
+
diff --git a/grub-core/osdep/journaled_fs.c b/grub-core/osdep/journaled_fs.c
new file mode 100644
index 0..fa5d0a199
--- /dev/null
+++ b/grub-core/osdep/journaled_fs.c
@@ -0,0 +1,5 @@
+#ifdef __linux__
+#include "linux/journaled_fs.c"
+#else
+#include "basic/journaled_fs.c"
+#endif
diff --git a/grub-core/osdep/linux/journaled_fs.c 
b/grub-core/osdep/linux/journaled_fs.c
new file mode 100644
index 0..0fa8360b1
--- /dev/null
+++ b/grub-core/osdep/linux/journaled_fs.c
@@ -0,0 +1,48 @@
+/*
+ *  GRUB  --  GRand Unified Bootloader
+ *  Copyright (C) 2010,2011,2012,2013  Free Software Foundation, Inc.
+ *
+ *  GRUB is free software: you can redistribute it and/or modify
+ *  it under the terms of the GNU General Public License as published by
+ *  the Free Software Foundation, either version 3 of the License, or
+ *  (at your option) any later version.
+ *
+ *  GRUB is distributed in the hope that it will be useful,
+ *  but WITHOUT ANY WARRANTY; without even the implied warranty of
+ *  MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ *  GNU General Public License for more details.
+ *
+ *  You should have received a copy of the GNU General Public License
+ *  along with GRUB.  If not, see .
+ */
+
+#include 
+#include 
+#include 
+#include 
+#include 
+
+int
+grub_install_sync_fs_journal (const char *path)
+{
+  int fd, ret;
+
+  fd = open (path, O_RDONLY);
+
+  if (fd == -1)
+return 1;
+
+  if (ioctl (fd, FIFREEZE, 0) == 0)
+{
+  ioctl(fd, FITHAW, 0);
+  ret = 1;
+}
+  else if (errno == EOPNOTSUPP)
+ret = 1;
+  else
+

Re: Feature request: Boot default subvolume in BTRFS

2020-04-29 Thread Michael Chang 
On Tue, Apr 28, 2020 at 03:59:53PM +0200, Thomas wrote:
> Hello,
> 
> this is the description of my feature request:
> Grub is booting the default subvolume in BTRFS w/o any manual
> modification in Grub configuration.
> 
> Current situation:
> Booting a snapshot created with Snapper
>  is not possible w/o
> modifying the Grub entries.
> This is related to the Snapper functionality that creates a ro snapshot
> that must be booted and then creates another rw snapshot of the
> currently booted ro snapshot. Then Grub configuration must be recreated
> to reflect the newly created rw snapshot.
> 
> Available solution:
> In OpenSuse Grub is always booting the default snapshot and hereby Grub
> configuration must not be adjusted.
> To my understanding OpenSuse uses a patched Grub.

I will send those patches to the mailing list once they are polished for
review. 

Thanks,
Michael

> 
> Regards
> Thomas

> ___
> Grub-devel mailing list
> Grub-devel@gnu.org
> https://lists.gnu.org/mailman/listinfo/grub-devel


___
Grub-devel mailing list
Grub-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/grub-devel

Re: [PATCH RFC/RFT 0/3] Add grub loader support for RISC-V Linux

2020-04-29 Thread Leif Lindholm 
Hi Atish,

On Tue, Apr 28, 2020 at 11:21:05 -0700, Atish Patra wrote:
> > > Hello Ard,
> > >
> > > Did I misunderstand you and you want to provide a LOAD_FILE2
> > > implementation in GRUB and not use the one in the firmware?
> >
> > Yes. If you use GRUB, it is provided by GRUB. Otherwise, it can be
> > provided by U-Boot or EDK2.
> 
> Thanks for the discussion. I got clarification as well.
> I will work on adding LOAD_FILE2 support in GRUB.

I had promised to look into that, but my paperwork with the FSF is
still pending. Feel free to contact me for bouncing ideas or whatever.

As for this set - the changes to the kernel header structs and naming
should be non-controversial, and will be required for a final unified
loader anyway. Could you possibly break those out as a separate set,
which could then be merged earlier?

Best Regards,

Leif

___
Grub-devel mailing list
Grub-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/grub-devel

[PATCH v2] tests: Don't run f2fs test on systems with PAGE_SIZE more than 4096 bytes

2020-04-29 Thread Anatoly Pugachev 
Don't run f2fs test on systems with PAGE_SIZE > 4096 bytes, since f2fs is not
supported on these systems and trying to mount a f2fs filesystem would fail.

v2 changes:

- fix compare
- quotes around variable expansion

Signed-off-by: Anatoly Pugachev 
Reviewed-by: Mike Gilbert 

---
 tests/f2fs_test.in | 6 ++
 1 file changed, 6 insertions(+)

diff --git a/tests/f2fs_test.in b/tests/f2fs_test.in
index 1ea77c826..3da1dad57 100644
--- a/tests/f2fs_test.in
+++ b/tests/f2fs_test.in
@@ -15,5 +15,11 @@ if ! which mkfs.f2fs >/dev/null 2>&1; then
  exit 77
 fi
 
+PAGE_SIZE=$(getconf PAGE_SIZE)
+F2FS_BLKSIZE=4096
+if [ "$PAGE_SIZE" -gt "$F2FS_BLKSIZE" ]; then
+ printf "f2fs is not supported on PAGE_SIZE(%d) != %d\n" $PAGE_SIZE 
$F2FS_BLKSIZE
+ exit 77
+fi
 
 "@builddir@/grub-fs-tester" f2fs
-- 
2.26.2


___
Grub-devel mailing list
Grub-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/grub-devel

[PATCH 2/2] core: commands: efi: add commands to get/set EFI vars

2020-04-29 Thread Flavio Suligoi 
Sometimes, using a GRUB boot script (i.e. grub.cfg), it is very important
to get/set an EFI variable, especially in embedded systems, to detect
some custom BIOS features.
For example, based on the content of some EFI variables, a script can decide
to boot a special kernel instead of the usual kernel, or add some custom boot
parameters to the kernel command line, etc.

This patch add two useful commands:

- efivar_get
- efivar_set

to get/set an EFI variable.

The "get" function can display the EFI variable in ASCII or in HEX+ASCII
mode (like the Linux command "hexdump"), while the "set" function can set the
EFI variable in ASCII mode only.

As GUID, the default GUID used is the standard EFI_GLOBAL_VARIABLE_GUID,
but it is possible to use any other GUID (standard or custom) using an
environment variable.

Example: set the EFI variable "Configuration"" to the ASCII value ‘Board_special
version 1.0.0’, with the following custom GUID:

‘Configuration-a87cb185-864c-4683-a93e-3666ad3cf6b6’

grub> set guid_used=a87cb185-864c-4683-a93e-3666ad3cf6b6
grub>
grub> efivar_set -g guid_used Configuration "Board_special version 1.0.0"
grub>
grub> efivar_get -g guid_used -xv Configuration
Configuration-a87cb185-864c-4683-a93e-3666ad3cf6b6 :

 - 42 6f 61 72 64 5f 73 70 65 63 69 61 6c 20 76 65 |Board_special ve|
0010 - 72 73 69 6f 6e 20 31 2e 30 2e 30 00 |rsion 1.0.0.|

-
Usage
-

Usage: efivar_get [OPTIONS] VAR

Read EFI VAR variable (if not specified, use EFI_GLOBAL_VARIABLE_GUID as
default GUID: {8be4df61-93ca-11d2-aa0d-00e098032b8c}).
If --set is specified, the version is set to a variable.

-s, --set=VARNAME   Assign return value to variable VARNAME.
-g, --guid-var=GUIDVARNAME   Use the content of GUIDVARNAME as GUID
-x, --hexdump   Display EFI variable content in hex mode
-v, --verbose   Display more info about EFI variable.
-h, --help  Display this help and exit.
-u, --usage Display the usage of this command and exit.

Usage: efivar_set [OPTIONS] VAR VALUE

Write VALUE to EFI variable VAR (if not specified, use EFI_GLOBAL_VARIABLE_GUID
as default GUID:{8be4df61-93ca-11d2-aa0d-00e098032b8c}).
If --set is specified, the same EFI variable value is set to a variable.
If --non-volatile is specified, the non-volatile EFI attribute is set.

Note: an existing EFI variable can be changed only maintainig the same
volatile/non-volatile attribute.

-s, --set=VARNAME   Assign the same EFI variable value to variable
VARNAME.
-g, --guid-var=GUIDVARNAME   Use the content of GUIDVARNAME as GUID
-n, --non-volatile  Set non-volatile attribute.
-v, --verbose   Display more info about EFI variable
-h, --help  Display this help and exit.
-u, --usage Display the usage of this command and exit.

Signed-off-by: Flavio Suligoi 
---
 docs/grub.texi  | 137 +++
 grub-core/Makefile.core.def |   5 +
 grub-core/commands/efi/efivar_get_set.c | 288 
 grub-core/kern/efi/efi.c|  57 +++
 include/grub/efi/efi.h  |   4 +
 5 files changed, 491 insertions(+)
 create mode 100644 grub-core/commands/efi/efivar_get_set.c

diff --git a/docs/grub.texi b/docs/grub.texi
index d6408d2..ee59079 100644
--- a/docs/grub.texi
+++ b/docs/grub.texi
@@ -3978,6 +3978,8 @@ you forget a command, you can run the command 
@command{help}
 * distrust::Remove a pubkey from trusted keys
 * drivemap::Map a drive to another
 * echo::Display a line of text
+* efivar_get::  Get EFI variable
+* efivar_set::  Set EFI variable
 * eval::Evaluate agruments as GRUB commands
 * export::  Export an environment variable
 * false::   Do nothing, unsuccessfully
@@ -4397,6 +4399,141 @@ character will print that character.
 @end deffn
 
 
+@node efivar_get
+@subsection efivar_get
+@deffn Command efivar_get [@option{--set} var] [@option{--guid-var} 
guidvarname] [@option{--hexdump}] [@option{--verbose}] efivar
+Get and display an EFI variable @var{efivar} in ASCII or hex+ASCII format.
+
+The EFI GUID can be loaded from an environment variable, using the option
+@option{--guid-var @var{guidvarname}}. If this option is not used and
+consequently no GUID is specified, the GUID:
+
+
+@center EFI_GLOBAL_VARIABLE_GUID = 8be4df61-93ca-11d2-aa0d-00e098032b8c
+
+is implicitly assumed.
+
+If the option @option{--set @var{var}} is specified, store the EFI variable to
+the environment variable @var{var} instead of printing it.
+
+If the option @option{--hexdump} is specified, the output is in hexdecimal
+format.
+
+The option @option{--verbose} improve the data information output.
+
+@emph{Example:} read the EFI variable:
+
+@center @samp{PlatformInfo-19ad5244-fd6b-4e5c-826a-414646d6da6a}
+
+@smallexample
+grub> set guid_used=19ad52

[PATCH 1/2] efi: add non-volatile parameter to grub_efi_set_variable

2020-04-29 Thread Flavio Suligoi 
The current version of the function:

grub_efi_set_variable

always set an EFI variable as "non-volatile", since the
variable attribute:

GRUB_EFI_VARIABLE_NON_VOLATILE

is fixed in the EFI SetVariable() call.

With this change it is possible to decide if the written
EFI variable is volatile or not.

Signed-off-by: Flavio Suligoi 
---
 grub-core/commands/efi/efifwsetup.c |  2 +-
 grub-core/kern/efi/efi.c| 15 +--
 include/grub/efi/efi.h  |  3 ++-
 3 files changed, 12 insertions(+), 8 deletions(-)

diff --git a/grub-core/commands/efi/efifwsetup.c 
b/grub-core/commands/efi/efifwsetup.c
index 7a137a7..4ccb578 100644
--- a/grub-core/commands/efi/efifwsetup.c
+++ b/grub-core/commands/efi/efifwsetup.c
@@ -45,7 +45,7 @@ grub_cmd_fwsetup (grub_command_t cmd __attribute__ ((unused)),
 os_indications |= *old_os_indications;
 
   status = grub_efi_set_variable ("OsIndications", &global, &os_indications,
- sizeof (os_indications));
+ sizeof (os_indications), 1);
   if (status != GRUB_ERR_NONE)
 return status;
 
diff --git a/grub-core/kern/efi/efi.c b/grub-core/kern/efi/efi.c
index 3a708ed..dcb09b2 100644
--- a/grub-core/kern/efi/efi.c
+++ b/grub-core/kern/efi/efi.c
@@ -193,12 +193,15 @@ grub_efi_set_virtual_address_map (grub_efi_uintn_t 
memory_map_size,
 
 grub_err_t
 grub_efi_set_variable(const char *var, const grub_efi_guid_t *guid,
- void *data, grub_size_t datasize)
+ void *data, grub_size_t datasize,
+  grub_efi_boolean_t non_volatile)
 {
   grub_efi_status_t status;
   grub_efi_runtime_services_t *r;
   grub_efi_char16_t *var16;
   grub_size_t len, len16;
+  grub_uint64_t efi_attrib = GRUB_EFI_VARIABLE_BOOTSERVICE_ACCESS |
+ GRUB_EFI_VARIABLE_RUNTIME_ACCESS;
 
   len = grub_strlen (var);
   len16 = len * GRUB_MAX_UTF16_PER_UTF8;
@@ -210,11 +213,11 @@ grub_efi_set_variable(const char *var, const 
grub_efi_guid_t *guid,
 
   r = grub_efi_system_table->runtime_services;
 
-  status = efi_call_5 (r->set_variable, var16, guid, 
-  (GRUB_EFI_VARIABLE_NON_VOLATILE
-   | GRUB_EFI_VARIABLE_BOOTSERVICE_ACCESS
-   | GRUB_EFI_VARIABLE_RUNTIME_ACCESS),
-  datasize, data);
+  if (non_volatile)
+efi_attrib |= GRUB_EFI_VARIABLE_NON_VOLATILE;
+
+  status = efi_call_5 (r->set_variable, var16, guid, efi_attrib,
+   datasize, data);
   grub_free (var16);
   if (status == GRUB_EFI_SUCCESS)
 return GRUB_ERR_NONE;
diff --git a/include/grub/efi/efi.h b/include/grub/efi/efi.h
index e90e00d..9b04150 100644
--- a/include/grub/efi/efi.h
+++ b/include/grub/efi/efi.h
@@ -81,7 +81,8 @@ grub_err_t
 EXPORT_FUNC (grub_efi_set_variable) (const char *var,
 const grub_efi_guid_t *guid,
 void *data,
-grub_size_t datasize);
+grub_size_t datasize,
+ grub_efi_boolean_t non_volatile);
 int
 EXPORT_FUNC (grub_efi_compare_device_paths) (const grub_efi_device_path_t *dp1,
 const grub_efi_device_path_t *dp2);
-- 
2.7.4


___
Grub-devel mailing list
Grub-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/grub-devel

Re: [PATCH v2] tests: Don't run f2fs test on systems with PAGE_SIZE more than 4096 bytes

2020-04-29 Thread Hans Ulrich Niedermann 
On Wed, 29 Apr 2020 17:06:36 +0300
Anatoly Pugachev  wrote:

> Don't run f2fs test on systems with PAGE_SIZE > 4096 bytes, since
> f2fs is not supported on these systems and trying to mount a f2fs
> filesystem would fail.

"Skip the f2fs test on ..." might be better wording, both in this
paragraph and the Subject.

Exit code 77 is certainly documented with the word "skip", and "exit
77" will show up in the "make check" output as "SKIP" as well.

> v2 changes:
> 
> - fix compare
> - quotes around variable expansion
> 
> Signed-off-by: Anatoly Pugachev 
> Reviewed-by: Mike Gilbert 
> 
> ---
>  tests/f2fs_test.in | 6 ++
>  1 file changed, 6 insertions(+)
> 
> diff --git a/tests/f2fs_test.in b/tests/f2fs_test.in
> index 1ea77c826..3da1dad57 100644
> --- a/tests/f2fs_test.in
> +++ b/tests/f2fs_test.in
> @@ -15,5 +15,11 @@ if ! which mkfs.f2fs >/dev/null 2>&1; then
>   exit 77
>  fi
>  
> +PAGE_SIZE=$(getconf PAGE_SIZE)
> +F2FS_BLKSIZE=4096
> +if [ "$PAGE_SIZE" -gt "$F2FS_BLKSIZE" ]; then
> + printf "f2fs is not supported on PAGE_SIZE(%d) != %d\n" $PAGE_SIZE
> $F2FS_BLKSIZE
> + exit 77
> +fi

This confusing to me. You are skipping the test when

PAGE_SIZE > F2FS_BLKSIZE

but the corresponding message says

PAGE_SIZE != F2FS_BLKSIZE

Now... which condition is it supposed to be? ">" or "!="?

I know from the Linux kernel's ext2 driver that it is very well
possible that PAGE_SIZE != EXT2_BLOCK_SIZE can work
unless EXT2_BLOCK_SIZE > PAGE_SIZE.

So ">" and "!=" are not necessarily the same thing, and IMHO the check
and the message use the same condition.

Uli

___
Grub-devel mailing list
Grub-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/grub-devel

Re: [PATCH v2] tests: Don't run f2fs test on systems with PAGE_SIZE more than 4096 bytes

2020-04-29 Thread Hans Ulrich Niedermann 
On Wed, 29 Apr 2020 19:25:57 +0200
Hans Ulrich Niedermann  wrote:

> On Wed, 29 Apr 2020 17:06:36 +0300
> Anatoly Pugachev  wrote:
> 
> > Don't run f2fs test on systems with PAGE_SIZE > 4096 bytes, since
> > f2fs is not supported on these systems and trying to mount a f2fs
> > filesystem would fail.  
> 
> "Skip the f2fs test on ..." might be better wording, both in this
> paragraph and the Subject.
> 
> Exit code 77 is certainly documented with the word "skip", and "exit
> 77" will show up in the "make check" output as "SKIP" as well.
> 
> > v2 changes:
> > 
> > - fix compare
> > - quotes around variable expansion
> > 
> > Signed-off-by: Anatoly Pugachev 
> > Reviewed-by: Mike Gilbert 
> > 
> > ---
> >  tests/f2fs_test.in | 6 ++
> >  1 file changed, 6 insertions(+)
> > 
> > diff --git a/tests/f2fs_test.in b/tests/f2fs_test.in
> > index 1ea77c826..3da1dad57 100644
> > --- a/tests/f2fs_test.in
> > +++ b/tests/f2fs_test.in
> > @@ -15,5 +15,11 @@ if ! which mkfs.f2fs >/dev/null 2>&1; then
> >   exit 77
> >  fi
> >  
> > +PAGE_SIZE=$(getconf PAGE_SIZE)
> > +F2FS_BLKSIZE=4096
> > +if [ "$PAGE_SIZE" -gt "$F2FS_BLKSIZE" ]; then
> > + printf "f2fs is not supported on PAGE_SIZE(%d) != %d\n" $PAGE_SIZE
> > $F2FS_BLKSIZE
> > + exit 77
> > +fi  
> 
> This confusing to me. You are skipping the test when
> 
> PAGE_SIZE > F2FS_BLKSIZE
> 
> but the corresponding message says
> 
> PAGE_SIZE != F2FS_BLKSIZE
> 
> Now... which condition is it supposed to be? ">" or "!="?
> 
> I know from the Linux kernel's ext2 driver that it is very well
> possible that PAGE_SIZE != EXT2_BLOCK_SIZE can work
> unless EXT2_BLOCK_SIZE > PAGE_SIZE.
> 
> So ">" and "!=" are not necessarily the same thing, and IMHO the check
> and the message use the same condition.

The commit title and the commit message are also talking about
"PAGE_SIZE > 4096" instead of "PAGE_SIZE != 4096".

This adds more confusion over whether F2FS_BLKSIZE just happens to be
4096 somewhere where PAGE_SIZE is 4096, whether F2FS_BLKSIZE is defined
to be 4096 everywhere, and whether the whole thing is about the number
4096 or about the value of F2FS_BLKSIZE.

(Again, in the ext2 example, filesystems can have many block sizes, but
the default is the Linux PAGE_SIZE, so all x86 PCs will create
filesystems with block size 4096 but a MIPS system like the Western
Digital SOHO external HDD NAS will create a filesystem with block
size 65536. The kernel driver just needs PAGE_SIZE >= block size to
work, so a MIPS created ext2 filesystem will not mount on a x86 PC.)

Mentioning the same condition in commit title, commit message, the
actual "if" branch, and then the "printf" message would certainly reduce
my confusion.

Uli

___
Grub-devel mailing list
Grub-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/grub-devel

ASN.1 parsing for verifying appended signatures

2020-04-29 Thread Daniel Axtens 
Hi,

I'd like to teach grub how to verify appended signatures. Appended
signatures are a format used by the Linux kernel to cryptographically
sign binaries. They're used to verify kernel modules on all platforms
[1], and they're also used on some platforms (such as PowerPC) for
signing the kernel itself [2]. The cryptographic content of an appended
signature is a PKCS#7 message, which is in ASN.1 DER format.

I have a proof-of-concept that adds a verifier much like the existing
detached signature verifier. It uses libtasn1 to parse the appended
signature and an x509 signing certificate and then uses the existing
libgcrypt functionality to perform the cryptographic verification.

I'd like to gradually upstream this verifier. As part of that, rather
than writing a bespoke ASN.1 parser for grub, I'm hoping to include
libtasn1 in grub. libtasn1 is LGPLv2.1+ licensed, which is the same as
libgcrypt, so I believe it's a compatible license already.

I have a couple of questions I was hoping to get some input on:

1) Would including libtasn1 into grub be acceptable? It has the
   advantage of being extensively tried and tested, so I think it's
   likely to be more reliable than a hand-rolled solution.

2) If so, would it be better to write a script like import_gcry.py, or
   to perform the import once manually?

   I think the idea of import_gcry.py was to ease future upgrades of
   libgcrypt, but it no longer works and we've taken to applying patches
   directly [3].

   My preference would be to import it manually in two commits - one
   which brings in the unmodified libtasn1 source, and then one that
   adapts it to grub. That way, we should be able to use git rebase to
   help us apply future updates. But I'm happy to do whatever would be
   most acceptable.

Regards,
Daniel

[1]: https://www.kernel.org/doc/html/v4.15/admin-guide/module-signing.html

[2]: These signatures are verified by the kernel's Integrity Measurement
 Architecture (IMA) when kexec()ing into a new kernel. Ubuntu
 already ships powerpc kernels signed in this way, and the OpenPower
 host bootloader (Petitboot) uses this for secure boot.

[3]: See e.g. commit e5ba6b26181b ("libgcrypt: Import replacement CRC
 operations"), which reads in part:

 The ideal solution would be to update to a new version of libgcrypt, 
and
 I spent some time trying to do that.  However, util/import_gcry.py
 requires complex modifications to cope with the new version, and I
 stalled part-way through; furthermore, GRUB's libgcrypt tree already
 contains some backports of upstream changes.  Rather than allowing the
 perfect to be the enemy of the good, I think it's best to backport this
 single change to at least sort out the licensing situation.  Doing so
 won't make things any harder for a future wholesale upgrade.


___
Grub-devel mailing list
Grub-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/grub-devel

Re: [PATCH RFC/RFT 0/3] Add grub loader support for RISC-V Linux

2020-04-29 Thread Atish Patra 
On Wed, Apr 29, 2020 at 4:22 AM Leif Lindholm  wrote:
>
> Hi Atish,
>
> On Tue, Apr 28, 2020 at 11:21:05 -0700, Atish Patra wrote:
> > > > Hello Ard,
> > > >
> > > > Did I misunderstand you and you want to provide a LOAD_FILE2
> > > > implementation in GRUB and not use the one in the firmware?
> > >
> > > Yes. If you use GRUB, it is provided by GRUB. Otherwise, it can be
> > > provided by U-Boot or EDK2.
> >
> > Thanks for the discussion. I got clarification as well.
> > I will work on adding LOAD_FILE2 support in GRUB.
>
> I had promised to look into that, but my paperwork with the FSF is
> still pending. Feel free to contact me for bouncing ideas or whatever.
>

Sure.


> As for this set - the changes to the kernel header structs and naming
> should be non-controversial, and will be required for a final unified
> loader anyway. Could you possibly break those out as a separate set,
> which could then be merged earlier?
>

Ok. I will send out riscv header changes separately.

> Best Regards,
>
> Leif



--
Regards,
Atish

___
Grub-devel mailing list
Grub-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/grub-devel

[PATCH] RISC-V: Update linux image header

2020-04-29 Thread Atish Patra 
Update the RISC-V Linux kernel image headers as per the current header
defined in Linux kernel.

Reference:
/Documentation/riscv/boot-image-header.rst

Signed-off-by: Atish Patra 
---
 include/grub/riscv32/linux.h | 15 ---
 include/grub/riscv64/linux.h | 15 ---
 2 files changed, 16 insertions(+), 14 deletions(-)

diff --git a/include/grub/riscv32/linux.h b/include/grub/riscv32/linux.h
index 12f66dbee0ee..706c69087546 100644
--- a/include/grub/riscv32/linux.h
+++ b/include/grub/riscv32/linux.h
@@ -19,20 +19,21 @@
 #ifndef GRUB_RISCV32_LINUX_HEADER
 #define GRUB_RISCV32_LINUX_HEADER 1
 
-#define GRUB_LINUX_RISCV_MAGIC_SIGNATURE 0x52534356 /* 'RSCV' */
+#define GRUB_LINUX_RISCV_MAGIC_SIGNATURE 0x05435352 /* 'RSC\0x5' */
 
-/* From linux/Documentation/riscv/booting.txt */
+/* From linux/Documentation/riscv/boot-image-header.rst */
 struct linux_riscv_kernel_header
 {
   grub_uint32_t code0; /* Executable code */
   grub_uint32_t code1; /* Executable code */
-  grub_uint64_t text_offset;   /* Image load offset */
-  grub_uint64_t res0;  /* reserved */
-  grub_uint64_t res1;  /* reserved */
+  grub_uint64_t text_offset;   /* Image load offset, little endian */
+  grub_uint64_t image_size;/* Effective Image size, little endian */
+  grub_uint64_t flags; /* kernel flags, little endian */
+  grub_uint32_t version;   /* Version of this header */
+  grub_uint32_t res1;  /* reserved */
   grub_uint64_t res2;  /* reserved */
   grub_uint64_t res3;  /* reserved */
-  grub_uint64_t res4;  /* reserved */
-  grub_uint32_t magic; /* Magic number, little endian, "RSCV" */
+  grub_uint32_t magic; /* Magic number, little endian, "RSC\x05" */
   grub_uint32_t hdr_offset;/* Offset of PE/COFF header */
 };
 
diff --git a/include/grub/riscv64/linux.h b/include/grub/riscv64/linux.h
index 682cd1812aa7..88d5df781899 100644
--- a/include/grub/riscv64/linux.h
+++ b/include/grub/riscv64/linux.h
@@ -19,22 +19,23 @@
 #ifndef GRUB_RISCV64_LINUX_HEADER
 #define GRUB_RISCV64_LINUX_HEADER 1
 
-#define GRUB_LINUX_RISCV_MAGIC_SIGNATURE 0x52534356 /* 'RSCV' */
+#define GRUB_LINUX_RISCV_MAGIC_SIGNATURE 0x05435352 /* 'RSC\0x5' */
 
 #define GRUB_EFI_PE_MAGIC  0x5A4D
 
-/* From linux/Documentation/riscv/booting.txt */
+/* From linux/Documentation/riscv/boot-image-header.rst */
 struct linux_riscv_kernel_header
 {
   grub_uint32_t code0; /* Executable code */
   grub_uint32_t code1; /* Executable code */
-  grub_uint64_t text_offset;   /* Image load offset */
-  grub_uint64_t res0;  /* reserved */
-  grub_uint64_t res1;  /* reserved */
+  grub_uint64_t text_offset;   /* Image load offset, little endian */
+  grub_uint64_t image_size;/* Effective Image size, little endian */
+  grub_uint64_t flags; /* kernel flags, little endian */
+  grub_uint32_t version;   /* Version of this header */
+  grub_uint32_t res1;  /* reserved */
   grub_uint64_t res2;  /* reserved */
   grub_uint64_t res3;  /* reserved */
-  grub_uint64_t res4;  /* reserved */
-  grub_uint32_t magic; /* Magic number, little endian, "RSCV" */
+  grub_uint32_t magic; /* Magic number, little endian, "RSC\x05" */
   grub_uint32_t hdr_offset;/* Offset of PE/COFF header */
 };
 
-- 
2.26.2


___
Grub-devel mailing list
Grub-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/grub-devel