All of lore.kernel.org
 help / color / mirror / Atom feed
* [U-Boot] [PATCH v5 1/2] spl: fix linker size check off-by-one errors
@ 2019-04-25 19:22 Simon Goldschmidt
  2019-04-25 19:22 ` [U-Boot] [PATCH v5 2/2] dlmalloc: fix malloc range at end of ram Simon Goldschmidt
                   ` (2 more replies)
  0 siblings, 3 replies; 15+ messages in thread
From: Simon Goldschmidt @ 2019-04-25 19:22 UTC (permalink / raw)
  To: u-boot

This fixes SPL linker script size checks for 3 lds files where the size
checks were implemented as "x < YYY_MAX_SIZE".

Fix the size checks to be "x <= YYY_MAX_SIZE" instead.

Signed-off-by: Simon Goldschmidt <simon.k.r.goldschmidt@gmail.com>
---

Changes in v5:
- added patch to fix linker size checks off-by-one error

 arch/arm/cpu/u-boot-spl.lds                 | 6 +++---
 arch/arm/mach-at91/arm926ejs/u-boot-spl.lds | 4 ++--
 arch/microblaze/cpu/u-boot-spl.lds          | 2 +-
 3 files changed, 6 insertions(+), 6 deletions(-)

diff --git a/arch/arm/cpu/u-boot-spl.lds b/arch/arm/cpu/u-boot-spl.lds
index a2aa93a735..97899a567f 100644
--- a/arch/arm/cpu/u-boot-spl.lds
+++ b/arch/arm/cpu/u-boot-spl.lds
@@ -79,16 +79,16 @@ SECTIONS
 }
 
 #if defined(IMAGE_MAX_SIZE)
