Re: [PATCH v4 04/10] modules: load module sections at page-aligned addresses
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
> Original-Author: Laszlo Ersek
> Signed-off-by: Mate Kukri
> ---
> 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 &
[PATCH v4 04/10] modules: load module sections at page-aligned addresses
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
Original-Author: Laszlo Ersek
Signed-off-by: Mate Kukri
---
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
+}
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
+}
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) &&
