Re: [PATCH] kern/efi/mm: Double the default heap size

2022-08-21 Thread Hector Martin via Grub-devel
On 21/08/2022 21.35, Daniel Axtens wrote:
> Hi Hector,
> 
> Thanks for your patch and for taking the trouble to put it together.
> 
>> GRUB is already running out of memory on Apple M1 systems, causing
>> graphics init to fail, as of the latest Git changes. Since dynamic
>> growing of the heap isn't done yet, double the default heap size for
>> now.
> 
> Huh, weird - those changes have landed in git, see commit 887f98f0db43
> ("mm: Allow dynamically requesting additional memory regions") for the
> overarching support and commit 1df2934822df ("kern/efi/mm: Implement
> runtime addition of pages"). It's not done on PowerPC, but if you're in
> EFI-land then it should complete.
> 
> The only reason I can think of off the top of my head where you would be
> having issues that your patch fixes is if we somehow need more memory to
> even get to the point where we can ask firmware for more memory. I
> suppose that's within the realm of possibility.

Interesting. I missed the indirection through the function pointer...
but either way, I do indeed have those commits in the broken tree that
Arch Linux ARM started shipping yesterday (0c6c1aff2a, which isn't
actually current master but it's from a couple weeks ago). The previous
version was 2f4430cc0, which doesn't have it, so I wonder if there was
actually a regression involved?

What I see is that GRUB briefly flashes an out of memory error and fails
to set the graphics mode, then ends up in text mode. My best guess
without digging further is that it fails to allocate a framebuffer or
console text buffer (since these machines have higher resolution screens
than most, this might not have come up elsewhere). But I don't see why
that would have to happen before it's allowed?

> I f my maths are right, this bumps up the initial allocation from 1M to
> 2M.

Correct.

> I think your experience tends to disprove the hypothesis that we
> could get away with a very small initial allocation (which was the
> thinking when the initial dynamic allocation patch set went in), so I'm
> wondering if we should take this opportunity to allocate 16M or 32M or
> something. My powerpc proposal kept the initial allocation at 32MB, I
> think that's probably sane for EFI too?

I think that makes sense.

- Hector

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


Re: [PATCH] kern/efi/mm: Double the default heap size

2022-08-21 Thread Daniel Axtens
Hi Hector,

Thanks for your patch and for taking the trouble to put it together.

> GRUB is already running out of memory on Apple M1 systems, causing
> graphics init to fail, as of the latest Git changes. Since dynamic
> growing of the heap isn't done yet, double the default heap size for
> now.

Huh, weird - those changes have landed in git, see commit 887f98f0db43
("mm: Allow dynamically requesting additional memory regions") for the
overarching support and commit 1df2934822df ("kern/efi/mm: Implement
runtime addition of pages"). It's not done on PowerPC, but if you're in
EFI-land then it should complete.

The only reason I can think of off the top of my head where you would be
having issues that your patch fixes is if we somehow need more memory to
even get to the point where we can ask firmware for more memory. I
suppose that's within the realm of possibility.

I f my maths are right, this bumps up the initial allocation from 1M to
2M. I think your experience tends to disprove the hypothesis that we
could get away with a very small initial allocation (which was the
thinking when the initial dynamic allocation patch set went in), so I'm
wondering if we should take this opportunity to allocate 16M or 32M or
something. My powerpc proposal kept the initial allocation at 32MB, I
think that's probably sane for EFI too?

Patrick, EFI is much more your area than it is mine, what do you think?

Kind regards,
Daniel

>
> Signed-off-by: Hector Martin 
> ---
>  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..377d8d3a1c1b 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_SIZE0x20
>  
>  static void *finish_mmap_buf = 0;
>  static grub_efi_uintn_t finish_mmap_size = 0;
> -- 
> 2.35.1
>
>
> ___
> 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


[PATCH 2/2] diskfilter: don't make a RAID array with more than 1024 disks

2022-08-21 Thread Daniel Axtens
This is 'belt and braces' with the last fix: we end up trying to use
too much memory in situations like corrupted Linux software raid setups
purporting to usew a huge number of disks. Simply refuse to permit such
configurations.

1024 is a bit arbitrary, yes, and I feel a bit like I'm tempting fate
here, but I think 1024 disks in an array (that grub has to read to boot!)
should be enough for anyone.

Signed-off-by: Daniel Axtens 
---
 grub-core/disk/diskfilter.c | 7 +++
 1 file changed, 7 insertions(+)

diff --git a/grub-core/disk/diskfilter.c b/grub-core/disk/diskfilter.c
index 4ac50320ef4e..79c5f4db940a 100644
--- a/grub-core/disk/diskfilter.c
+++ b/grub-core/disk/diskfilter.c
@@ -1046,6 +1046,13 @@ grub_diskfilter_make_raid (grub_size_t uuidlen, char 
*uuid, int nmemb,
   struct grub_diskfilter_pv *pv;
   grub_err_t err;
 
+  /* We choose not to support more than 1024 disks */
+  if (nmemb > 1024)
+{
+  grub_free (uuid);
+  return NULL;
+}
+
   switch (level)
 {
 case 1:
-- 
2.25.1


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


[PATCH 1/2] diskfilter: check calloc() result for NULL

2022-08-21 Thread Daniel Axtens
With wildly corrupt inputs, we can end up trying to calloc a very
large amount of memory, which will fail and give us a NULL pointer.
We need to check that to avoid a crash. (And, even if we blocked
such inputs, it is good practice to check the results of allocations
anyway.)

Signed-off-by: Daniel Axtens 
---
 grub-core/disk/diskfilter.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/grub-core/disk/diskfilter.c b/grub-core/disk/diskfilter.c
index 2edcff6e8987..4ac50320ef4e 100644
--- a/grub-core/disk/diskfilter.c
+++ b/grub-core/disk/diskfilter.c
@@ -1163,6 +1163,9 @@ grub_diskfilter_make_raid (grub_size_t uuidlen, char 
*uuid, int nmemb,
   array->lvs->segments->raid_member_size = disk_size;
   array->lvs->segments->nodes
 = grub_calloc (nmemb, sizeof (array->lvs->segments->nodes[0]));
+  if (array->lvs->segments->nodes == NULL)
+goto fail;
+
   array->lvs->segments->stripe_size = stripe_size;
   for (i = 0; i < nmemb; i++)
 {
-- 
2.25.1


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


[PATCH] templates: introduce GRUB_TOP_LEVEL_* vars

2022-08-21 Thread Denton Liu
A user may wish to use an image that is not sorted as the "latest"
version as the top-level entry. For example, in Arch Linux, if a user
has the LTS and regular kernels installed, `/boot/vmlinuz-linux-lts`
gets sorted as the "latest" compared to `/boot/vmlinuz-linux`. However,
a user may wish to use the regular kernel as the default with the LTS
only existing as a backup.

Introduce the GRUB_TOP_LEVEL_LINUX and GRUB_TOP_LEVEL_XEN variables to
allow users to specify the top-level entry.

Signed-off-by: Denton Liu 
---
 docs/grub.texi  |  5 +
 util/grub-mkconfig.in   |  2 ++
 util/grub-mkconfig_lib.in   | 21 +
 util/grub.d/10_linux.in |  4 
 util/grub.d/20_linux_xen.in |  7 +++
 5 files changed, 39 insertions(+)

diff --git a/docs/grub.texi b/docs/grub.texi
index 107f66ebc..54bd32882 100644
--- a/docs/grub.texi
+++ b/docs/grub.texi
@@ -1444,6 +1444,11 @@ for all respectively normal entries.
 The values of these options replace the values of @samp{GRUB_CMDLINE_LINUX}
 and @samp{GRUB_CMDLINE_LINUX_DEFAULT} for Linux and Xen menu entries.
 
+@item GRUB_TOP_LEVEL_LINUX
+@item GRUB_TOP_LEVEL_XEN
+If this option is provided, the given image file will be made the top-level
+entry if the image file is found in the scan.
+
 @item GRUB_EARLY_INITRD_LINUX_CUSTOM
 @itemx GRUB_EARLY_INITRD_LINUX_STOCK
 List of space-separated early initrd images to be loaded from @samp{/boot}.
diff --git a/util/grub-mkconfig.in b/util/grub-mkconfig.in
index 62335d027..f7ad160b4 100644
--- a/util/grub-mkconfig.in
+++ b/util/grub-mkconfig.in
@@ -233,6 +233,8 @@ export GRUB_DEFAULT \
   GRUB_CMDLINE_NETBSD \
   GRUB_CMDLINE_NETBSD_DEFAULT \
   GRUB_CMDLINE_GNUMACH \
+  GRUB_TOP_LEVEL_LINUX \
+  GRUB_TOP_LEVEL_XEN \
   GRUB_EARLY_INITRD_LINUX_CUSTOM \
   GRUB_EARLY_INITRD_LINUX_STOCK \
   GRUB_TERMINAL_INPUT \
diff --git a/util/grub-mkconfig_lib.in b/util/grub-mkconfig_lib.in
index 634bc8a50..5be49a07c 100644
--- a/util/grub-mkconfig_lib.in
+++ b/util/grub-mkconfig_lib.in
@@ -218,6 +218,27 @@ version_sort ()
esac
 }
 
+grub_move_entry_to_front ()
+{
+  entry="$1"
+  shift
+
+  entry_found=false
+  for i in "$@"; do
+if [ "x$i" = "x$entry" ]; then
+  entry_found=true
+fi
+  done
+
+  if [ "x$entry_found" = xtrue ]; then
+echo "$entry"
+  fi
+  for i in "$@"; do
+if [ "x$i" = "x$entry" ]; then continue; fi
+echo "$i"
+  done
+}
+
 # One layer of quotation is eaten by "" and the second by sed; so this turns
 # ' into \'.
 grub_quote () {
diff --git a/util/grub.d/10_linux.in b/util/grub.d/10_linux.in
index c6a1ec935..05e01fc85 100644
--- a/util/grub.d/10_linux.in
+++ b/util/grub.d/10_linux.in
@@ -202,6 +202,10 @@ submenu_indentation=""
 
 reverse_sorted_list=$(echo $list | tr ' ' '\n' | sed -e 's/\.old$/ 1/; / 1$/! 
s/$/ 2/' | version_sort -r | sed -e 's/ 1$/.old/; s/ 2$//')
 
+if [ "x$GRUB_TOP_LEVEL_LINUX" != x ]; then
+  reverse_sorted_list=$(grub_move_entry_to_front "$GRUB_TOP_LEVEL_LINUX" 
${reverse_sorted_list})
+fi
+
 is_top_level=true
 for linux in ${reverse_sorted_list}; do
   gettext_printf "Found linux image: %s\n" "$linux" >&2
diff --git a/util/grub.d/20_linux_xen.in b/util/grub.d/20_linux_xen.in
index 626aed40c..c32bb979f 100644
--- a/util/grub.d/20_linux_xen.in
+++ b/util/grub.d/20_linux_xen.in
@@ -245,6 +245,13 @@ submenu_indentation=""
 reverse_sorted_xen_list=$(echo ${xen_list} | tr ' ' '\n' | sed -e 's/\.old$/ 
1/; / 1$/! s/$/ 2/' | version_sort -r | sed -e 's/ 1$/.old/; s/ 2$//')
 reverse_sorted_linux_list=$(echo ${linux_list} | tr ' ' '\n' | sed -e 
's/\.old$/ 1/; / 1$/! s/$/ 2/' | version_sort -r | sed -e 's/ 1$/.old/; s/ 
2$//')
 
+if [ "x$GRUB_TOP_LEVEL_XEN" != x ]; then
+  reverse_sorted_xen_list=$(grub_move_entry_to_front "$GRUB_TOP_LEVEL_XEN" 
${reverse_sorted_xen_list})
+fi
+if [ "x$GRUB_TOP_LEVEL_LINUX" != x ]; then
+  reverse_sorted_linux_list=$(grub_move_entry_to_front "$GRUB_TOP_LEVEL_LINUX" 
${reverse_sorted_linux_list})
+fi
+
 is_top_level=true
 
 for current_xen in ${reverse_sorted_xen_list}; do
-- 
2.37.2


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