Re: [PATCH] grub-core/disk/cryptodisk.c: Fix unintentional integer overflow
Hi Alec,
This looks good, thanks for fixing it.
On Thursday, 2022-10-13 at 22:13:44 +01, Alec Brown wrote:
> In the function grub_cryptodisk_endecrypt(), a for loop is incrementing the
> variable i by (1U << log_sector_size). The variable i is of type grub_size_t
> which is a 64-bit unsigned integer on x86_64 architecture. On the other hand,
> 1U
> is a 32-bit unsigned integer. By performing a left shift between these two
> values of different types, there may be potential for an integer overflow. To
> avoid this, we replace 1U with (grub_size_t) 1.
>
> Fixes: CID 307788
>
> Signed-off-by: Alec Brown
Reviewed-by: Darren Kenny
Thanks,
Darren.
> ---
> grub-core/disk/cryptodisk.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/grub-core/disk/cryptodisk.c b/grub-core/disk/cryptodisk.c
> index 9f5dc7acb..cdcb882ca 100644
> --- a/grub-core/disk/cryptodisk.c
> +++ b/grub-core/disk/cryptodisk.c
> @@ -239,7 +239,7 @@ grub_cryptodisk_endecrypt (struct grub_cryptodisk *dev,
> return (do_encrypt ? grub_crypto_ecb_encrypt (dev->cipher, data, data,
> len)
> : grub_crypto_ecb_decrypt (dev->cipher, data, data, len));
>
> - for (i = 0; i < len; i += (1U << log_sector_size))
> + for (i = 0; i < len; i += ((grub_size_t) 1 << log_sector_size))
> {
>grub_size_t sz = ((dev->cipher->cipher->blocksize
>+ sizeof (grub_uint32_t) - 1)
> --
> 2.27.0
___
Grub-devel mailing list
Grub-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/grub-devel
Re: [PATCH v2] tpm: Disable tpm verifier if tpm is not present
On Fri, Oct 07, 2022 at 01:37:10PM +0800, Michael Chang via Grub-devel wrote: > This helps to prevent out of memory error when reading large files via > disabling > tpm device as verifier has to read all content into memory in one chunk to > measure the hash and extend to tpm. How does this patch help when the TPM is present in the system? Daniel ___ Grub-devel mailing list Grub-devel@gnu.org https://lists.gnu.org/mailman/listinfo/grub-devel
Re: [PATCH v3] templates: introduce GRUB_TOP_LEVEL_* vars
On Wed, Oct 05, 2022 at 02:22:38AM -0700, Denton Liu wrote:
> 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, GRUB_TOP_LEVEL_XEN and
> GRUB_TOP_LEVEL_OS_PROBER variables to allow users to specify the
> top-level entry.
>
> Create grub_move_to_front() as a helper function which moves entries to
> the front of a list. This function does the heavy lifting of moving
> the menu entry to the front in each script.
>
> In 10_netbsd, since there isn't an explicit list variable, extract the
> items that are being iterated through into a list so that we can
> optionally apply grub_move_to_front() to the list before the loop.
>
> Signed-off-by: Denton Liu
I skimmed through the patch and it looks more or less good to me. Just
one nit below...
Please repost the patch by starting new thread instead of replying to
old one and add to CC following addresses: mathieu.desnoy...@efficios.com,
rharw...@redhat.com, samuel.thiba...@ens-lyon.org, debian-...@lists.debian.org,
xen-de...@lists.xenproject.org.
[...]
> diff --git a/util/grub-mkconfig_lib.in b/util/grub-mkconfig_lib.in
> index 634bc8a50..f4ae41f86 100644
> --- a/util/grub-mkconfig_lib.in
> +++ b/util/grub-mkconfig_lib.in
> @@ -218,6 +218,30 @@ version_sort ()
> esac
> }
>
> +# Given an item as the first argument and a list as the subsequent arguments,
> +# returns the list with the first argument moved to the front if it exists in
> +# the list.
> +grub_move_to_front ()
> +{
> + item="$1"
> + shift
> +
> + item_found=false
> + for i in "$@"; do
> +if [ "x$i" = "x$item" ]; then
> + item_found=true
> +fi
> + done
> +
> + if [ "x$item_found" = xtrue ]; then
> +echo "$item"
> + fi
> + for i in "$@"; do
> +if [ "x$i" = "x$item" ]; then continue; fi
I prefer
if [ "x$i" = "x$item" ]; then
continue;
fi
Daniel
___
Grub-devel mailing list
Grub-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/grub-devel
Re: [PATCH v4 1/1] Add support for grub-emu to kexec Linux menu entries
On Tue, Oct 04, 2022 at 03:16:48PM -0400, Robbie Harwood wrote:
> From: Raymund Will
>
> The GRUB emulator is used as a debugging utility but it could also be
> used as a user-space bootloader if there is support to boot an operating
> system.
>
> The Linux kernel is already able to (re)boot another kernel via the
> kexec boot mechanism. So the grub-emu tool could rely on this feature
> and have linux and initrd commands that are used to pass a kernel,
> initramfs image and command line parameters to kexec for booting a
> selected menu entry.
>
> By default the systemctl kexec option is used so systemd can shutdown
> all of the running services before doing a reboot using kexec. But if
> this is not present, it can fall back to executing the kexec user-space
> tool directly. The ability to force a kexec-reboot when systemctl kexec
> fails must only be used in controlled environments to avoid possible
> filesystem corruption and data loss.
>
> Signed-off-by: Raymund Will
> Signed-off-by: John Jolly
> Signed-off-by: Javier Martinez Canillas
> Signed-off-by: Robbie Harwood
> ---
> grub-core/Makefile.am| 1 +
> grub-core/Makefile.core.def | 2 +-
> grub-core/kern/emu/main.c| 4 +
> grub-core/kern/emu/misc.c| 18 +++-
> grub-core/loader/emu/linux.c | 181 +++
> include/grub/emu/exec.h | 4 +-
> include/grub/emu/hostfile.h | 3 +-
> include/grub/emu/misc.h | 3 +
Please document this functionality in the user documentation.
> 8 files changed, 212 insertions(+), 4 deletions(-)
> create mode 100644 grub-core/loader/emu/linux.c
>
> diff --git a/grub-core/Makefile.am b/grub-core/Makefile.am
> index ee88e44e97..80e7a83edf 100644
> --- a/grub-core/Makefile.am
> +++ b/grub-core/Makefile.am
> @@ -307,6 +307,7 @@ KERNEL_HEADER_FILES +=
> $(top_srcdir)/include/grub/emu/net.h
> KERNEL_HEADER_FILES += $(top_srcdir)/include/grub/emu/hostdisk.h
> KERNEL_HEADER_FILES += $(top_srcdir)/include/grub/emu/hostfile.h
> KERNEL_HEADER_FILES += $(top_srcdir)/include/grub/extcmd.h
> +KERNEL_HEADER_FILES += $(top_srcdir)/include/grub/emu/exec.h
> if COND_GRUB_EMU_SDL
> KERNEL_HEADER_FILES += $(top_srcdir)/include/grub/sdl.h
> endif
> diff --git a/grub-core/Makefile.core.def b/grub-core/Makefile.core.def
> index 7159948721..5350408601 100644
> --- a/grub-core/Makefile.core.def
> +++ b/grub-core/Makefile.core.def
> @@ -1816,9 +1816,9 @@ module = {
>arm64 = loader/arm64/linux.c;
>riscv32 = loader/riscv/linux.c;
>riscv64 = loader/riscv/linux.c;
> + emu = loader/emu/linux.c;
>common = loader/linux.c;
>common = lib/cmdline.c;
> - enable = noemu;
> };
>
> module = {
> diff --git a/grub-core/kern/emu/main.c b/grub-core/kern/emu/main.c
> index 44e087e988..855b11c3de 100644
> --- a/grub-core/kern/emu/main.c
> +++ b/grub-core/kern/emu/main.c
> @@ -107,6 +107,7 @@ static struct argp_option options[] = {
> N_("use GRUB files in the directory DIR [default=%s]"), 0},
>{"verbose", 'v', 0, 0, N_("print verbose messages."), 0},
>{"hold", 'H', N_("SECS"), OPTION_ARG_OPTIONAL, N_("wait until a
> debugger will attach"), 0},
> + {"kexec", 'X', 0, 0, N_("use kexec to boot Linux kernels via
> systemctl (pass twice to enable dangerous fallback to non-systemctl)."), 0},
>{ 0, 0, 0, 0, 0, 0 }
> };
>
> @@ -164,6 +165,9 @@ argp_parser (int key, char *arg, struct argp_state *state)
> case 'v':
>verbosity++;
>break;
> +case 'X':
> + grub_util_set_kexecute ();
> + break;
>
> case ARGP_KEY_ARG:
>{
> diff --git a/grub-core/kern/emu/misc.c b/grub-core/kern/emu/misc.c
> index d0e7a107e7..521220b49d 100644
> --- a/grub-core/kern/emu/misc.c
> +++ b/grub-core/kern/emu/misc.c
> @@ -39,6 +39,7 @@
> #include
>
> int verbosity;
> +int kexecute;
>
> void
> grub_util_warn (const char *fmt, ...)
> @@ -82,7 +83,7 @@ grub_util_error (const char *fmt, ...)
>vfprintf (stderr, fmt, ap);
>va_end (ap);
>fprintf (stderr, ".\n");
> - exit (1);
> + grub_exit ();
> }
>
> void *
> @@ -153,6 +154,9 @@ xasprintf (const char *fmt, ...)
> void
> grub_exit (void)
> {
> +#if defined (GRUB_KERNEL)
> + grub_reboot ();
> +#endif
>exit (1);
> }
> #endif
> @@ -214,3 +218,15 @@ grub_util_load_image (const char *path, char *buf)
>
>fclose (fp);
> }
> +
> +void
> +grub_util_set_kexecute (void)
> +{
> + kexecute++;
> +}
> +
> +int
> +grub_util_get_kexecute (void)
> +{
> + return kexecute;
> +}
> diff --git a/grub-core/loader/emu/linux.c b/grub-core/loader/emu/linux.c
> new file mode 100644
> index 00..bdcdbb0ff4
> --- /dev/null
> +++ b/grub-core/loader/emu/linux.c
> @@ -0,0 +1,181 @@
> +/*
> + * GRUB -- GRand Unified Bootloader
> + * Copyright (C) 2006,2007,2008,2009,2010 Free Software Foundation, Inc.
s/2006,2007,2008,2009,2010/2022/
> + *
> + * GRUB is free software: you can redistribute it and/or modify
> + * it under the terms of the GNU Gene
Re: [PATCH v4 2/6] linux/arm: unify ARM/arm64 vs Xen PE/COFF header handling
On Thu, Sep 08, 2022 at 03:30:13PM +0200, Ard Biesheuvel wrote:
> Xen has its own version of the image header, to account for the
> additional PE/COFF header fields. Since we are adding references to
> those in the shared EFI loader code, update the common definitions
> and drop the Xen specific one which no longer has a purpose.
>
> Signed-off-by: Ard Biesheuvel
> ---
> grub-core/loader/arm64/linux.c| 12 +-
> grub-core/loader/arm64/xen_boot.c | 23
> include/grub/arm/linux.h | 6 +
> include/grub/arm64/linux.h| 4
> include/grub/efi/efi.h| 4 +++-
> 5 files changed, 24 insertions(+), 25 deletions(-)
>
> diff --git a/grub-core/loader/arm64/linux.c b/grub-core/loader/arm64/linux.c
> index b5b559c236e0..7c0f17cf933d 100644
> --- a/grub-core/loader/arm64/linux.c
> +++ b/grub-core/loader/arm64/linux.c
> @@ -49,8 +49,13 @@ static grub_addr_t initrd_start;
> static grub_addr_t initrd_end;
>
> grub_err_t
> -grub_arch_efi_linux_check_image (struct linux_arch_kernel_header * lh)
> +grub_arch_efi_linux_load_image_header (grub_file_t file,
> + struct linux_arch_kernel_header * lh)
> {
> + grub_file_seek (file, 0);
> + if (grub_file_read (file, lh, sizeof (*lh)) < (long) sizeof (*lh))
s/(long)/(grub_ssize_t)/
Please mention this change in the commit message.
> +return grub_error(GRUB_ERR_FILE_READ_ERROR, "failed to read Linux image
> header");
> +
>if ((lh->code0 & 0x) != GRUB_PE32_MAGIC)
> return grub_error (GRUB_ERR_NOT_IMPLEMENTED_YET,
> N_("plain image kernel not supported - rebuild with
> CONFIG_(U)EFI_STUB enabled"));
> @@ -301,10 +306,7 @@ grub_cmd_linux (grub_command_t cmd __attribute__
> ((unused)),
>
>kernel_size = grub_file_size (file);
>
> - if (grub_file_read (file, &lh, sizeof (lh)) < (long) sizeof (lh))
> -return grub_errno;
> -
> - if (grub_arch_efi_linux_check_image (&lh) != GRUB_ERR_NONE)
> + if (grub_arch_efi_linux_load_image_header (file, &lh) != GRUB_ERR_NONE)
> goto fail;
>
>grub_loader_unset();
> diff --git a/grub-core/loader/arm64/xen_boot.c
> b/grub-core/loader/arm64/xen_boot.c
> index 22cc25eccd9d..e5895ee78218 100644
> --- a/grub-core/loader/arm64/xen_boot.c
> +++ b/grub-core/loader/arm64/xen_boot.c
> @@ -31,7 +31,6 @@
> #include
> #include
> #include
> -#include/* required by struct xen_hypervisor_header */
> #include
> #include
>
> @@ -65,18 +64,6 @@ enum module_type
> };
> typedef enum module_type module_type_t;
>
> -struct xen_hypervisor_header
> -{
> - struct linux_arm64_kernel_header efi_head;
> -
> - /* This is always PE\0\0. */
> - grub_uint8_t signature[GRUB_PE32_SIGNATURE_SIZE];
> - /* The COFF file header. */
> - struct grub_pe32_coff_header coff_header;
> - /* The Optional header. */
> - struct grub_pe64_optional_header optional_header;
> -};
> -
> struct xen_boot_binary
> {
>struct xen_boot_binary *next;
> @@ -452,7 +439,7 @@ static grub_err_t
> grub_cmd_xen_hypervisor (grub_command_t cmd __attribute__ ((unused)),
>int argc, char *argv[])
> {
> - struct xen_hypervisor_header sh;
> + struct linux_arm64_kernel_header lh;
>grub_file_t file = NULL;
>
>grub_dl_ref (my_mod);
> @@ -467,10 +454,7 @@ grub_cmd_xen_hypervisor (grub_command_t cmd
> __attribute__ ((unused)),
>if (!file)
> goto fail;
>
> - if (grub_file_read (file, &sh, sizeof (sh)) != (long) sizeof (sh))
> -goto fail;
> - if (grub_arch_efi_linux_check_image
> - ((struct linux_arch_kernel_header *) &sh) != GRUB_ERR_NONE)
> + if (grub_arch_efi_linux_load_image_header (file, &lh) != GRUB_ERR_NONE)
> goto fail;
>grub_file_seek (file, 0);
>
> @@ -484,7 +468,8 @@ grub_cmd_xen_hypervisor (grub_command_t cmd __attribute__
> ((unused)),
> return grub_errno;
>
>xen_hypervisor->is_hypervisor = 1;
> - xen_hypervisor->align = (grub_size_t) sh.optional_header.section_alignment;
> + xen_hypervisor->align
> += (grub_size_t) lh.coff_image_header.optional_header.section_alignment;
>
>xen_boot_binary_load (xen_hypervisor, file, argc, argv);
>if (grub_errno == GRUB_ERR_NONE)
> diff --git a/include/grub/arm/linux.h b/include/grub/arm/linux.h
> index bcd5a7eb186e..ea815db13417 100644
> --- a/include/grub/arm/linux.h
> +++ b/include/grub/arm/linux.h
> @@ -22,6 +22,8 @@
>
> #include "system.h"
>
> +#include
> +
> #define GRUB_LINUX_ARM_MAGIC_SIGNATURE 0x016f2818
>
> struct linux_arm_kernel_header {
> @@ -32,6 +34,10 @@ struct linux_arm_kernel_header {
>grub_uint32_t end; /* _edata */
>grub_uint32_t reserved2[3];
>grub_uint32_t hdr_offset;
> +
Please drop this empty line.
> +#if defined GRUB_MACHINE_EFI
> + struct grub_coff_image_header coff_image_header;
> +#endif
> };
>
> #if defined(__arm__)
> diff --git a/include/grub/arm64/linux.h b/include/grub/arm64/linux.h
> index 7e22b4ab6990..c5ea9e8f503a 100644
> --- a/include/grub
Re: [PATCH v4 1/6] efi: move MS-DOS stub out of generic PE header definition
On Thu, Sep 15, 2022 at 04:03:36PM +0200, Ard Biesheuvel wrote:
> On Thu, 15 Sept 2022 at 14:21, Leif Lindholm
> wrote:
> >
> > On Thu, Sep 08, 2022 at 15:30:12 +0200, Ard Biesheuvel wrote:
> > > The PE/COFF spec permits the COFF signature and file header to appear
> > > anywhere in the file, and the actual offset is recorded in 4 byte
> > > little endian field at offset 0x3c of the image.
> > >
> > > When GRUB is emitted as a PE/COFF binary, we reuse the 128 byte MS-DOS
> > > stub (even for non-x86 architectures), putting the COFF signature and
> > > file header at offset 0x80. However, other PE/COFF images may use
> > > different values, and non-x86 Linux kernels use an offset of 0x40
> > > instead.
> > >
> > > So let's get rid of the grub_pe32_header struct from pe32.h, given that
> > > it does not represent anything defined by the PE/COFF spec. Instead,
> > > introduce a minimal struct grub_msdos_image_header type based on the
> > > PE/COFF spec's description of the image header, and use the offset
> > > recorded at file position 0x3c to discover the actual location of the PE
> > > signature and the COFF image header.
> > >
> > > The remaining fields are moved into a struct grub_coff_image_header,
> > > which we will use later to access COFF header fields of arbitrary
> > > images (and which may therefore appear at different offsets)
> > >
> > > Signed-off-by: Ard Biesheuvel
> > > ---
> > > grub-core/kern/efi/efi.c | 8 ++--
> > > include/grub/efi/pe32.h | 16
> > > 2 files changed, 18 insertions(+), 6 deletions(-)
> > >
> > > diff --git a/grub-core/kern/efi/efi.c b/grub-core/kern/efi/efi.c
> > > index e8a976a22f15..f85587d66635 100644
> > > --- a/grub-core/kern/efi/efi.c
> > > +++ b/grub-core/kern/efi/efi.c
> > > @@ -302,7 +302,8 @@ grub_addr_t
> > > grub_efi_modules_addr (void)
> > > {
> > >grub_efi_loaded_image_t *image;
> > > - struct grub_pe32_header *header;
> > > + struct grub_msdos_image_header *dos_header;
> > > + struct grub_coff_image_header *header;
> > >struct grub_pe32_coff_header *coff_header;
> > >struct grub_pe32_section_table *sections;
> > >struct grub_pe32_section_table *section;
> > > @@ -313,7 +314,10 @@ grub_efi_modules_addr (void)
> > >if (! image)
> > > return 0;
> > >
> > > - header = image->image_base;
> > > + dos_header = (struct grub_msdos_image_header *)image->image_base;
> > > +
> > > + header = (struct grub_coff_image_header *) ((char *) dos_header
> > > + +
> > > dos_header->pe_signature_offset);
> > >coff_header = &(header->coff_header);
> >
> > This is where I get semantically confused.
> > We now have a coff_image_header called header (pointing at the space
> > for the PE\0\0 signature, which comes immediately before the COFF
> > header, and isn't part of it).
> >
> > And then we have a pe32_coff_header called coff_header, pointing at
> > the machine field (the start) of the COFF header.
> >
> > Since "header" is no longer referring to an image header, could we
> > chuck out the signature as well from the structure and add
> > GRUB_PE32_SIGNATURE_SIZE when calculating?
> >
> > I.e. drop "header" altogether and do:
> >
> > dos_header = (struct grub_msdos_image_header *)image->image_base;
> >
> > coff_header = (struct grub_coff_image_header *) ((char *) dos_header
> > +
> > dos_header->pe_signature_offset
> > + GRUB_PE32_SIGNATURE_SIZE
> > );
> > ?
> >
>
> I suppose that should work, but it does mean we have to add a 'char
> signature[GRUB_PE32_SIGNATURE_SIZE];' member to the existing structs
> that incorporate grub_coff_image_header, along with adding
> GRUB_PE32_SIGNATURE_SIZE every time we refer to the PE header offset.
After checking the PE/COFF spec it seems to me the grub_coff_image_header
should be renamed to grub_pe_image_header. Then everything should be OK.
Daniel
___
Grub-devel mailing list
Grub-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/grub-devel
Re: [PATCH v4 3/6] linux/arm: account for COFF headers appearing at unexpected offsets
On Thu, Sep 08, 2022 at 03:30:14PM +0200, Ard Biesheuvel wrote:
> The way we load the Linux and PE/COFF image headers depends on a fixed
> placement of the COFF header at offset 0x40 into the file. This is a
> reasonable default, given that this is where Linux emits it today.
> However, in order to comply with the PE/COFF spec, which allows this
> header to appear anywhere in the file, let's ensure that we read the
> header from where it actually appears in the file if it is not located
> at offset 0x40.
>
> Signed-off-by: Ard Biesheuvel
> ---
> grub-core/loader/arm64/linux.c | 15 +++
> 1 file changed, 15 insertions(+)
>
> diff --git a/grub-core/loader/arm64/linux.c b/grub-core/loader/arm64/linux.c
> index 7c0f17cf933d..56ba8d0a6ea3 100644
> --- a/grub-core/loader/arm64/linux.c
> +++ b/grub-core/loader/arm64/linux.c
> @@ -63,6 +63,21 @@ grub_arch_efi_linux_load_image_header (grub_file_t file,
>grub_dprintf ("linux", "UEFI stub kernel:\n");
>grub_dprintf ("linux", "PE/COFF header @ %08x\n", lh->hdr_offset);
>
> + /*
> + * The PE/COFF spec permits the COFF header to appear anywhere in the
> file, so
> + * we need to double check whether it was where we expected it, and if
> not, we
> + * must load it from the correct offset into the coff_image_header field of
> + * struct linux_arch_kernel_header.
> + */
> + if ((grub_uint8_t *) lh + lh->hdr_offset != (grub_uint8_t *)
> &lh->coff_image_header)
> +{
> + grub_file_seek (file, lh->hdr_offset);
I would check if grub_file_seek() does not return -1.
> + if (grub_file_read (file, &lh->coff_image_header, sizeof(struct
> grub_coff_image_header))
> + != sizeof(struct grub_coff_image_header))
Missing spaces before "("...
> + return grub_error(GRUB_ERR_FILE_READ_ERROR, "failed to read COFF
> image header");
Please add missing spaces before return and after grub_error.
Daniel
___
Grub-devel mailing list
Grub-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/grub-devel
Re: [PATCH v4 4/6] efi/efinet: Don't close connections at fini_hw() time
On Thu, Sep 08, 2022 at 03:30:15PM +0200, Ard Biesheuvel wrote: > When GRUB runs on top of EFI firmware, it only has access to block and > network device abstractions exposed by the firmware, and it is up to the > firmware to quiesce the underlying hardware when exiting boot services > and handing over to the OS. > > This is especially important for network devices, to prevent incoming > packets from being DMA'd straight into memory after the OS has taken > over but before it has managed to reconfigure the network hardware. > > GRUB handles this by means of the grub_net_fini_hw() preboot hook, which > is executed before calling into the booted image. This means that all > network devices disappear or become inoperable before the EFI stub > executes on EFI targeted builds. This is problematic as it prevents the > EFI stub from calling back into GRUB provided protocols such as > LoadFile2 for the initrd, which we will provide in a subsequent patch. > > So add a flag that indicates to the network core that EFI network > devices should not be closed when grub_net_fini_hw() is called. > > Signed-off-by: Ard Biesheuvel > Reviewed-by: Heinrich Schuchardt Reviewed-by: Daniel Kiper Daniel ___ Grub-devel mailing list Grub-devel@gnu.org https://lists.gnu.org/mailman/listinfo/grub-devel
[PATCH 2/2] configure: Fix building with clang
Building the current code with clang and the latest gnulib fails due to the use of a variable-length-array (vla) warning, which turns in to an error due to the presence of the -Werror during the build. The gnulib team stated that their code should not be built with -Werror. At present, the only way to do this is for the complete code-base, by using the --disable-werror option to configure. Rather than doing this, and failing to gain any benefit that it provides, instead, if building with clang, this patch makes it possible to specifically not error on vlas, while retaining the -Werror functionality otherwise. Signed-off-by: Darren Kenny --- configure.ac | 4 1 file changed, 4 insertions(+) diff --git a/configure.ac b/configure.ac index 1348b06a985a..93626b7982d4 100644 --- a/configure.ac +++ b/configure.ac @@ -1939,6 +1939,10 @@ AC_ARG_ENABLE([werror], if test x"$enable_werror" != xno ; then TARGET_CFLAGS="$TARGET_CFLAGS -Werror" HOST_CFLAGS="$HOST_CFLAGS -Werror" + if test "x$grub_cv_cc_target_clang" = xyes; then +TARGET_CFLAGS="$TARGET_CFLAGS -Wno-error=vla" +HOST_CFLAGS="$HOST_CFLAGS -Wno-error=vla" + fi fi TARGET_CPP="$TARGET_CC -E" -- 2.31.1 ___ Grub-devel mailing list Grub-devel@gnu.org https://lists.gnu.org/mailman/listinfo/grub-devel
[PATCH 1/2] gnulib: Provide abort() implementation for gnulib
The recent gnulib updates require an implemention of abort(), but the
current macro provided by changeset:
cd37d3d3916c gnulib: Drop no-abort.patch
to config.h.in does not work with the clang compiler since it doesn't
provide a __builtin_trap implementation, so this element of the
changeset needs to be reverted, and replaced.
After some discussion with Vladimir 'phcoder' Serbinenko and Daniel
Kiper it was suggested to bring back in the change from the changeset:
db7337a3d353 "* grub-core/gnulib/regcomp.c (regerror): ..."
Which implements abort() as an inline call to grub_abort(), but since
that was made static by changeset:
a8f15bceeafe "* grub-core/kern/misc.c (grub_abort): Make static"
it is also necessary to revert the specific part that makes it a static
function too.
Signed-off-by: Darren Kenny
---
config.h.in | 10 --
grub-core/kern/misc.c | 2 +-
grub-core/lib/posix_wrap/stdlib.h | 6 ++
include/grub/misc.h | 5 +
4 files changed, 8 insertions(+), 15 deletions(-)
diff --git a/config.h.in b/config.h.in
index 01dcbbfc82f0..4d1e50eba79c 100644
--- a/config.h.in
+++ b/config.h.in
@@ -137,16 +137,6 @@ typedef __UINT_FAST32_TYPE__ uint_fast32_t;
void * \
reallocarray (void *ptr, unsigned int nmemb, unsigned int size);
#define _GL_INLINE_HEADER_END _Pragma ("GCC diagnostic pop")
-
-/*
- * We don't have an abort() for gnulib to call in regexp. Because gnulib is
- * built as a separate object that is then linked with, it doesn't have any
- * of our headers or functions around to use - so we unfortunately can't
- * print anything. Builds of emu include the system's stdlib, which includes
- * a prototype for abort(), so leave this as a macro that doesn't take
- * arguments.
- */
-#define abort __builtin_trap
# endif /* !_GL_INLINE_HEADER_BEGIN */
/* gnulib doesn't build cleanly with older compilers. */
diff --git a/grub-core/kern/misc.c b/grub-core/kern/misc.c
index 6c0221cc336d..dfae4f9d7897 100644
--- a/grub-core/kern/misc.c
+++ b/grub-core/kern/misc.c
@@ -1249,7 +1249,7 @@ grub_printf_fmt_check (const char *fmt, const char
*fmt_expected)
/* Abort GRUB. This function does not return. */
-static void __attribute__ ((noreturn))
+void __attribute__ ((noreturn))
grub_abort (void)
{
grub_printf ("\nAborted.");
diff --git a/grub-core/lib/posix_wrap/stdlib.h
b/grub-core/lib/posix_wrap/stdlib.h
index 148e9d94bde0..f5279756abef 100644
--- a/grub-core/lib/posix_wrap/stdlib.h
+++ b/grub-core/lib/posix_wrap/stdlib.h
@@ -58,4 +58,10 @@ abs (int c)
return (c >= 0) ? c : -c;
}
+static inline void __attribute__ ((noreturn))
+abort (void)
+{
+ grub_abort ();
+}
+
#endif
diff --git a/include/grub/misc.h b/include/grub/misc.h
index 892ac6a42dda..ddac3aae8bcc 100644
--- a/include/grub/misc.h
+++ b/include/grub/misc.h
@@ -385,6 +385,7 @@ char *EXPORT_FUNC(grub_xasprintf) (const char *fmt, ...)
__attribute__ ((format (GNU_PRINTF, 1, 2))) WARN_UNUSED_RESULT;
char *EXPORT_FUNC(grub_xvasprintf) (const char *fmt, va_list args)
WARN_UNUSED_RESULT;
void EXPORT_FUNC(grub_exit) (void) __attribute__ ((noreturn));
+void EXPORT_FUNC(grub_abort) (void) __attribute__ ((noreturn));
grub_uint64_t EXPORT_FUNC(grub_divmod64) (grub_uint64_t n,
grub_uint64_t d,
grub_uint64_t *r);
@@ -454,10 +455,6 @@ void EXPORT_FUNC(grub_reboot) (void) __attribute__
((noreturn));
void grub_reboot (void) __attribute__ ((noreturn));
#endif
-#if defined (__clang__) && !defined (GRUB_UTIL)
-void __attribute__ ((noreturn)) EXPORT_FUNC (abort) (void);
-#endif
-
#ifdef GRUB_MACHINE_PCBIOS
/* Halt the system, using APM if possible. If NO_APM is true, don't
* use APM even if it is available. */
--
2.31.1
___
Grub-devel mailing list
Grub-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/grub-devel
[PATCH 0/2] Fix building with clang
The abiltiy to build with clang was broken in the last release after the upgrade of gnulib. There were two main issues: - The use of __builtin_trap in the abort() macro. This builtin doesn't exist for clang builds After some discussion between Daniel and Vladimir, it was requested that I should revert some past changes in this area, and re-introduce the use of grub_abort(). - The is some use of variable length arrays (vla) in gnulib's code, and when an attempt was made to resolve this in gnulib itself, I was informed that we shouldn't be building gnulib with -Werror. Rather than totally disabling -Werror, it seemed better to just limit it for the specific case that is causing problems, i.e. vla. Thanks, Darren. Darren Kenny (2): gnulib: Provide abort() implementation for gnulib configure: Fix building with clang config.h.in | 10 -- configure.ac | 4 grub-core/kern/misc.c | 2 +- grub-core/lib/posix_wrap/stdlib.h | 6 ++ include/grub/misc.h | 5 + 5 files changed, 12 insertions(+), 15 deletions(-) -- 2.31.1 ___ Grub-devel mailing list Grub-devel@gnu.org https://lists.gnu.org/mailman/listinfo/grub-devel
[PATCH v2] grub-core/disk/cryptodisk.c: Fix unintentional integer overflow
In the function grub_cryptodisk_endecrypt(), a for loop is incrementing the
variable i by (1U << log_sector_size). The variable i is of type grub_size_t
which is a 64-bit unsigned integer on x86_64 architecture. On the other hand, 1U
is a 32-bit unsigned integer. By performing a left shift on a 32-bit value and
assigning it to a 64-bit variable, the 64-bit variable may have incorrect values
in the high 32-bits if the shift has an overflow. To avoid this, we replace 1U
with (grub_size_t) 1.
Fixes: CID 307788
Signed-off-by: Alec Brown
---
There was a mistake in v1 of the commit message describing the issue in the
code. This version fixes the commit message so that it's accurate.
grub-core/disk/cryptodisk.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/grub-core/disk/cryptodisk.c b/grub-core/disk/cryptodisk.c
index 9f5dc7acb..cdcb882ca 100644
--- a/grub-core/disk/cryptodisk.c
+++ b/grub-core/disk/cryptodisk.c
@@ -239,7 +239,7 @@ grub_cryptodisk_endecrypt (struct grub_cryptodisk *dev,
return (do_encrypt ? grub_crypto_ecb_encrypt (dev->cipher, data, data, len)
: grub_crypto_ecb_decrypt (dev->cipher, data, data, len));
- for (i = 0; i < len; i += (1U << log_sector_size))
+ for (i = 0; i < len; i += ((grub_size_t) 1 << log_sector_size))
{
grub_size_t sz = ((dev->cipher->cipher->blocksize
+ sizeof (grub_uint32_t) - 1)
--
2.27.0
___
Grub-devel mailing list
Grub-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/grub-devel
