[U-Boot] [PATCH v5 2/2] dlmalloc: fix malloc range at end of ram

[Simon Goldschmidt]

On Fri, Apr 26, 2019 at 11:32 AM Marek Vasut <marek.vasut at gmail.com> wrote:
>
> On 4/26/19 8:19 AM, Simon Goldschmidt wrote:
> > Marek Vasut <marek.vasut at gmail.com> schrieb am Fr., 26. Apr. 2019, 00:22:
> >
> >> On 4/25/19 9:22 PM, Simon Goldschmidt wrote:
> >>> If the malloc range passed to mem_malloc_init() is at the end of address
> >>> range and 'start + size' overflows to 0, following allocations fail as
> >>> mem_malloc_end is zero (which looks like uninitialized).
> >>>
> >>> Fix this by subtracting 1 of 'start + size' overflows to zero.
> >>>
> >>> Signed-off-by: Simon Goldschmidt <simon.k.r.goldschmidt at gmail.com>
> >>> ---
> >>>
> >>> Changes in v5:
> >>> - this patch was 1/2 in v4 but is now 2/2 as the 2nd patch of v4 has
> >>>   already been accepted
> >>> - rearrange the code to make it only 8 bytes plus in code size for arm
> >>>   (which fixes smartweb SPL overflowing)
> >>>
> >>>  common/dlmalloc.c | 6 +++++-
> >>>  1 file changed, 5 insertions(+), 1 deletion(-)
> >>>
> >>> diff --git a/common/dlmalloc.c b/common/dlmalloc.c
> >>> index 6f12a18d54..38859ecbd4 100644
> >>> --- a/common/dlmalloc.c
> >>> +++ b/common/dlmalloc.c
> >>> @@ -601,8 +601,12 @@ void *sbrk(ptrdiff_t increment)
> >>>  void mem_malloc_init(ulong start, ulong size)
> >>>  {
> >>>       mem_malloc_start = start;
> >>> -     mem_malloc_end = start + size;
> >>>       mem_malloc_brk = start;
> >>> +     mem_malloc_end = start + size;
> >>> +     if (size > mem_malloc_end) {
> >>> +             /* overflow: malloc area is at end of address range */
> >>> +             mem_malloc_end--;
> >>
> >> Does this mean a memory wrap-around happened ?
> >> I don't think decrementing malloc area size by 1 is a proper solution.
> >> You can have it overflow by 2 and decrementing by 1 won't help.
> >>
> >
> > No, not a real overflow. Instead, as I tried to described in the commit
> > message, mem_malloc_end gets 0 if the range is at the end of addr range,
> > e.g. malloc start is 0xffff0000 and malloc size is 0x10000. Subtracting 1
> > will be enough here. It reduces the available mall of aize, but I don't
> > think that should be a problem.
>
> That's a wrap-around . What happens with your example if malloc_size is
> 0x10001 ? Hint: It fails ...

Yes it fails. But in contrast, that's an invalid configuration, while
my patch makes
a valid configuration work. I don't know if we want to fix all invalid
configurations.
You could as well enter a range without RAM, that would fail as well.

A different approach to fix my valid end-of-ram configuration would be to set
the end to "start + size - 1" and to change all the checks using it. But that
would probably lead to more code size problems in various SPL...

Regards,
Simon

>
> > I got this when experimenting with full heap in socfpga. Due to other
> > patches not being accepted, this is not an issue currebtly, but can easily
> > become one on the future.
> >
> > Regrds,
> > Simon
> >
> >
> >>> +     }
> >>>
> >>>       debug("using memory %#lx-%#lx for malloc()\n", mem_malloc_start,
> >>>             mem_malloc_end);
> >>>
> >>
> >>
> >> --
> >> Best regards,
> >> Marek Vasut
> >>
> >
>
>
> --
> Best regards,
> Marek Vasut