Re: [PATCH v8 08/22] protectors: Add key protectors framework
On Thu, Jan 18, 2024 at 03:02:19PM +0800, Gary Lin wrote:
> On Wed, Jan 17, 2024 at 05:58:25AM +0300, Vladimir 'phcoder' Serbinenko wrote:
> > Any strong reason to have it in kernel? It doesn't seem to be
> > necessary in non-crypto cases. Separate module or cryptodisk looks
> > like better places
> >
> I have no strong opinion about the location as long as it works.
> Will move the key protector to cryptodisk since it's the only user currently.
>
It turned out that moving key protector to cryptodisk doesn't work
because the tpm2 module (added later) registers itself as the key
protector and then cryptodisk calls the specific key protector with
recover_key(). Both modules need a common ground to access the key
protector functions, and kernel is the ideal place.
Gary Lin
> Gary Lin
>
> > On Tue, Jan 16, 2024 at 12:22 PM Gary Lin via Grub-devel
> > wrote:
> > >
> > > From: Hernan Gatta
> > >
> > > A key protector encapsulates functionality to retrieve an unlocking key
> > > for a fully-encrypted disk from a specific source. A key protector
> > > module registers itself with the key protectors framework when it is
> > > loaded and unregisters when unloaded. Additionally, a key protector may
> > > accept parameters that describe how it should operate.
> > >
> > > The key protectors framework, besides offering registration and
> > > unregistration functions, also offers a one-stop routine for finding and
> > > invoking a key protector by name. If a key protector with the specified
> > > name exists and if an unlocking key is successfully retrieved by it, the
> > > function returns to the caller the retrieved key and its length.
> > >
> > > Signed-off-by: Hernan Gatta
> > > Signed-off-by: Gary Lin
> > > ---
> > > grub-core/Makefile.am | 1 +
> > > grub-core/Makefile.core.def | 1 +
> > > grub-core/kern/protectors.c | 75 +
> > > include/grub/protector.h| 48
> > > 4 files changed, 125 insertions(+)
> > > create mode 100644 grub-core/kern/protectors.c
> > > create mode 100644 include/grub/protector.h
> > >
> > > diff --git a/grub-core/Makefile.am b/grub-core/Makefile.am
> > > index f18550c1c..af21fc72d 100644
> > > --- a/grub-core/Makefile.am
> > > +++ b/grub-core/Makefile.am
> > > @@ -90,6 +90,7 @@ endif
> > > KERNEL_HEADER_FILES += $(top_srcdir)/include/grub/mm.h
> > > KERNEL_HEADER_FILES += $(top_srcdir)/include/grub/parser.h
> > > KERNEL_HEADER_FILES += $(top_srcdir)/include/grub/partition.h
> > > +KERNEL_HEADER_FILES += $(top_srcdir)/include/grub/protector.h
> > > KERNEL_HEADER_FILES += $(top_srcdir)/include/grub/stack_protector.h
> > > KERNEL_HEADER_FILES += $(top_srcdir)/include/grub/term.h
> > > KERNEL_HEADER_FILES += $(top_srcdir)/include/grub/time.h
> > > diff --git a/grub-core/Makefile.core.def b/grub-core/Makefile.core.def
> > > index c9d81b56a..70d5e0e00 100644
> > > --- a/grub-core/Makefile.core.def
> > > +++ b/grub-core/Makefile.core.def
> > > @@ -149,6 +149,7 @@ kernel = {
> > >common = kern/misc.c;
> > >common = kern/parser.c;
> > >common = kern/partition.c;
> > > + common = kern/protectors.c;
> > >common = kern/rescue_parser.c;
> > >common = kern/rescue_reader.c;
> > >common = kern/term.c;
> > > diff --git a/grub-core/kern/protectors.c b/grub-core/kern/protectors.c
> > > new file mode 100644
> > > index 0..5ee059565
> > > --- /dev/null
> > > +++ b/grub-core/kern/protectors.c
> > > @@ -0,0 +1,75 @@
> > > +/*
> > > + * GRUB -- GRand Unified Bootloader
> > > + * Copyright (C) 2022 Microsoft Corporation
> > > + *
> > > + * 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
[PATCH] util/grub.d/30_os-prober.in: Fix GRUB_OS_PROBER_SKIP_LIST for non-EFI
GRUB documentation states:
'GRUB_OS_PROBER_SKIP_LIST'
List of space-separated FS UUIDs of filesystems to be ignored from
os-prober output. For efi chainloaders it's @
But the actual behaviour does not match this description. Setting
GRUB_OS_PROBER_SKIP_LIST=""
does not ignore non-EFI boot loaders detected on filesystem .
In order to skip non-EFI boot loaders, you must set
GRUB_OS_PROBER_SKIP_LIST="@"
which is both absurd (UUID and device are redundant) and wrong
(device names such as /dev/sd* may not be persistent across boots).
This patch fixes the detection of "@" in the device string
reported by os-prober.
Fixes: 55e706c9 (Add GRUB_OS_PROBER_SKIP_LIST to selectively skipping systems)
Note: the UUID matching regex uses word boundaries '\b' but '@' is
not a word character. As a consequence, setting
GRUB_OS_PROBER_SKIP_LIST="@"
will ignore non-EFI boot loaders detected on filesystem even
though the goal is to ignore a given EFI boot loader only.
However I think this is desirable to preserve the behaviour of
existing setups which use GRUB_OS_PROBER_SKIP_LIST="@".
Signed-off-by: Pascal Hambourg
---
util/grub.d/30_os-prober.in | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/util/grub.d/30_os-prober.in b/util/grub.d/30_os-prober.in
index e9e217208..a24c01334 100644
--- a/util/grub.d/30_os-prober.in
+++ b/util/grub.d/30_os-prober.in
@@ -125,7 +125,7 @@ for OS in ${OSPROBED} ; do
if UUID="`${grub_probe} --target=fs_uuid --device ${DEVICE%@*}`"; then
EXPUUID="$UUID"
-if [ x"${DEVICE#*@}" != x ] ; then
+if [ x"${DEVICE%@*}" != x"${DEVICE}" ] ; then
EXPUUID="${EXPUUID}@${DEVICE#*@}"
fi
--
2.30.2
___
Grub-devel mailing list
Grub-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/grub-devel
[PATCH v2] util/grub.d/30_os-prober.in: Skip drivemap for Windows 8 to 19, Server 2012 and later
If Windows Vista, Seven and Server 2008 do not need drivemap,
then later versions using bootmgr should not need it either.
This patch checks Windows versions up to 19 because distinguishing 20
from 2000 requires more complex logic. Hopefully Windows will drop
legacy BIOS boot support before version 20.
Signed-off-by: Pascal Hambourg
---
v2:
- Harden the version check to exclude Windows 1.* (even though
os-prober does not detect it)
util/grub.d/30_os-prober.in | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/util/grub.d/30_os-prober.in b/util/grub.d/30_os-prober.in
index 656301eaf..043e5dfea 100644
--- a/util/grub.d/30_os-prober.in
+++ b/util/grub.d/30_os-prober.in
@@ -167,7 +167,7 @@ EOF
fi
case ${LONGNAME} in
- Windows\ Vista*|Windows\ 7*|Windows\ Server\ 2008*)
+ Windows\ Vista*|Windows\ [78]*|Windows\ 1[0-9]*|Windows\ Server\
2008*|Windows\ Server\ 20[1-9]*)
;;
*)
cat << EOF
--
2.30.2
___
Grub-devel mailing list
Grub-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/grub-devel
[PATCH v2] util/grub.d/30_os-prober.in: Conditionally show or hide chain and efi menu entries
On systems which support multiple boot platforms such as BIOS or EFI,
it makes no sense to show menu entries which are not supported by
the current boot platform. Menu entries generated from os-prober
'chain' boot type use boot sector chainloading which is supported
by PC BIOS platform only.
Show 'chain' menu entries only if boot platform is PC BIOS.
Show 'efi' menu entries only if boot platform is EFI.
This is aimed to allow os-prober to report both EFI and PC BIOS
boot loaders regardless of the current boot mode on x86 systems
which support both EFI and legacy BIOS boot.
Signed-off-by: Pascal Hambourg
---
Note: I did not change indentation to keep the patch minimal.
v2:
- show 'chain' menu entries if platform=pc instead of platform!=efi
- add explanatory comments in generated config
util/grub.d/30_os-prober.in | 6 ++
1 file changed, 6 insertions(+)
diff --git a/util/grub.d/30_os-prober.in b/util/grub.d/30_os-prober.in
index 043e5dfea..e9e217208 100644
--- a/util/grub.d/30_os-prober.in
+++ b/util/grub.d/30_os-prober.in
@@ -155,6 +155,8 @@ for OS in ${OSPROBED} ; do
onstr="$(gettext_printf "(on %s)" "${DEVICE}")"
cat << EOF
+# this menu entry is supported only by PC BIOS platform
+if [ "\$grub_platform" = "pc" ]; then
menuentry '$(echo "${LONGNAME} $onstr" | grub_quote)' $CLASS --class os
\$menuentry_id_option 'osprober-chain-$(grub_get_device_id "${DEVICE}")' {
EOF
save_default_entry | grub_add_tab
@@ -179,6 +181,7 @@ EOF
cat
[PATCH 0/3] Clean up unused values
Coverity listed three unused value bugs in the GRUB. These patches help clean up and remove these uneccessary bits of code. The Coverity bugs being addressed are: CID 428875 CID 428876 CID 428877 Alec Brown (3): fs/jfs.c: Clean up redundant code osdep/unix/getroot.c: Clean up redundant code loader/i386/multiboot_mbi.c: Clean up redundant code grub-core/fs/jfs.c| 1 - grub-core/loader/i386/multiboot_mbi.c | 2 +- grub-core/osdep/unix/getroot.c| 1 - 3 files changed, 1 insertion(+), 3 deletions(-) ___ Grub-devel mailing list Grub-devel@gnu.org https://lists.gnu.org/mailman/listinfo/grub-devel
[PATCH 3/3] loader/i386/multiboot_mbi.c: Clean up redundant code
In grub-core/loader/i386/multiboot_mbi.c, coverity spotted redundant code where
the variable err was being set to GRUB_ERR_NONE and then being overwritten
later without being used. Since this is unnecessary, we can remove the code
that sets err to GRUB_ERR_NONE.
Fixes: CID 428877
Signed-off-by: Alec Brown
---
grub-core/loader/i386/multiboot_mbi.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/grub-core/loader/i386/multiboot_mbi.c
b/grub-core/loader/i386/multiboot_mbi.c
index 11a6e224f..fae5d6fb8 100644
--- a/grub-core/loader/i386/multiboot_mbi.c
+++ b/grub-core/loader/i386/multiboot_mbi.c
@@ -86,7 +86,7 @@ load_kernel (grub_file_t file, const char *filename,
return GRUB_ERR_NONE;
}
if (err == GRUB_ERR_UNKNOWN_OS && (header->flags &
MULTIBOOT_AOUT_KLUDGE))
- grub_errno = err = GRUB_ERR_NONE;
+ grub_errno = GRUB_ERR_NONE;
}
if (header->flags & MULTIBOOT_AOUT_KLUDGE)
{
--
2.27.0
___
Grub-devel mailing list
Grub-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/grub-devel
[PATCH 1/3] fs/jfs.c: Clean up redundant code
In grub-core/fs/jfs.c, coverity spotted redundant code where the pointer diro was being set to 0 and then being overwritten later without being used. Since this is unnecessary, we can remove the code that sets diro to 0. Fixes: CID 428876 Signed-off-by: Alec Brown --- grub-core/fs/jfs.c | 1 - 1 file changed, 1 deletion(-) diff --git a/grub-core/fs/jfs.c b/grub-core/fs/jfs.c index 6f7c43904..62e20ef6f 100644 --- a/grub-core/fs/jfs.c +++ b/grub-core/fs/jfs.c @@ -716,7 +716,6 @@ grub_jfs_find_file (struct grub_jfs_data *data, const char *path, grub_uint32_t dirino = grub_le_to_cpu32 (data->currinode.inode); grub_jfs_closedir (diro); - diro = 0; if (grub_jfs_read_inode (data, ino, &data->currinode)) break; -- 2.27.0 ___ Grub-devel mailing list Grub-devel@gnu.org https://lists.gnu.org/mailman/listinfo/grub-devel
[PATCH 2/3] osdep/unix/getroot.c: Clean up redundant code
In grub-core/osdep/unix/getroot.c, coverity spotted redundant code where the double pointer os_dev was being set to 0 and then being overwritten later without being used. Since this is unnecessary, we can remove the code that sets os_dev to 0. Fixes: CID 428875 Signed-off-by: Alec Brown --- grub-core/osdep/unix/getroot.c | 1 - 1 file changed, 1 deletion(-) diff --git a/grub-core/osdep/unix/getroot.c b/grub-core/osdep/unix/getroot.c index cde821eb9..12b111634 100644 --- a/grub-core/osdep/unix/getroot.c +++ b/grub-core/osdep/unix/getroot.c @@ -540,7 +540,6 @@ grub_guess_root_devices (const char *dir_in) for (cur = os_dev; *cur; cur++) free (*cur); free (os_dev); - os_dev = 0; } if (stat (dir, &st) < 0) -- 2.27.0 ___ Grub-devel mailing list Grub-devel@gnu.org https://lists.gnu.org/mailman/listinfo/grub-devel