-ASSERT(__image_copy_end - __image_copy_start < (IMAGE_MAX_SIZE), \
+ASSERT(__image_copy_end - __image_copy_start <= (IMAGE_MAX_SIZE), \
 	"SPL image too big");
 #endif
 
 #if defined(CONFIG_SPL_BSS_MAX_SIZE)
-ASSERT(__bss_end - __bss_start < (CONFIG_SPL_BSS_MAX_SIZE), \
+ASSERT(__bss_end - __bss_start <= (CONFIG_SPL_BSS_MAX_SIZE), \
 	"SPL image BSS too big");
 #endif
 
 #if defined(CONFIG_SPL_MAX_FOOTPRINT)
-ASSERT(__bss_end - _start < (CONFIG_SPL_MAX_FOOTPRINT), \
+ASSERT(__bss_end - _start <= (CONFIG_SPL_MAX_FOOTPRINT), \
 	"SPL image plus BSS too big");
 #endif
diff --git a/arch/arm/mach-at91/arm926ejs/u-boot-spl.lds b/arch/arm/mach-at91/arm926ejs/u-boot-spl.lds
index 3955bea23a..74f6355229 100644
--- a/arch/arm/mach-at91/arm926ejs/u-boot-spl.lds
+++ b/arch/arm/mach-at91/arm926ejs/u-boot-spl.lds
@@ -52,11 +52,11 @@ SECTIONS
 }
 
 #if defined(IMAGE_MAX_SIZE)
-ASSERT(__image_copy_end - __start < (IMAGE_MAX_SIZE), \
+ASSERT(__image_copy_end - __start <= (IMAGE_MAX_SIZE), \
 	"SPL image too big");
 #endif
 
 #if defined(CONFIG_SPL_BSS_MAX_SIZE)
-ASSERT(__bss_end - __bss_start < (CONFIG_SPL_BSS_MAX_SIZE), \
+ASSERT(__bss_end - __bss_start <= (CONFIG_SPL_BSS_MAX_SIZE), \
 	"SPL image BSS too big");
 #endif
diff --git a/arch/microblaze/cpu/u-boot-spl.lds b/arch/microblaze/cpu/u-boot-spl.lds
index 99c62b51a1..3387eb7189 100644
--- a/arch/microblaze/cpu/u-boot-spl.lds
+++ b/arch/microblaze/cpu/u-boot-spl.lds
@@ -57,6 +57,6 @@ SECTIONS
 }
 
 #if defined(CONFIG_SPL_MAX_FOOTPRINT)
-ASSERT(__end - _start < (CONFIG_SPL_MAX_FOOTPRINT), \
+ASSERT(__end - _start <= (CONFIG_SPL_MAX_FOOTPRINT), \
         "SPL image plus BSS too big");
 #endif
-- 
2.17.1

^ permalink raw reply related	[flat|nested] 15+ messages in thread

* [U-Boot] [PATCH v5 2/2] dlmalloc: fix malloc range at end of ram
  2019-04-25 19:22 [U-Boot] [PATCH v5 1/2] spl: fix linker size check off-by-one errors Simon Goldschmidt
@ 2019-04-25 19:22 ` Simon Goldschmidt
  2019-04-25 22:22   ` Marek Vasut
  2019-04-27 20:17 ` [U-Boot] [PATCH v5 1/2] spl: fix linker size check off-by-one errors Simon Goldschmidt
  2019-05-06 11:14 ` Tom Rini
  2 siblings, 1 reply; 15+ messages in thread
From: Simon Goldschmidt @ 2019-04-25 19:22 UTC (permalink / raw)
  To: u-boot

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@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--;
+	}
 
 	debug("using memory %#lx-%#lx for malloc()\n", mem_malloc_start,
 	      mem_malloc_end);
-- 
2.17.1

^ permalink raw reply related	[flat|nested] 15+ messages in thread

* [U-Boot] [PATCH v5 2/2] dlmalloc: fix malloc range at end of ram
  2019-04-25 19:22 ` [U-Boot] [PATCH v5 2/2] dlmalloc: fix malloc range at end of ram Simon Goldschmidt
@ 2019-04-25 22:22   ` Marek Vasut
  2019-04-26  6:19     ` Simon Goldschmidt
  0 siblings, 1 reply; 15+ messages in thread
From: Marek Vasut @ 2019-04-25 22:22 UTC (permalink / raw)
  To: u-boot

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@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.

> +	}
>  
>  	debug("using memory %#lx-%#lx for malloc()\n", mem_malloc_start,
>  	      mem_malloc_end);
> 


-- 
Best regards,
Marek Vasut

^ permalink raw reply	[flat|nested] 15+ messages in thread

* [U-Boot] [PATCH v5 2/2] dlmalloc: fix malloc range at end of ram
  2019-04-25 22:22   ` Marek Vasut
@ 2019-04-26  6:19     ` Simon Goldschmidt
  2019-04-26  9:32       ` Marek Vasut
  0 siblings, 1 reply; 15+ messages in thread
From: Simon Goldschmidt @ 2019-04-26  6:19 UTC (permalink / raw)
  To: u-boot

Marek Vasut <marek.vasut@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@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.

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
>

^ permalink raw reply	[flat|nested] 15+ messages in thread

* [U-Boot] [PATCH v5 2/2] dlmalloc: fix malloc range at end of ram
  2019-04-26  6:19     ` Simon Goldschmidt
@ 2019-04-26  9:32       ` Marek Vasut
  2019-04-26  9:36         ` Simon Goldschmidt
  0 siblings, 1 reply; 15+ messages in thread
From: Marek Vasut @ 2019-04-26  9:32 UTC (permalink / raw)
  To: u-boot

On 4/26/19 8:19 AM, Simon Goldschmidt wrote:
> Marek Vasut <marek.vasut@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@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 ...

> 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

^ permalink raw reply	[flat|nested] 15+ messages in thread

* [U-Boot] [PATCH v5 2/2] dlmalloc: fix malloc range at end of ram
  2019-04-26  9:32       ` Marek Vasut
@ 2019-04-26  9:36         ` Simon Goldschmidt
  2019-04-26  9:56           ` Marek Vasut
  0 siblings, 1 reply; 15+ messages in thread
From: Simon Goldschmidt @ 2019-04-26  9:36 UTC (permalink / raw)
  To: u-boot

On Fri, Apr 26, 2019 at 11:32 AM Marek Vasut <marek.vasut@gmail.com> wrote:
>
> On 4/26/19 8:19 AM, Simon Goldschmidt wrote:
> > Marek Vasut <marek.vasut@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@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

^ permalink raw reply	[flat|nested] 15+ messages in thread

* [U-Boot] [PATCH v5 2/2] dlmalloc: fix malloc range at end of ram
  2019-04-26  9:36         ` Simon Goldschmidt
@ 2019-04-26  9:56           ` Marek Vasut
  2019-04-26 10:19             ` Simon Goldschmidt
  0 siblings, 1 reply; 15+ messages in thread
From: Marek Vasut @ 2019-04-26  9:56 UTC (permalink / raw)
  To: u-boot

On 4/26/19 11:36 AM, Simon Goldschmidt wrote:
> On Fri, Apr 26, 2019 at 11:32 AM Marek Vasut <marek.vasut@gmail.com> wrote:
>>
>> On 4/26/19 8:19 AM, Simon Goldschmidt wrote:
>>> Marek Vasut <marek.vasut@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@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

^ permalink raw reply	[flat|nested] 15+ messages in thread

* [U-Boot] [PATCH v5 2/2] dlmalloc: fix malloc range at end of ram
  2019-04-26  9:56           ` Marek Vasut
@ 2019-04-26 10:19             ` Simon Goldschmidt
  2019-04-26 11:00               ` Marek Vasut
  0 siblings, 1 reply; 15+ messages in thread
From: Simon Goldschmidt @ 2019-04-26 10:19 UTC (permalink / raw)
  To: u-boot

On Fri, Apr 26, 2019 at 11:56 AM Marek Vasut <marek.vasut@gmail.com> wrote:
>
> On 4/26/19 11:36 AM, Simon Goldschmidt wrote:
> > On Fri, Apr 26, 2019 at 11:32 AM Marek Vasut <marek.vasut@gmail.com> wrote:
> >>
> >> On 4/26/19 8:19 AM, Simon Goldschmidt wrote:
> >>> Marek Vasut <marek.vasut@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@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.

I'm not convinced we should. This range is normally generated using
something like:
SIZE=2048
START=RAM_END - SIZE

I don't want to be overprotective here. I don't think there's much point
in fixing the out-of-ram-range check if it produces an overflow but not
fix it if it's in the middle of an address space.

Again, this patch simply fixes the case for something like this:
RAM_SIZE=0x10000
RAM_START=0xFFFF0000
so RAM_END=0

We can use clamp as you suggested, but what would it be good for
if it only fixes an out-of-range heap if an overflow occurs?

>
> > 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.

Indeed, that would seem misplaced here.

Regards,
Simon

>
> > 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

^ permalink raw reply	[flat|nested] 15+ messages in thread

* [U-Boot] [PATCH v5 2/2] dlmalloc: fix malloc range at end of ram
  2019-04-26 10:19             ` Simon Goldschmidt
@ 2019-04-26 11:00               ` Marek Vasut
  2019-05-04 18:16                 ` Simon Goldschmidt
  0 siblings, 1 reply; 15+ messages in thread
From: Marek Vasut @ 2019-04-26 11:00 UTC (permalink / raw)
  To: u-boot

On 4/26/19 12:19 PM, Simon Goldschmidt wrote:
> On Fri, Apr 26, 2019 at 11:56 AM Marek Vasut <marek.vasut@gmail.com> wrote:
>>
>> On 4/26/19 11:36 AM, Simon Goldschmidt wrote:
>>> On Fri, Apr 26, 2019 at 11:32 AM Marek Vasut <marek.vasut@gmail.com> wrote:
>>>>
>>>> On 4/26/19 8:19 AM, Simon Goldschmidt wrote:
>>>>> Marek Vasut <marek.vasut@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@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.
> 
> I'm not convinced we should. This range is normally generated using
> something like:
> SIZE=2048
> START=RAM_END - SIZE

Normally ... on SoCFPGA . Other ARM32 platforms can have OCRAM mapped
somewhere in the middle of the address space. Take R-Car Gen2, which has
it at 0xe6300000 + 64k or something like that.

And, to make things worse, you cannot detect these overflows at compile
time, since the DRAM layout, which is passed to malloc init can come
from DT.

Thus, you might want to sanitize the input, properly.

> I don't want to be overprotective here. I don't think there's much point
> in fixing the out-of-ram-range check if it produces an overflow but not
> fix it if it's in the middle of an address space.
> 
> Again, this patch simply fixes the case for something like this:
> RAM_SIZE=0x10000
> RAM_START=0xFFFF0000
> so RAM_END=0
> 
> We can use clamp as you suggested, but what would it be good for
> if it only fixes an out-of-range heap if an overflow occurs?

It's better than nothing. Further refinements welcome.

>>> 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.
> 
> Indeed, that would seem misplaced here.
> 
> Regards,
> Simon
> 
>>
>>> 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


-- 
Best regards,
Marek Vasut

^ permalink raw reply	[flat|nested] 15+ messages in thread

* [U-Boot] [PATCH v5 1/2] spl: fix linker size check off-by-one errors
  2019-04-25 19:22 [U-Boot] [PATCH v5 1/2] spl: fix linker size check off-by-one errors Simon Goldschmidt
  2019-04-25 19:22 ` [U-Boot] [PATCH v5 2/2] dlmalloc: fix malloc range at end of ram Simon Goldschmidt
@ 2019-04-27 20:17 ` Simon Goldschmidt
  2019-05-06 11:14 ` Tom Rini
  2 siblings, 0 replies; 15+ messages in thread
From: Simon Goldschmidt @ 2019-04-27 20:17 UTC (permalink / raw)
  To: u-boot



On 25.04.19 21:22, Simon Goldschmidt wrote:
> This fixes SPL linker script size checks for 3 lds files where the size
> checks were implemented as "x < YYY_MAX_SIZE".
> 
> Fix the size checks to be "x <= YYY_MAX_SIZE" instead.

After updating Ubuntu to 19.04 today, the new 8.3 gcc makes 
smartweb_defconfig fail compiling SPL without this because 
__image_copy_end==0x1000

I guess we're lucky travis uses gcc 7.3... (or, are we?)

Regards,
Simon

> 
> Signed-off-by: Simon Goldschmidt <simon.k.r.goldschmidt@gmail.com>
> ---
> 
> Changes in v5:
> - added patch to fix linker size checks off-by-one error
> 
>   arch/arm/cpu/u-boot-spl.lds                 | 6 +++---
>   arch/arm/mach-at91/arm926ejs/u-boot-spl.lds | 4 ++--
>   arch/microblaze/cpu/u-boot-spl.lds          | 2 +-
>   3 files changed, 6 insertions(+), 6 deletions(-)
> 
> diff --git a/arch/arm/cpu/u-boot-spl.lds b/arch/arm/cpu/u-boot-spl.lds
> index a2aa93a735..97899a567f 100644
> --- a/arch/arm/cpu/u-boot-spl.lds
> +++ b/arch/arm/cpu/u-boot-spl.lds
> @@ -79,16 +79,16 @@ SECTIONS
>   }
>   
>   #if defined(IMAGE_MAX_SIZE)
> -ASSERT(__image_copy_end - __image_copy_start < (IMAGE_MAX_SIZE), \
> +ASSERT(__image_copy_end - __image_copy_start <= (IMAGE_MAX_SIZE), \
>   	"SPL image too big");
>   #endif
>   
>   #if defined(CONFIG_SPL_BSS_MAX_SIZE)
> -ASSERT(__bss_end - __bss_start < (CONFIG_SPL_BSS_MAX_SIZE), \
> +ASSERT(__bss_end - __bss_start <= (CONFIG_SPL_BSS_MAX_SIZE), \
>   	"SPL image BSS too big");
>   #endif
>   
>   #if defined(CONFIG_SPL_MAX_FOOTPRINT)
> -ASSERT(__bss_end - _start < (CONFIG_SPL_MAX_FOOTPRINT), \
> +ASSERT(__bss_end - _start <= (CONFIG_SPL_MAX_FOOTPRINT), \
>   	"SPL image plus BSS too big");
>   #endif
> diff --git a/arch/arm/mach-at91/arm926ejs/u-boot-spl.lds b/arch/arm/mach-at91/arm926ejs/u-boot-spl.lds
> index 3955bea23a..74f6355229 100644
> --- a/arch/arm/mach-at91/arm926ejs/u-boot-spl.lds
> +++ b/arch/arm/mach-at91/arm926ejs/u-boot-spl.lds
> @@ -52,11 +52,11 @@ SECTIONS
>   }
>   
>   #if defined(IMAGE_MAX_SIZE)
> -ASSERT(__image_copy_end - __start < (IMAGE_MAX_SIZE), \
> +ASSERT(__image_copy_end - __start <= (IMAGE_MAX_SIZE), \
>   	"SPL image too big");
>   #endif
>   
>   #if defined(CONFIG_SPL_BSS_MAX_SIZE)
> -ASSERT(__bss_end - __bss_start < (CONFIG_SPL_BSS_MAX_SIZE), \
> +ASSERT(__bss_end - __bss_start <= (CONFIG_SPL_BSS_MAX_SIZE), \
>   	"SPL image BSS too big");
>   #endif
> diff --git a/arch/microblaze/cpu/u-boot-spl.lds b/arch/microblaze/cpu/u-boot-spl.lds
> index 99c62b51a1..3387eb7189 100644
> --- a/arch/microblaze/cpu/u-boot-spl.lds
> +++ b/arch/microblaze/cpu/u-boot-spl.lds
> @@ -57,6 +57,6 @@ SECTIONS
>   }
>   
>   #if defined(CONFIG_SPL_MAX_FOOTPRINT)
> -ASSERT(__end - _start < (CONFIG_SPL_MAX_FOOTPRINT), \
> +ASSERT(__end - _start <= (CONFIG_SPL_MAX_FOOTPRINT), \
>           "SPL image plus BSS too big");
>   #endif
> 

^ permalink raw reply	[flat|nested] 15+ messages in thread

* [U-Boot] [PATCH v5 2/2] dlmalloc: fix malloc range at end of ram
  2019-04-26 11:00               ` Marek Vasut
@ 2019-05-04 18:16                 ` Simon Goldschmidt
  2019-05-05 11:38                   ` Tom Rini
  0 siblings, 1 reply; 15+ messages in thread
From: Simon Goldschmidt @ 2019-05-04 18:16 UTC (permalink / raw)
  To: u-boot

Tom,

Am 26.04.2019 um 13:00 schrieb Marek Vasut:
> On 4/26/19 12:19 PM, Simon Goldschmidt wrote:
>> On Fri, Apr 26, 2019 at 11:56 AM Marek Vasut <marek.vasut@gmail.com> wrote:
>>>
>>> On 4/26/19 11:36 AM, Simon Goldschmidt wrote:
>>>> On Fri, Apr 26, 2019 at 11:32 AM Marek Vasut <marek.vasut@gmail.com> wrote:
>>>>>
>>>>> On 4/26/19 8:19 AM, Simon Goldschmidt wrote:
>>>>>> Marek Vasut <marek.vasut@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@gmail.com>

Since there's no way this fits without breaking smartweb, I'd rather 
drop this for now in order to get 1/2 accepted.

Regards,
Simon

>>>>>>>> ---
>>>>>>>>
>>>>>>>> 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.
>>
>> I'm not convinced we should. This range is normally generated using
>> something like:
>> SIZE=2048
>> START=RAM_END - SIZE
> 
> Normally ... on SoCFPGA . Other ARM32 platforms can have OCRAM mapped
> somewhere in the middle of the address space. Take R-Car Gen2, which has
> it at 0xe6300000 + 64k or something like that.
> 
> And, to make things worse, you cannot detect these overflows at compile
> time, since the DRAM layout, which is passed to malloc init can come
> from DT.
> 
> Thus, you might want to sanitize the input, properly.
> 
>> I don't want to be overprotective here. I don't think there's much point
>> in fixing the out-of-ram-range check if it produces an overflow but not
>> fix it if it's in the middle of an address space.
>>
>> Again, this patch simply fixes the case for something like this:
>> RAM_SIZE=0x10000
>> RAM_START=0xFFFF0000
>> so RAM_END=0
>>
>> We can use clamp as you suggested, but what would it be good for
>> if it only fixes an out-of-range heap if an overflow occurs?
> 
> It's better than nothing. Further refinements welcome.
> 
>>>> 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.
>>
>> Indeed, that would seem misplaced here.
>>
>> Regards,
>> Simon
>>
>>>
>>>> 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
> 
> 

^ permalink raw reply	[flat|nested] 15+ messages in thread

* [U-Boot] [PATCH v5 2/2] dlmalloc: fix malloc range at end of ram
  2019-05-04 18:16                 ` Simon Goldschmidt
@ 2019-05-05 11:38                   ` Tom Rini
  2019-05-05 17:55                     ` Simon Goldschmidt
  0 siblings, 1 reply; 15+ messages in thread
From: Tom Rini @ 2019-05-05 11:38 UTC (permalink / raw)
  To: u-boot

On Sat, May 04, 2019 at 08:16:38PM +0200, Simon Goldschmidt wrote:
> Tom,
> 
> Am 26.04.2019 um 13:00 schrieb Marek Vasut:
> >On 4/26/19 12:19 PM, Simon Goldschmidt wrote:
> >>On Fri, Apr 26, 2019 at 11:56 AM Marek Vasut <marek.vasut@gmail.com> wrote:
> >>>
> >>>On 4/26/19 11:36 AM, Simon Goldschmidt wrote:
> >>>>On Fri, Apr 26, 2019 at 11:32 AM Marek Vasut <marek.vasut@gmail.com> wrote:
> >>>>>
> >>>>>On 4/26/19 8:19 AM, Simon Goldschmidt wrote:
> >>>>>>Marek Vasut <marek.vasut@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@gmail.com>
> 
> Since there's no way this fits without breaking smartweb, I'd rather drop
> this for now in order to get 1/2 accepted.

I thought that with 1/2 this fit again, with gcc-7.3 at least?  Thanks!

-- 
Tom
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: not available
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20190505/6d10c4d9/attachment.sig>

^ permalink raw reply	[flat|nested] 15+ messages in thread

* [U-Boot] [PATCH v5 2/2] dlmalloc: fix malloc range at end of ram
  2019-05-05 11:38                   ` Tom Rini
@ 2019-05-05 17:55                     ` Simon Goldschmidt
  2019-05-05 18:12                       ` Tom Rini
  0 siblings, 1 reply; 15+ messages in thread
From: Simon Goldschmidt @ 2019-05-05 17:55 UTC (permalink / raw)
  To: u-boot

Am 05.05.2019 um 13:38 schrieb Tom Rini:
> On Sat, May 04, 2019 at 08:16:38PM +0200, Simon Goldschmidt wrote:
>> Tom,
>>
>> Am 26.04.2019 um 13:00 schrieb Marek Vasut:
>>> On 4/26/19 12:19 PM, Simon Goldschmidt wrote:
>>>> On Fri, Apr 26, 2019 at 11:56 AM Marek Vasut <marek.vasut@gmail.com> wrote:
>>>>>
>>>>> On 4/26/19 11:36 AM, Simon Goldschmidt wrote:
>>>>>> On Fri, Apr 26, 2019 at 11:32 AM Marek Vasut <marek.vasut@gmail.com> wrote:
>>>>>>>
>>>>>>> On 4/26/19 8:19 AM, Simon Goldschmidt wrote:
>>>>>>>> Marek Vasut <marek.vasut@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@gmail.com>
>>
>> Since there's no way this fits without breaking smartweb, I'd rather drop
>> this for now in order to get 1/2 accepted.
> 
> I thought that with 1/2 this fit again, with gcc-7.3 at least?  Thanks!

I'm not sure, as I don't have it here to test. But as this patch doesn't 
actually fix a board but fixes an issue in the code that *might* appear 
in the future, I'm not convinced it would be the right thing to merge it 
like it is.

And I'm also a little short on time to investigate this further, as it's 
not a real bug, currently.

Regards,
Simon

^ permalink raw reply	[flat|nested] 15+ messages in thread

* [U-Boot] [PATCH v5 2/2] dlmalloc: fix malloc range at end of ram
  2019-05-05 17:55                     ` Simon Goldschmidt
@ 2019-05-05 18:12                       ` Tom Rini
  0 siblings, 0 replies; 15+ messages in thread
From: Tom Rini @ 2019-05-05 18:12 UTC (permalink / raw)
  To: u-boot

On Sun, May 05, 2019 at 07:55:10PM +0200, Simon Goldschmidt wrote:
> Am 05.05.2019 um 13:38 schrieb Tom Rini:
> >On Sat, May 04, 2019 at 08:16:38PM +0200, Simon Goldschmidt wrote:
> >>Tom,
> >>
> >>Am 26.04.2019 um 13:00 schrieb Marek Vasut:
> >>>On 4/26/19 12:19 PM, Simon Goldschmidt wrote:
> >>>>On Fri, Apr 26, 2019 at 11:56 AM Marek Vasut <marek.vasut@gmail.com> wrote:
> >>>>>
> >>>>>On 4/26/19 11:36 AM, Simon Goldschmidt wrote:
> >>>>>>On Fri, Apr 26, 2019 at 11:32 AM Marek Vasut <marek.vasut@gmail.com> wrote:
> >>>>>>>
> >>>>>>>On 4/26/19 8:19 AM, Simon Goldschmidt wrote:
> >>>>>>>>Marek Vasut <marek.vasut@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@gmail.com>
> >>
> >>Since there's no way this fits without breaking smartweb, I'd rather drop
> >>this for now in order to get 1/2 accepted.
> >
> >I thought that with 1/2 this fit again, with gcc-7.3 at least?  Thanks!
> 
> I'm not sure, as I don't have it here to test. But as this patch doesn't
> actually fix a board but fixes an issue in the code that *might* appear in
> the future, I'm not convinced it would be the right thing to merge it like
> it is.
> 
> And I'm also a little short on time to investigate this further, as it's not
> a real bug, currently.

OK, thanks!

-- 
Tom
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: not available
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20190505/862d365b/attachment.sig>

^ permalink raw reply	[flat|nested] 15+ messages in thread

* [U-Boot] [PATCH v5 1/2] spl: fix linker size check off-by-one errors
  2019-04-25 19:22 [U-Boot] [PATCH v5 1/2] spl: fix linker size check off-by-one errors Simon Goldschmidt
  2019-04-25 19:22 ` [U-Boot] [PATCH v5 2/2] dlmalloc: fix malloc range at end of ram Simon Goldschmidt
  2019-04-27 20:17 ` [U-Boot] [PATCH v5 1/2] spl: fix linker size check off-by-one errors Simon Goldschmidt
@ 2019-05-06 11:14 ` Tom Rini
  2 siblings, 0 replies; 15+ messages in thread
From: Tom Rini @ 2019-05-06 11:14 UTC (permalink / raw)
  To: u-boot

On Thu, Apr 25, 2019 at 09:22:39PM +0200, Simon Goldschmidt wrote:

> This fixes SPL linker script size checks for 3 lds files where the size
> checks were implemented as "x < YYY_MAX_SIZE".
> 
> Fix the size checks to be "x <= YYY_MAX_SIZE" instead.
> 
> Signed-off-by: Simon Goldschmidt <simon.k.r.goldschmidt@gmail.com>

Applied to u-boot/master, thanks!

-- 
Tom
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: not available
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20190506/1ec9026e/attachment.sig>

^ permalink raw reply	[flat|nested] 15+ messages in thread

end of thread, other threads:[~2019-05-06 11:14 UTC | newest]

Thread overview: 15+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-04-25 19:22 [U-Boot] [PATCH v5 1/2] spl: fix linker size check off-by-one errors Simon Goldschmidt
2019-04-25 19:22 ` [U-Boot] [PATCH v5 2/2] dlmalloc: fix malloc range at end of ram Simon Goldschmidt
2019-04-25 22:22   ` Marek Vasut
2019-04-26  6:19     ` Simon Goldschmidt
2019-04-26  9:32       ` Marek Vasut
2019-04-26  9:36         ` Simon Goldschmidt
2019-04-26  9:56           ` Marek Vasut
2019-04-26 10:19             ` Simon Goldschmidt
2019-04-26 11:00               ` Marek Vasut
2019-05-04 18:16                 ` Simon Goldschmidt
2019-05-05 11:38                   ` Tom Rini
2019-05-05 17:55                     ` Simon Goldschmidt
2019-05-05 18:12                       ` Tom Rini
2019-04-27 20:17 ` [U-Boot] [PATCH v5 1/2] spl: fix linker size check off-by-one errors Simon Goldschmidt
2019-05-06 11:14 ` Tom Rini

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.