On 5 March 2016 at 10:30, Stephen Warren <swar...@wwwdotorg.org> wrote: > When running sandbox, the following phases occur, each with different > malloc implementations or behaviors: > > 1) Dynamic linker execution, using the dynamic linker's own malloc() > implementation. This is fully functional. > > 2) After U-Boot's malloc symbol has been hooked into the GOT, but before > any U-Boot code has run. This phase is entirely non-functional, since > U-Boot's gd symbol is NULL and U-Boot's initf_malloc() and > mem_malloc_init() have not been called. > > At least on Ubuntu Xenial, the dynamic linker does make both malloc() and > free() calls during this phase. Currently these free() calls crash since > they dereference gd, which is NULL. > > U-Boot itself makes no use of malloc() during this phase. > > 3) U-Boot execution after gd is set and initf_malloc() has been called. > This is fully functional, albeit via a very simple malloc() > implementation. > > 4) U-Boot execution after mem_malloc_init() has been called. This is fully > functional with a complete malloc() implementation. > > Furthermore, if code that called malloc() during phase 1 calls free() in > phase 3 or later, it is likely that heap corruption will occur, since > U-Boot's malloc implementation will assume the pointer is part of its own > heap, although it isn't. I have not actively observed this happening. > > To prevent phase 2 from happening, this patch makes all of U-Boot's malloc > library public symbols have hidden visibility. This prevents them from > being hooked into the GOT, so only code in the U-Boot binary itself > actually calls them; any other code will call into the standard C library > malloc(). This also avoids the "furthermore" issue mentioned above. > > I have seen references to this GCC pragma in blog posts from 2008, and > RHEL5's ancient gcc appears to accept it fine, so I believe it's quite > safe to use it without checking gcc version. > > Cc: Rabin Vincent <ra...@rab.in> > Signed-off-by: Stephen Warren <swar...@wwwdotorg.org> > --- > v2: A whole different approach. > --- > include/malloc.h | 2 ++ > 1 file changed, 2 insertions(+) > > diff --git a/include/malloc.h b/include/malloc.h > index f20e4d3d2a6b..8175c75920cf 100644 > --- a/include/malloc.h > +++ b/include/malloc.h > @@ -914,6 +914,7 @@ int initf_malloc(void); > /* Simple versions which can be used when space is tight */ > void *malloc_simple(size_t size); > > +#pragma GCC visibility push(hidden) > # if __STD_C > > Void_t* mALLOc(size_t); > @@ -945,6 +946,7 @@ int mALLOPt(); > struct mallinfo mALLINFo(); > # endif > #endif > +#pragma GCC visibility pop > > /* > * Begin and End of memory area for malloc(), and current "brk" > -- > 2.7.0 >
This seems better than what we have. Thanks for digging into it. Reviewed-by: Simon Glass <s...@chromium.org> _______________________________________________ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
