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

[Marek Vasut]

On 4/26/19 11:36 AM, Simon Goldschmidt wrote:
> 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.

Yes ? Should be easy, just clamp() size to (size, (BIT(32) - 1) -
mem_malloc_start) or similar for 64bit systems.

> You could as well enter a range without RAM, that would fail as well.

That info is available in gd , but I wonder whether this is the right
place to check for it.

> 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


-- 
Best regards,
Marek Vasut