On 4/26/19 11:36 AM, Simon Goldschmidt wrote: > On Fri, Apr 26, 2019 at 11:32 AM Marek Vasut <marek.va...@gmail.com> wrote: >> >> On 4/26/19 8:19 AM, Simon Goldschmidt wrote: >>> Marek Vasut <marek.va...@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.goldschm...@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 _______________________________________________ U-Boot mailing list U-Boot@lists.denx.de https://lists.denx.de/listinfo/u-boot