[U-Boot] [PATCH V2 1/2] malloc: use hidden visibility

[Tom Rini]

On Sat, Mar 05, 2016 at 11:31:46AM -0700, Stephen Warren wrote:
> On 03/05/2016 10:44 AM, Tom Rini wrote:
> >On Sat, Mar 05, 2016 at 10:30:52AM -0700, Stephen Warren 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 <rabin at rab.in>
> >>Signed-off-by: Stephen Warren <swarren at wwwdotorg.org>
> >
> >Reviewed-by: Tom Rini <trini at konsulko.com>
> >
> >But have you tried with clang instead of gcc?  I suspect it's got code
> >to pretend to be gcc and do the right thing here but it's worth
> >confirming.
> 
> Well, clang for sandbox doesn't compile for reasons other than these
> patches on either Ubuntu 14.04 or 16.04 pre-release, nor using the
> instructions in doc/README.clang for rpi on 16.04. However, it did
> work for rpi on Ubuntu 14.04.
> 
> So in summary, I think there's no issue using this pragma with
> clang, but I can't test that clang actually implements it and hence
> solves the problem these patches fix.

OK, I did a quick local check after fixing the __BIGGEST_ALIGNMENT__
problem (but not the linking problem) and see llvm 3.5 at least is OK
with this, so we're good enough then, thanks!

-- 
Tom
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 836 bytes
Desc: Digital signature
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20160305/2d169f1d/attachment.sig>