Re: [PATCH] efi: Increase default memory allocation to 32MB
Adding some folks who may be interested in this thing too... On Tue, Sep 20, 2022 at 12:30:30AM +1000, Daniel Axtens wrote: > We have multiple reports of things being slower with a 1MB initial static > allocation, and a report (more difficult to nail down) of a boot failure > as a result of the smaller initial allocation. > > Make the initial memory allocation 32MB. > > Signed-off-by: Daniel Axtens Reviewed-by: Daniel Kiper However, I think we should consider [1] patch set too. I will be looking at it in the following days. Daniel [1] https://lists.gnu.org/archive/html/grub-devel/2022-09/msg00080.html > --- > 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 d290c9a76270..3705b8b1b465 100644 > --- a/grub-core/kern/efi/mm.c > +++ b/grub-core/kern/efi/mm.c > @@ -39,7 +39,7 @@ > #define MEMORY_MAP_SIZE 0x3000 > > /* The default heap size for GRUB itself in bytes. */ > -#define DEFAULT_HEAP_SIZE0x10 > +#define DEFAULT_HEAP_SIZE0x200 > > static void *finish_mmap_buf = 0; > static grub_efi_uintn_t finish_mmap_size = 0; ___ 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 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. Daniel ___ Grub-devel mailing list Grub-devel@gnu.org https://lists.gnu.org/mailman/listinfo/grub-devel
Re: [PATCH 1/1] templates: filter C.UTF-8 locale for translation
On Mon, Sep 19, 2022 at 03:31:28PM +0200, Christian Hesse wrote:
> From: Christian Hesse
>
> In addition to C locale there is also C.UTF-8 locale now. Filter that as
> well, by using ${grub_lang}, which contains a stripped value.
> This fixes the following message and resulting boot failure:
>
> error: file `/boot/grub/locale/C.gmo' not found.
>
> Signed-off-by: Christian Hesse
Reviewed-by: Daniel Kiper
Daniel
___
Grub-devel mailing list
Grub-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/grub-devel
Re: [PATCH 1/1] commands/efi/lsefisystab: Short text for EFI_CONFORMANCE_PROFILES_TABLE
On Fri, Sep 02, 2022 at 02:58:45PM +0200, Heinrich Schuchardt wrote: > The EFI_CONFORMANCE_PROFILES_TABLE_GUID is used for a table of GUIDs for > conformance profiles (cf. UEFI specification 2.10, > 4.6.5 EFI_CONFORMANCE_PROFILE_TABLE). > > The lsefisystab command is used to display installed EFI configuration > tables. Currently it only shows the GUID but not a short text for the > table. > > Provide a short text for the EFI_CONFORMANCE_PROFILES_TABLE_GUID. > > Signed-off-by: Heinrich Schuchardt Reviewed-by: Daniel Kiper Daniel ___ Grub-devel mailing list Grub-devel@gnu.org https://lists.gnu.org/mailman/listinfo/grub-devel
Re: [PATCH] menu: Add ctrl-L to refresh the menu
On Wed, Aug 31, 2022 at 06:24:28PM +1000, Benjamin Herrenschmidt wrote: > This is useful on cloud instances with remote serial ports as it can be > difficult to connect "fast enough" to get the initial menu display > > Signed-off-by: Benjamin Herrenschmidt I am OK with this patch but it does not seem to apply on master. Please rebase and repost. Daniel ___ 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
Re: [PATCH] Initialize BSD relocator state variables
On Wed, Aug 24, 2022 at 10:47:02AM -0400, Ross Philipson wrote:
> Numerous register fields in the relocator state are simply not
> used depending on the relocator. This causes Coverity to flag
> these fields but there is no real bug here. Simply initializing
> the variable to {0} solves the issue. Fixed in the else case too
> for consistency.
>
> Fixes: CID 396932
>
> Signed-off-by: Ross Philipson
Reviewed-by: Daniel Kiper
Daniel
___
Grub-devel mailing list
Grub-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/grub-devel
Re: [PATCH] efi: Correct function prototype for register_key_notify method of grub_efi_simple_text_input_ex_interface
On Thu, Aug 25, 2022 at 12:12:17AM -0500, Glenn Washburn wrote: > From: Li Gen > > The register_key_notify() method should have an output parameter which is a > pointer to the unique handle assigned to the registered notification. > > Signed-off-by: Li Gen > Signed-off-by: Glenn Washburn Reviewed-by: Daniel Kiper Daniel ___ Grub-devel mailing list Grub-devel@gnu.org https://lists.gnu.org/mailman/listinfo/grub-devel
Re: [PATCH v2] net/drivers/ieee1275/ofnet: fix incorrect netmask
On Wed, Aug 24, 2022 at 01:36:37PM -0400, Robbie Harwood wrote: > From: Masahiro Matsuya > > The netmask configured in firmware is not respected on ppc64 (big > endian). When 255.255.252.0 is set as netmask in firmware, the > following is the value of bootpath string in > grub_ieee1275_parse_bootpath(): > > /vdevice/l-lan@3002:speed=auto,duplex=auto,192.168.88.10,,192.168.89.113,192.168.88.1,5,5,255.255.252.0,512 > > The netmask in this bootpath is no problem, since it's a value specified > in firmware. But the value of 'subnet_mask.ipv4' was set with > 0xfc00, and __builtin_ctz (~grub_le_to_cpu32 (subnet_mask.ipv4)) > returned 16 (not 22). As a result, 16 was used for netmask wrongly: > > 1100 # subnet_mask.ipv4 (=0xfc00) > 1100 # grub_le_to_cpu32 (subnet_mask.ipv4) > 0011 # ~grub_le_to_cpu32 (subnet_mask.ipv4) > > and the count of zero with __builtin_ctz can be 16. This patch changes > it as below: > > 1100 # subnet_mask.ipv4 (=0xfc00) > 1100 # grub_le_to_cpu32 (subnet_mask.ipv4) > 1100 # grub_be_to_cpu32 (subnet_mask.ipv4) > 0011 # ~grub_be_to_cpu32 (subnet_mask.ipv4) > > The count of zero with __builtin_clz can be 22. (clz counts the number > of one bits preceding the most significant zero bit) > > Signed-off-by: Masahiro Matsuya > Signed-off-by: Robbie Harwood Reviewed-by: Daniel Kiper Daniel ___ Grub-devel mailing list Grub-devel@gnu.org https://lists.gnu.org/mailman/listinfo/grub-devel
Re: [PATCH v2] search: Support searching for GPT partition label
On Fri, 9 Sep 2022 11:49:13 +0200
Daniel Wagenknecht wrote:
> GPT partitions have a name property that is useable for identifying the
> partition in Linux via root=PARTLABEL= boot argument or via
> /dev/disk/by-partlabel/. Add support for searching for it in
> GRUB. Compared to the fs label, the partition label does not change when
> reformatting a partition.
>
> Signed-off-by: Daniel Wagenknecht
> ---
>
> This is a resend of a patch from 2020. Glenn Washburn suggested to rebase and
> resend with Daniel Kiper explicitely added to the recipients.
Thanks Daniel. Following are some suggestions.
>
> The rebase required some changes to adapt to the changes in no_floppy search
> flags but is otherwise unchanged.
>
> Tested by manually searching for a partition by GPT partition label in grub in
> qemu.
>
> Cheers
> Daniel Wagenknecht
>
>
> grub-core/Makefile.core.def | 5 +++
> grub-core/commands/search.c | 57 +++
> grub-core/commands/search_partlabel.c | 5 +++
> grub-core/commands/search_wrap.c | 9 -
> include/grub/search.h | 3 ++
> 5 files changed, 77 insertions(+), 2 deletions(-)
> create mode 100644 grub-core/commands/search_partlabel.c
>
> diff --git a/grub-core/Makefile.core.def b/grub-core/Makefile.core.def
> index 5212dfab1..d76c47113 100644
> --- a/grub-core/Makefile.core.def
> +++ b/grub-core/Makefile.core.def
> @@ -1084,6 +1084,11 @@ module = {
>common = commands/search_label.c;
> };
>
> +module = {
> + name = search_part_label;
> + common = commands/search_partlabel.c;
> +};
> +
> module = {
>name = setpci;
>common = commands/setpci.c;
> diff --git a/grub-core/commands/search.c b/grub-core/commands/search.c
> index 263f1501c..56aa97278 100644
> --- a/grub-core/commands/search.c
> +++ b/grub-core/commands/search.c
> @@ -30,6 +30,8 @@
> #include
> #include
> #include
> +#include
> +#include
>
> GRUB_MOD_LICENSE ("GPLv3+");
>
> @@ -54,6 +56,29 @@ struct search_ctx
>int is_cache;
> };
>
> +/* helper for grub_search_part_label */
> +static char *
> +get_utf8 (grub_uint8_t *in, grub_size_t len)
> +{
> + grub_uint8_t *buf;
> + grub_uint16_t *tmp;
> + grub_size_t i;
> +
> + buf = grub_malloc (len * GRUB_MAX_UTF8_PER_UTF16 + 1);
Use explicit parenthesis here and remove the extra space. I also don't
like the use of GRUB_MAX_UTF8_PER_UTF16 because its wasteful. It should
be half of what it is. But I'll let it pass because that code should be
fixed as part of a different series addressing this.
> + tmp = grub_malloc (len * sizeof (tmp[0]));
> + if (!buf || !tmp)
> +{
> + grub_free (buf);
> + grub_free (tmp);
> + return NULL;
> +}
> + for (i = 0; i < len; i++)
> +tmp[i] = grub_le_to_cpu16 (grub_get_unaligned16 (in + 2 * i));
I think this should be wrapped in an inline function in charset.h,
perhaps called grub_utf16le_to_utf16be. If you can think of a better
name, let me know.
> + *grub_utf16_to_utf8 (buf, tmp, len) = '\0';
> + grub_free (tmp);
> + return (char *) buf;
> +}
> +
> /* Helper for FUNC_NAME. */
> static int
> iterate_device (const char *name, void *data)
> @@ -110,6 +135,34 @@ iterate_device (const char *name, void *data)
> }
>grub_free (buf);
> }
> +#elif defined (DO_SEARCH_PART_LABEL)
> +{
> + grub_device_t dev;
> + char *buf;
> +
> + dev = grub_device_open (name);
> + if (dev && dev->disk && dev->disk->partition)
> + {
> + grub_partition_t p = dev->disk->partition;
> + grub_disk_t disk = grub_disk_open(dev->disk->name);
> +
> + if (!disk)
> + return 1;
> + if (grub_strcmp (p->partmap->name, "gpt") == 0)
> + {
> + struct grub_gpt_partentry gptdata;
> +
> + if (grub_disk_read (disk, p->offset, p->index, sizeof (gptdata),
> &gptdata))
This should probably be using partition entry size from the GPT header
instead of "sizeof (gptdata)" [1]. It looks like you probably took this
code from other places in GRUB that do exactly this. The reason that it
works there and not here (at least in the 3 places that I found) is
that those other uses do not use the name field.
According to the UEFI 2.9 spec section 5.3.3 page 125, the partition
name field is a "Null-terminated string containing a human-readable
name of the partition." So I can accept the code as is, if we only read
the name up to the lower of first wide NULL byte (2 NULLs in a row) or
72 bytes.
[1] https://wiki.osdev.org/GPT#LBA_2:_Partition_Entries
> + return 1;
> + buf = get_utf8(gptdata.name, 36);
So the literal 36 should instead be something like "grub_min
(grub_utf16_strlen (gptdata.name), sizeof (gptdata.name) / 2)". And an
inline function grub_utf16_strlen should be created in
include/grub/charset.h.
> + if (compare_fn (buf, ctx->key) == 0)
> + found = 1;
> + grub_free (buf);
> + }
> + grub_d
Re: [PATCH] menu: Add ctrl-L to refresh the menu
On Thu, 2022-09-22 at 19:36 +0200, Daniel Kiper wrote: > On Wed, Aug 31, 2022 at 06:24:28PM +1000, Benjamin Herrenschmidt wrote: > > This is useful on cloud instances with remote serial ports as it can be > > difficult to connect "fast enough" to get the initial menu display > > > > Signed-off-by: Benjamin Herrenschmidt > > I am OK with this patch but it does not seem to apply on master. Please > rebase and repost. Odd, I must have screwed up something, will do. Thanks ! Cheers, Ben. ___ Grub-devel mailing list Grub-devel@gnu.org https://lists.gnu.org/mailman/listinfo/grub-devel
Re: [PATCH] menu: Add ctrl-L to refresh the menu
On Thu, 2022-09-22 at 19:36 +0200, Daniel Kiper wrote: > On Wed, Aug 31, 2022 at 06:24:28PM +1000, Benjamin Herrenschmidt > wrote: > > This is useful on cloud instances with remote serial ports as it > > can be > > difficult to connect "fast enough" to get the initial menu display > > > > Signed-off-by: Benjamin Herrenschmidt > > I am OK with this patch but it does not seem to apply on master. > Please > rebase and repost. Hrm... what am I missing ? My clone of master is at d9b4638c50b16d4722e66d334e2c1a674b4a45cc (no commit since Aug 20) I've just run git am on the email I sent on Aug 31 (retreived from my local grub-devel archive) and it applied fine on top of the above. Cheers, Ben. ___ Grub-devel mailing list Grub-devel@gnu.org https://lists.gnu.org/mailman/listinfo/grub-devel
Re: [PATCH v2 0/5] Automatic TPM Disk Unlock
I really like the "key protector" interface as more of a generic
provider interface. It seems like a great way to extend and create new
unlock scenarios such as using security tokens, or chaining
cryptographic features. If the interface is stable enough, it allows
third party projects to create their own protector modules, so we don't
wind up with people having to maintain full forks as happened with
TrustedGrub.
To support this extensibility, I think labels for providers should be
user-defined. The following is just scratching the surface of what's
possible:
tpm2_key_provider --name=platform --keyfile=/path/to/key
passphrase_key_provider --name=pass
xor_key_provider --name=combined --provider=platform --provider=pass
cryptomount DISK1 --provider=combined
This would retrieve a key from the TPM, ask the user for a passphrase
and then combine the two keys using xor, using the result to unlock the
device. More advanced examples could use HSMs or biometric interfaces.
I think there only needs to be a few small changes to the interface
(currently grub_key_protector) for this to happen:
1. It should be designed to store a reference to the context object
2. It should contain a function to cleanup the context object, wiping
any sensitive data.
3. It should contain a user supplied label, and all key protector
commands should require a user supplied label if they are registered.
4. The current name field should be renamed to 'module_name', for
removing entries when the module is unloaded.
Basically something like:
struct grub_key_producer
{
struct grub_key_producer *next;
struct grub_key_producer **prev;
const char *name;
const char *module_name;
void * ctx;
grub_err_t (*recover_key) (grub_uint8_t **key, grub_size_t *key_size);
grub_err_t (*destroy_ctx) (void * ctx);
};
I'll try and get a patch against master out this weekend supporting
passphrase/keyfile providers.
--
Max Vohra
On 2/1/22 05:02, Hernan Gatta wrote:
Updates since v1:
1. One key can unlock multiple disks:
It is now possible to use key protectors with cryptomount's -a and -b
options.
2. No passphrase prompt on error if key protector(s) specified:
cryptomount no longer prompts for a passphrase if key protectors are
specified but fail to provide a working unlock key seeing as the user
explicitly requested unlocking via key protectors.
3. Key protector parameterization is separate:
Previously, one would parameterize a key protector via a colon-separated
argument list nested within a cryptomount argument. Now, key protectors are
expected to provide an initialization function, if necessary.
As such, instead of:
cryptomount -k tpm2:mode=srk:keyfile=KEYFILE:pcrs=7,11...
one now writes:
tpm2_key_protector_init --mode=srk --keyfile=KEYFILE --pcrs=7,11 ...
cryptomount -k tpm2
Additionally, one may write:
cryptomount -k protector_1 -k protector_2 ...
where cryptomount will try each in order on failure.
4. Standard argument parsing:
The TPM2 key protector now uses 'struct grub_arg_option' and the
grub-protect
tool uses 'struct argp_option'. Additionally, common argument parsing
functionality is now shared between the module and the tool.
5. More useful messages:
Both the TPM2 module and the grub-protect tool now provide more useful
messages to help the user learn how to use their functionality (--help and
--usage) as well as to determine what is wrong, if anything. Furthermore,
the
module now prints additional debug output to help diagnose problems.
I forgot to mention last time that this patch series intends to address:
https://bugzilla.redhat.com/show_bug.cgi?id=1854177
Previous series:
https://lists.gnu.org/archive/html/grub-devel/2022-01/msg00125.html
Thank you,
Hernan
Signed-off-by: Hernan Gatta
Hernan Gatta (5):
protectors: Add key protectors framework
tpm2: Add TPM Software Stack (TSS)
protectors: Add TPM2 Key Protector
cryptodisk: Support key protectors
util/grub-protect: Add new tool
.gitignore |1 +
Makefile.util.def | 19 +
configure.ac |1 +
grub-core/Makefile.am |1 +
grub-core/Makefile.core.def| 11 +
grub-core/disk/cryptodisk.c| 166 +++-
grub-core/kern/protectors.c| 75 ++
grub-core/tpm2/args.c | 129
grub-core/tpm2/buffer.c| 145
grub-core/tpm2/module.c| 710 +
grub-core/tpm2/mu.c| 807
grub-core/tpm2/tcg2.c | 143
grub-core/tpm2/tpm2.c | 711 +
include/grub/cryptodisk.h | 14 +
include/grub/protector.h | 48 ++
include/grub/tpm2/buffer.h | 65 ++
include/grub
