On Thu, Sep 19, 2024 at 05:31:58PM +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.
The commit message is stale. Please update it to reflect what current
patch does.
> Signed-off-by: Peter Jones <pjo...@redhat.com>
> Signed-off-by: Laszlo Ersek <ler...@redhat.com>
> Signed-off-by: Jan Setje-Eilers <jan.setjeeil...@oracle.com>
> Signed-off-by: Mate Kukri <mate.ku...@canonical.com>
> ---
> grub-core/kern/dl.c | 52 +++++++++++++++++++++++++++++++--------------
> include/grub/dl.h | 9 ++++++++
> 2 files changed, 45 insertions(+), 16 deletions(-)
>
> diff --git a/grub-core/kern/dl.c b/grub-core/kern/dl.c
> index 37db9fab0..47705f5af 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_DL_ALIGN;
> +
> 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);
Coding style, missing space before "("...
> + sh_size = ALIGN_UP(s->sh_size, sh_addralign);
Ditto and below please...
> +
> + 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;
tramp_align = grub_max (tramp_align, arch_addralign) ?
> + tsize += ALIGN_UP (tramp, tramp_align);
> + if (talign < tramp_align)
> + talign = tramp_align;
talign = grub_max (talign, tramp_align) ?
... and probably below...
> + 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,18 @@ 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);
Missing space after ")"...
Daniel
_______________________________________________
Grub-devel mailing list
Grub-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/grub-devel
