On 5/4/21 11:26 AM, Simon Glass wrote:
Hi Sean,

On Sun, 2 May 2021 at 20:55, Sean Anderson <[email protected]> wrote:

This fixes memory being cleared after releasing it. Instead, clear memory
before releasing it. In addition, suppress valgrind warnings about writing
to free'd memory.

Signed-off-by: Sean Anderson <[email protected]>
---

  common/dlmalloc.c | 10 ++++++----
  1 file changed, 6 insertions(+), 4 deletions(-)

diff --git a/common/dlmalloc.c b/common/dlmalloc.c
index 05c8fd87e7..ea51bdf6a6 100644
--- a/common/dlmalloc.c
+++ b/common/dlmalloc.c
@@ -592,11 +592,13 @@ void *sbrk(ptrdiff_t increment)
         ulong new = old + increment;

         /*
-        * if we are giving memory back make sure we clear it out since
-        * we set MORECORE_CLEARS to 1
+        * if we are allocating memory make sure we clear it out since we set
+        * MORECORE_CLEARS to 1
          */
-       if (increment < 0)
-               memset((void *)new, 0, -increment);
+       if (increment > 0) {
+               VALGRIND_MAKE_MEM_UNDEFINED(old, increment);
+               memset((void *)old, 0, increment);
+       }

Can you explain this a bit more? What is the difference?

As it turns out, this patch is wrong. We need to clear memory when we
release it if SYS_MALLOC_CLEAR_ON_INIT is set, since calloc assumes that
memory has already been cleared if it gets it from sbrk.

Do you need the cast?

Yes (but this is moot)

common/dlmalloc.c: In function ‘sbrk’:
common/dlmalloc.c:600:10: warning: passing argument 1 of ‘memset’ makes pointer 
from integer without a cast [-Wint-conversion]
  600 |   memset(old, 0, increment);
      |          ^~~
      |          |
      |          ulong {aka long unsigned int}
In file included from include/common.h:21,
                 from common/dlmalloc.c:1:
include/linux/string.h:111:22: note: expected ‘void *’ but argument is of type 
‘ulong’ {aka ‘long unsigned int’}
  111 | extern void * memset(void *,int,__kernel_size_t);
      |                      ^~~~~~

--Sean



         if ((new < mem_malloc_start) || (new > mem_malloc_end))
                 return (void *)MORECORE_FAILURE;
--
2.31.0


Regards,
Simon



Reply via email to