Re: [PATCH] lib/relocator: always enforce the requested alignment in malloc_in_range()

[Daniel Kiper]

On Thu, Apr 27, 2023 at 05:06:54PM +0200, Roger Pau Monne via Grub-devel wrote:
> On failure to allocate from grub_relocator_firmware_alloc_region() in
> malloc_in_range() the function would stop enforcing the alignment, and
> the following was returned:
>
> lib/relocator.c:431: trying to allocate in 0x200000-0xffbf9fff aligned 
> 0x200000 size 0x406000
> lib/relocator.c:1197: allocated: 0x74de2000+0x406000
> lib/relocator.c:1407: allocated 0x74de2000/0x74de2000
>
> Fix this by making sure that target always contains a suitably aligned
> address.  After the change the return from the function is:
>
> lib/relocator.c:431: trying to allocate in 0x200000-0xffb87fff aligned 
> 0x200000 size 0x478000
> lib/relocator.c:1204: allocated: 0x74c00000+0x478000
> lib/relocator.c:1414: allocated 0x74c00000/0x74c00000
>
> Fixes: 3a5768645c05 ('First version of allocation from firmware')
> Signed-off-by: Roger Pau Monné <roger.pau@citrix.com>

LGTM but I would like to hear Vladimir's opinion too.

Daniel

> ---
>  grub-core/lib/relocator.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/grub-core/lib/relocator.c b/grub-core/lib/relocator.c
> index bfcc70dac3cc..d8840682c19c 100644
> --- a/grub-core/lib/relocator.c
> +++ b/grub-core/lib/relocator.c
> @@ -744,7 +744,7 @@ malloc_in_range (struct grub_relocator *rel,
>           {
>             target = starta;
>             if (target < start)
> -             target = start;
> +             target = ALIGN_UP (start, align);
>             if (target + size <= end && target + size <= events[j].pos)
>               /* Found an usable address.  */
>               goto found;
> @@ -761,7 +761,7 @@ malloc_in_range (struct grub_relocator *rel,
>           {
>             target = starta - size;
>             if (target > end - size)
> -             target = end - size;
> +             target = ALIGN_UP (end - size, align);
>             if (target >= start && target >= events[j].pos)
>               goto found;
>           }