Re: [PATCH] alpha: Fix personality flag propagation across an exec

[Kees Cook]

On Fri, Jan 03, 2025 at 03:01:46PM +0100, John Paul Adrian Glaubitz wrote:
> It was observed that on alpha, the misc/setarch test of
> the util-linux testsuite failed with the following error:
> 
>    misc: setarch                        ...
>           : options                     ... OK
>           : uname26                     ... OK
>           : uname26-version             ... FAILED (misc/setarch-uname26-version)
>           : show                        ... OK
>      ... FAILED (1 from 4 sub-tests)
> 
> Running the setarch binary manually confirmed that setting
> the kernel version with the help --uname-2.6 flag does not
> work and the version remains unchanged.
> 
> It turned out that on alpha, the personality flags are not
> propagated but overridden during an exec. The same issue was
> previously fixed on arm in commit 5e143436d044 ("ARM: 6878/1:
> fix personality flag propagation across an exec") and on powerpc
> in commit a91a03ee31a5 ("powerpc: Keep 3 high personality bytes
> across exec"). This patch fixes the issue on alpha.

Good catch!

> 
> With the patch applied, the misc/setarch test succeeds on
> alpha as expected:
> 
>    misc: setarch                        ...
>           : options                     ... OK
>           : uname26                     ... OK
>           : uname26-version             ... OK
>           : show                        ... OK
>      ... OK (all 4 sub-tests PASSED)
> 
> However, as a side-effect, a warning is printed on the kernel
> message buffer which might indicate another unreleated bug:
> 
> [   39.964823] pid=509, couldn't seal address 0, ret=-12.

This is from mseal vs MMAP_PAGE_ZERO in fs/binfmt_elf.c

                error = vm_mmap(NULL, 0, PAGE_SIZE, PROT_READ | PROT_EXEC,
                                MAP_FIXED | MAP_PRIVATE, 0);

                retval = do_mseal(0, PAGE_SIZE, 0);
                if (retval)
                        pr_warn_ratelimited("pid=%d, couldn't seal address 0, ret=%d.\n",
                                            task_pid_nr(current), retval);

-12 is ENOMEM, which implies, I think, that check_mm_seal() failed. I
note that "error" isn't being checked, so if the vm_mmap() failed, I
think the do_mseal() would fail with ENOMEM?

> Signed-off-by: John Paul Adrian Glaubitz <glaubitz@xxxxxxxxxxxxxxxxxxx>

Reviewed-by: Kees Cook <kees@xxxxxxxxxx>

-Kees

> ---
>  arch/alpha/include/asm/elf.h | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/arch/alpha/include/asm/elf.h b/arch/alpha/include/asm/elf.h
> index 4d7c46f50382..81f8473bb7c0 100644
> --- a/arch/alpha/include/asm/elf.h
> +++ b/arch/alpha/include/asm/elf.h
> @@ -138,8 +138,8 @@ extern int dump_elf_task(elf_greg_t *dest, struct task_struct *task);
>  })
>  
>  #define SET_PERSONALITY(EX)					\
> -	set_personality(((EX).e_flags & EF_ALPHA_32BIT)		\
> -	   ? PER_LINUX_32BIT : PER_LINUX)
> +	set_personality((((EX).e_flags & EF_ALPHA_32BIT)	\
> +	   ? PER_LINUX_32BIT : PER_LINUX) | (current->personality & (~PER_MASK)))
>  
>  extern int alpha_l1i_cacheshape;
>  extern int alpha_l1d_cacheshape;
> -- 
> 2.39.5
> 

-- 
Kees Cook