On Sat, Aug 03, 2024 at 03:46:41PM +0800, David Gow wrote:
> The generic mmap_base code tries to leave a gap between the top
> of the stack and the mmap base address, but enforces a minimum
> gap size (MIN_GAP) of 128MB, which is too large on some setups. In
> particular, on arm tasks without ADDR_LIMIT_32BIT, the STACK_TOP
> value is less than 128MB, so it's impossible to fit such a gap in.
>
> Only enforce this minimum if MIN_GAP < MAX_GAP, as we'd prefer to honour
> MAX_GAP, which is defined proportionally, so scales better and always
> leaves us with both _some_ stack space and some room for mmap.
>
> This fixes the usercopy KUnit test suite on 32-bit arm, as it doesn't
> set any personality flags so gets the default (in this case 26-bit)
> task size. This test can be run with:
> ./tools/testing/kunit/kunit.py run --arch arm usercopy --make_options LLVM=1
>
> Fixes: dba79c3df4a2 ("arm: use generic mmap top-down layout and brk randomization")
> Signed-off-by: David Gow <davidgow@xxxxxxxxxx>
> ---
>
> This is one possible fix for an issue with the usercopy_kunit suite
> (and, indeed, the KUnit user_alloc features) on 32-bit arm. The other
> options are to:
> - hack the KUnit allocation to force ADDR_LIMIT_32BIT or
> ADDR_COMPAT_LAYOUT; or
> - similarly, use an unlimited stack, which forces the legacy layout
> behind the scenes; or
> - adjust MIN_GAP based on either STACK_TOP or architecture.
>
> Of them, I made the arbitrary call that this was least hacky, but am
> happy to go with something else if someone who actually knows what's
> going on suggests it.
>
> (Also, does this issue actually mean some strange legacy binaries have
> been broken with an rlimit-ed stack for ages? Or am I missing something?)
>
> Cheers,
> -- David
I see akpm already snagged this, but yeah, this looks like a totally
sane fix. Thanks for digging in and finding the problem!
Reviewed-by: Kees Cook <kees@xxxxxxxxxx>
-Kees
>
> ---
> mm/util.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/mm/util.c b/mm/util.c
> index bd283e2132e0..baca6cafc9f1 100644
> --- a/mm/util.c
> +++ b/mm/util.c
> @@ -463,7 +463,7 @@ static unsigned long mmap_base(unsigned long rnd, struct rlimit *rlim_stack)
> if (gap + pad > gap)
> gap += pad;
>
> - if (gap < MIN_GAP)
> + if (gap < MIN_GAP && MIN_GAP < MAX_GAP)
> gap = MIN_GAP;
> else if (gap > MAX_GAP)
> gap = MAX_GAP;
> --
> 2.46.0.rc2.264.g509ed76dc8-goog
>
--
Kees Cook
