[PATCH 6/6] arm64: module: rework module VA range selection

[Mark Rutland]

On Wed, May 10, 2023 at 07:12:42AM -0500, Shanker Donthineni wrote:
> Hi Mark,
> 
> On 5/9/23 06:14, Mark Rutland wrote:
> >          if (IS_ENABLED(CONFIG_RANDOMIZE_MODULE_REGION_FULL)) {
> > -               /*
> > -                * Randomize the module region over a 2 GB window covering the
> > -                * kernel. This reduces the risk of modules leaking information
> > -                * about the address of the kernel itself, but results in
> > -                * branches between modules and the core kernel that are
> > -                * resolved via PLTs. (Branches between modules will be
> > -                * resolved normally.)
> > -                */
> > -               module_range = SZ_2G - (u64)(_end - _stext);
> > -               module_alloc_base = max((u64)_end - SZ_2G, (u64)MODULES_VADDR);
> > +               pr_info("2G module region forced by RANDOMIZE_MODULE_REGION_FULL\n");
> > +       } else if (kernel_size >= SZ_128M) {
> > +               pr_info("2G module region forced by kernel size (%llu bytes)\n",
> > +                       kernel_size);
> > +       } else if (IS_ENABLED(CONFIG_RANOMIZE_BASE)) {
> > +               module_direct_base = choose_random_bounding_box(SZ_128M,
> > +                                                               kernel_start,
> > +                                                               kernel_end);
> >          } else {
> > -               /*
> > -                * Randomize the module region by setting module_alloc_base to
> > -                * a PAGE_SIZE multiple in the range [_etext - MODULES_VSIZE,
> > -                * _stext) . This guarantees that the resulting region still
> > -                * covers [_stext, _etext], and that all relative branches can
> > -                * be resolved without veneers unless this region is exhausted
> > -                * and we fall back to a larger 2GB window in module_alloc()
> > -                * when ARM64_MODULE_PLTS is enabled.
> > -                */
> > -               module_range = MODULES_VSIZE - (u64)(_etext - _stext);
> > +               module_direct_base = kernel_end - SZ_128M;
> >          }
> > 
> > -       /* use the lower 21 bits to randomize the base of the module region */
> > -       module_alloc_base += (module_range * (seed & ((1 << 21) - 1))) >> 21;
> > -       module_alloc_base &= PAGE_MASK;
> > +       if (IS_ENABLED(CONFIG_RANDOMIZE_BASE)) {
> > +               u64 min, max;
> > +
> > +               if (module_direct_base) {
> > +                       min = module_direct_base;
> > +                       max = module_direct_base + SZ_128M;
> > +               } else {
> > +                       min = kernel_start;
> > +                       max = kernel_end;
> > +               }
> > +
> > +               module_plt_base = choose_random_bounding_box(SZ_2G, min, max);
> > +       } else {
> > +               module_plt_base = kernel_end - SZ_2G;
> > +       }
> 
> Sharing the 2GB space with other VMALLOC callers caused it to run out, which was
> the original problem. Is it feasible to avoid randomizing 'module_plt_base' and
> 'module_direct_base' when KASLR is disabled via the kernel boot parameter or when
> a random seed is not available? This approach would guarantee the full availability
> of the entire 2GB space for modules/kernel.

Ah, sorry, I'd messed up the conditions here, I had meant to use the deafult
region when KASLR was disabled dynamically.

I'll spin a v2 with that corrected.

> --- a/arch/arm64/kernel/module.c
> +++ b/arch/arm64/kernel/module.c
> @@ -88,7 +88,7 @@ static int __init module_init_limits(void)
>         } else if (kernel_size >= SZ_128M) {
>                 pr_info("2G module region forced by kernel size (%llu bytes)\n",
>                         kernel_size);
> -       } else if (IS_ENABLED(CONFIG_RANOMIZE_BASE)) {
> +       } else if (IS_ENABLED(CONFIG_RANOMIZE_BASE) && kaslr_enabled()) {

An earlier patch makes kaslr_enabled() consider CONFIG_RANOMIZE_BASE, so we
only need to check kaslr_enabled().

I'll respin with this reworked.

Thanks,
Mark.