Re: [PATCH 2/2] sparc64: Randomize the load address of PIE executables

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] sparc64: 32-bit compat PIE and brk collide with TASK_UNMAPPED_BASE
--

commit 91a8982a72732f896c36479ab4e561774ee258f9
Author: Stian Halseth <stian@itx.no>

sparc64: Randomize the load address of PIE executables

This commit enables PIE load address and brk randomization on sparc64 by
renaming mmap_rnd() to arch_mmap_rnd(), adding arch_randomize_brk(), and
selecting ARCH_HAS_ELF_RANDOMIZE.

> diff --git a/arch/sparc/Kconfig b/arch/sparc/Kconfig
> index ab77d3f2536e1..82c989b9e7831 100644
> --- a/arch/sparc/Kconfig
> +++ b/arch/sparc/Kconfig
> @@ -71,6 +71,7 @@ config SPARC64
>  	def_bool 64BIT
>  	select ALTERNATE_USER_ADDRESS_SPACE
>  	select ARCH_HAS_DELAY_TIMER
> +	select ARCH_HAS_ELF_RANDOMIZE
>  	select HAVE_FUNCTION_TRACER
>  	select HAVE_FUNCTION_GRAPH_TRACER
>  	select HAVE_KRETPROBES
> diff --git a/arch/sparc/kernel/sys_sparc_64.c b/arch/sparc/kernel/sys_sparc_64.c
> index 087595fa43081..2948d5769e768 100644
> --- a/arch/sparc/kernel/sys_sparc_64.c
> +++ b/arch/sparc/kernel/sys_sparc_64.c
> @@ -26,6 +26,8 @@
>  #include <linux/ipc.h>
>  #include <linux/personality.h>
>  #include <linux/random.h>
> +#include <linux/elf-randomize.h>
> +#include <linux/sizes.h>
>  #include <linux/export.h>
>  #include <linux/context_tracking.h>
>  #include <linux/timex.h>
> @@ -280,7 +282,7 @@ unsigned long get_fb_unmapped_area(struct file *filp, unsigned long orig_addr, u
>  EXPORT_SYMBOL(get_fb_unmapped_area);
>  
>  /* Essentially the same as PowerPC.  */
> -static unsigned long mmap_rnd(void)
> +unsigned long arch_mmap_rnd(void)
>  {
>  	unsigned long rnd = 0UL;
>  
> @@ -294,9 +296,17 @@ static unsigned long mmap_rnd(void)
>  	return rnd << PAGE_SHIFT;
>  }
>  
> +unsigned long arch_randomize_brk(struct mm_struct *mm)
> +{
> +	if (test_thread_flag(TIF_32BIT))
> +		return randomize_page(mm->brk, SZ_32M);
> +
> +	return randomize_page(mm->brk, SZ_1G);
> +}

[Severity: High]
Does enabling ARCH_HAS_ELF_RANDOMIZE and arch_randomize_brk() cause 32-bit
compat PIE binaries and their brk heap to collide with TASK_UNMAPPED_BASE
when using the bottom-up mmap layout?

On sparc64, 32-bit tasks (TIF_32BIT) have COMPAT_ELF_ET_DYN_BASE and
TASK_UNMAPPED_BASE set to the same address (0x70000000UL):

arch/sparc/include/asm/elf_64.h:
    #define ELF_ET_DYN_BASE		0x0000010000000000UL
    #define COMPAT_ELF_ET_DYN_BASE	0x0000000070000000UL

arch/sparc/include/asm/page_64.h:
    #define TASK_UNMAPPED_BASE	(test_thread_flag(TIF_32BIT) ? \
    				 _AC(0x0000000070000000,UL) : \
    				 VA_EXCLUDE_END)

When a 32-bit compat task uses the bottom-up layout in
arch_pick_mmap_layout() (when RLIMIT_STACK is RLIM_INFINITY,
ADDR_COMPAT_LAYOUT is set in personality, or sysctl_legacy_va_layout is
enabled), mm->mmap_base is set to 0x70000000UL + random_factor (with
random_factor in [0, 8MB)).

With CONFIG_ARCH_HAS_ELF_RANDOMIZE selected, fs/compat_binfmt_elf.c (which
builds load_elf_binary() with ELF_ET_DYN_BASE defined as
COMPAT_ELF_ET_DYN_BASE) changes behavior in two ways for 32-bit bottom-up
tasks:

1. For 32-bit ET_DYN binaries without PT_INTERP (static PIE or direct
   invocation of ld.so), load_elf_binary() maps the binary at load_bias = 0,
   which resolves via arch_get_unmapped_area() to mm->mmap_base
   (0x70000000UL + rnd1). Then load_elf_binary() moves elf_brk:

   fs/binfmt_elf.c:load_elf_binary() {
       ...
       if (!IS_ENABLED(CONFIG_COMPAT_BRK) &&
           IS_ENABLED(CONFIG_ARCH_HAS_ELF_RANDOMIZE) &&
           elf_ex->e_type == ET_DYN && !interpreter) {
           elf_brk = ELF_ET_DYN_BASE;
           /* This counts as moving the brk, so let brk(2) know. */
           brk_moved = true;
       }
   }

   When ASLR is disabled or randomize_va_space is 1, mm->start_brk and
   mm->brk are set to 0x70000000UL, pointing directly at the start of the
   mapped binary and causing sys_brk() to hit the binary's own VMA and fail
   on the first heap expansion. When randomize_va_space is 2,
   arch_randomize_brk() places mm->brk in [0x70000000UL, 0x70000000UL + 32MB),
   overlapping the binary, vDSO, and bottom-up mmap region.

2. For 32-bit dynamic PIE binaries (ET_DYN with PT_INTERP) in bottom-up
   mode, load_elf_binary() randomizes load_bias to 0x70000000UL + rnd2
   (with rnd2 in [0, 8MB)), mapping the executable in the same
   [0x70000000UL, 0x70800000UL) window as mm->mmap_base. Next,
   load_elf_interp() and arch_setup_additional_pages() map ld.so and the
   vDSO bottom-up from mm->mmap_base, placing them at or shortly above
   elf_brk. Then arch_randomize_brk() advances mm->start_brk and mm->brk by
   up to SZ_32M past elf_brk without checking for existing VMAs, placing
   mm->brk inside or above ld.so/vDSO right in the middle of the bottom-up
   mmap region.

Should COMPAT_ELF_ET_DYN_BASE be separated from 32-bit TASK_UNMAPPED_BASE
(similar to powerpc, x86, and arm64, which place 32-bit COMPAT_ELF_ET_DYN_BASE
at 0x000400000UL or offset from TASK_UNMAPPED_BASE) so that 32-bit PIE and
brk do not collide with the bottom-up mmap base?

> +
>  void arch_pick_mmap_layout(struct mm_struct *mm, const struct rlimit *rlim_stack)
>  {
> -	unsigned long random_factor = mmap_rnd();
> +	unsigned long random_factor = arch_mmap_rnd();
>  	unsigned long gap;
>  
>  	/*

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261017100451.2266781-1-stian@itx.no?part=2