On Wed, Jun 12, 2024 at 04:57:07PM +0100, Mate Kukri wrote:
> Currently we load module sections at whatever alignment gcc+ld happened
> to dump into the ELF section header, which is often less then the page
> size. Since NX protections are page based, this alignment must be
> rounded up to page size on platforms supporting NX protections.
>
> This patch switches most EFI platforms to load module sections at 4kB
> page-aligned addresses. To do so, it adds an new per-arch function,
> grub_arch_dl_min_alignment(), which returns the alignment needed for
> dynamically loaded sections (in bytes). Currently it sets it to 4096
> when GRUB_MACHINE_EFI is true on x86_64, i386, arm, arm64, and emu, and
> 1-byte alignment on everything else.
>
> It then changes the allocation size computation and the loader code in
> grub_dl_load_segments() to align the locations and sizes up to these
> boundaries, and fills any added padding with zeros.
>
> All of this happens before relocations are applied, so the relocations
> factor that in with no change.
>
> Original-Author: Peter Jones <pjo...@redhat.com>
> Original-Author: Laszlo Ersek <ler...@redhat.com>
> Signed-off-by: Mate Kukri <mate.ku...@canonical.com>
> ---
> docs/grub-dev.texi | 6 ++---
> grub-core/kern/arm/dl.c | 13 +++++++++
> grub-core/kern/arm64/dl.c | 13 +++++++++
> grub-core/kern/dl.c | 53 ++++++++++++++++++++++++++-----------
> grub-core/kern/emu/full.c | 13 +++++++++
> grub-core/kern/i386/dl.c | 13 +++++++++
> grub-core/kern/ia64/dl.c | 9 +++++++
> grub-core/kern/mips/dl.c | 8 ++++++
> grub-core/kern/powerpc/dl.c | 9 +++++++
> grub-core/kern/riscv/dl.c | 13 +++++++++
> grub-core/kern/sparc64/dl.c | 9 +++++++
> grub-core/kern/x86_64/dl.c | 13 +++++++++
> include/grub/dl.h | 2 ++
> 13 files changed, 155 insertions(+), 19 deletions(-)
>
> diff --git a/docs/grub-dev.texi b/docs/grub-dev.texi
> index 1276c5930..2f782cda5 100644
> --- a/docs/grub-dev.texi
> +++ b/docs/grub-dev.texi
> @@ -996,9 +996,9 @@ declare startup asm file ($cpu_$platform_startup) as well
> as any other files
> (e.g. init.c and callwrap.S) (e.g. $cpu_$platform =
> kern/$cpu/$platform/init.c).
> At this stage you will also need to add dummy dl.c and cache.S with functions
> grub_err_t grub_arch_dl_check_header (void *ehdr), grub_err_t
> -grub_arch_dl_relocate_symbols (grub_dl_t mod, void *ehdr) (dl.c) and
> -void grub_arch_sync_caches (void *address, grub_size_t len) (cache.S). They
> -won't be used for now.
> +grub_arch_dl_relocate_symbols (grub_dl_t mod, void *ehdr) (dl.c),
> grub_uint32_t
> +grub_arch_dl_min_alignment (void), and void grub_arch_sync_caches (void
> +*address, grub_size_t len) (cache.S). They won't be used for now.
>
> You will need to create directory include/$cpu/$platform and a file
> include/$cpu/types.h. The latter following this template:
> diff --git a/grub-core/kern/arm/dl.c b/grub-core/kern/arm/dl.c
> index eab9d17ff..926073793 100644
> --- a/grub-core/kern/arm/dl.c
> +++ b/grub-core/kern/arm/dl.c
> @@ -278,3 +278,16 @@ grub_arch_dl_check_header (void *ehdr)
>
> return GRUB_ERR_NONE;
> }
> +
> +/*
> + * Tell the loader what our minimum section alignment is.
> + */
> +grub_size_t
> +grub_arch_dl_min_alignment (void)
> +{
> +#ifdef GRUB_MACHINE_EFI
> + return 4096;
> +#else
> + return 1;
> +#endif
> +}
I would define this as a constant in the include/grub/efi/memory.h.
OK, we have to have definition for non-EFI case somewhere as well.
> diff --git a/grub-core/kern/arm64/dl.c b/grub-core/kern/arm64/dl.c
> index a2b5789a9..95c6d5bf4 100644
> --- a/grub-core/kern/arm64/dl.c
> +++ b/grub-core/kern/arm64/dl.c
> @@ -196,3 +196,16 @@ grub_arch_dl_relocate_symbols (grub_dl_t mod, void *ehdr,
>
> return GRUB_ERR_NONE;
> }
> +
> +/*
> + * Tell the loader what our minimum section alignment is.
> + */
> +grub_size_t
> +grub_arch_dl_min_alignment (void)
> +{
> +#ifdef GRUB_MACHINE_EFI
> + return 4096;
> +#else
> + return 1;
> +#endif
> +}
Ditto and below as well please...
> diff --git a/grub-core/kern/dl.c b/grub-core/kern/dl.c
> index 37db9fab0..8338f7436 100644
> --- a/grub-core/kern/dl.c
> +++ b/grub-core/kern/dl.c
> @@ -224,25 +224,35 @@ grub_dl_load_segments (grub_dl_t mod, const Elf_Ehdr *e)
> {
> unsigned i;
> const Elf_Shdr *s;
> - grub_size_t tsize = 0, talign = 1;
> + grub_size_t tsize = 0, talign = 1, arch_addralign = 1;
> #if !defined (__i386__) && !defined (__x86_64__) && !defined(__riscv) && \
> !defined (__loongarch__)
> grub_size_t tramp;
> + grub_size_t tramp_align;
> grub_size_t got;
> + grub_size_t got_align;
> grub_err_t err;
> #endif
> char *ptr;
>
> + arch_addralign = grub_arch_dl_min_alignment ();
> +
> for (i = 0, s = (const Elf_Shdr *)((const char *) e + e->e_shoff);
> i < e->e_shnum;
> i++, s = (const Elf_Shdr *)((const char *) s + e->e_shentsize))
> {
> + grub_size_t sh_addralign;
> + grub_size_t sh_size;
> +
> if (s->sh_size == 0 || !(s->sh_flags & SHF_ALLOC))
> continue;
>
> - tsize = ALIGN_UP (tsize, s->sh_addralign) + s->sh_size;
> - if (talign < s->sh_addralign)
> - talign = s->sh_addralign;
> + sh_addralign = ALIGN_UP(s->sh_addralign, arch_addralign);
> + sh_size = ALIGN_UP(s->sh_size, sh_addralign);
> +
> + tsize = ALIGN_UP (tsize, sh_addralign) + sh_size;
> + if (talign < sh_addralign)
> + talign = sh_addralign;
> }
>
> #if !defined (__i386__) && !defined (__x86_64__) && !defined(__riscv) && \
> @@ -250,12 +260,18 @@ grub_dl_load_segments (grub_dl_t mod, const Elf_Ehdr *e)
> err = grub_arch_dl_get_tramp_got_size (e, &tramp, &got);
> if (err)
> return err;
> - tsize += ALIGN_UP (tramp, GRUB_ARCH_DL_TRAMP_ALIGN);
> - if (talign < GRUB_ARCH_DL_TRAMP_ALIGN)
> - talign = GRUB_ARCH_DL_TRAMP_ALIGN;
> - tsize += ALIGN_UP (got, GRUB_ARCH_DL_GOT_ALIGN);
> - if (talign < GRUB_ARCH_DL_GOT_ALIGN)
> - talign = GRUB_ARCH_DL_GOT_ALIGN;
> + tramp_align = GRUB_ARCH_DL_TRAMP_ALIGN;
> + if (tramp_align < arch_addralign)
> + tramp_align = arch_addralign;
> + tsize += ALIGN_UP (tramp, tramp_align);
> + if (talign < tramp_align)
> + talign = tramp_align;
> + got_align = GRUB_ARCH_DL_GOT_ALIGN;
> + if (got_align < arch_addralign)
> + got_align = arch_addralign;
> + tsize += ALIGN_UP (got, got_align);
> + if (talign < got_align)
> + talign = got_align;
> #endif
>
> #ifdef GRUB_MACHINE_EMU
> @@ -272,6 +288,9 @@ grub_dl_load_segments (grub_dl_t mod, const Elf_Ehdr *e)
> i < e->e_shnum;
> i++, s = (Elf_Shdr *)((char *) s + e->e_shentsize))
> {
> + grub_size_t sh_addralign = ALIGN_UP(s->sh_addralign, arch_addralign);
> + grub_size_t sh_size = ALIGN_UP(s->sh_size, sh_addralign);
> +
> if (s->sh_flags & SHF_ALLOC)
> {
> grub_dl_segment_t seg;
> @@ -284,17 +303,19 @@ grub_dl_load_segments (grub_dl_t mod, const Elf_Ehdr *e)
> {
> void *addr;
>
> - ptr = (char *) ALIGN_UP ((grub_addr_t) ptr, s->sh_addralign);
> + ptr = (char *) ALIGN_UP ((grub_addr_t) ptr, sh_addralign);
> addr = ptr;
> - ptr += s->sh_size;
> + ptr += sh_size;
>
> switch (s->sh_type)
> {
> case SHT_PROGBITS:
> grub_memcpy (addr, (char *) e + s->sh_offset, s->sh_size);
> + grub_memset ((char *)addr + s->sh_size, 0,
> + sh_size - s->sh_size);
You do not need wrap line here. I am OK with lines (a bit) longer than 80 chars.
> break;
> case SHT_NOBITS:
> - grub_memset (addr, 0, s->sh_size);
> + grub_memset (addr, 0, sh_size);
> break;
> }
>
Daniel
_______________________________________________
Grub-devel mailing list
Grub-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/grub-devel
