[webkit-dev] RenderArena: Teaching an old dog new tricks

[Adam Barth]

On Wed, Nov 14, 2012 at 2:48 PM, Geoffrey Garen <ggaren at apple.com> wrote:
> Hi Eric.
>
> Here are some problems in RenderArena that I know of:
>
> - Grows memory without bound
> - Duplicates the functionality of FastMalloc
> - Makes allocation error-prone (allocation in the wrong arena is sometimes a leak, sometimes a use-after-free, and sometimes a heizenbug of the two)
> - Makes allocation verbose (you have to thread an arena pointer everywhere)
> - Makes object lifetime complicated (all objects are implicitly tied to a single owner they may outlive)
> - Uses C-style macros and manual initialization and destruction, instead of modern WebKit C++ style
>
> You didn't mention any of these problems in your email, so I'll assume you weren't aware of them.
>
> Considering these problems now, please don't use RenderArena in more places.
>
>> Slab-allocators (i.e. RenderArena) hand out memory all from a single
>> region, guaranteeing (among other things) that free'd objects can only
>> be ever overwritten by other objects from the same pool.  This makes
>> it much harder, for example to find a Use-After-Free of a RenderBlock
>> and then fill that RenderBlock's memory (and vtable pointer) with
>> arbitrary memory (like the contents of a javascript array).
>> http://en.wikipedia.org/wiki/Slab_allocation
>
> This is magical thinking. RenderArena is no different from FastMalloc.
>
> (1) RenderArena recycles by object size, just like FastMalloc.
>
> (2) FastMalloc is a slab allocator, just like RenderArena.
>
> (3) RenderArena grows by calling FastMalloc.
>
> Isolating object types from each other -- and specifically isolating objects of arbitrary size and contents like arrays -- is an interesting idea. RenderArena is neither necessary nor sufficient for implementing this feature.

I wonder if this is the main point of miscommunication.  I think
Chris' main goal here is to isolate objects types from one another,
particularly DOM nodes, to help mitigate use-after-free
vulnerabilities.  The best path forward might be to think about how
best to achieve that goal.

It sounds like you see the potential benefits of the goal but do not
view RenerArena as the best path.  Do you have a suggestion for an
approach that Chris and/or Cris should investigate?

> The only reason RenderArena seems isolated from other object types is social, not technical: we actively discourage using RenderArena, so few object types currently use it.
>
>> Since RenderArena is generic, the current plan to move it to WTF (as
>> by Chris Marrin suggested back in
>> http://www.mail-archive.com/webkit-dev@lists.webkit.org/msg12672.html),
>> clean up the code further, and investigate wider deployment (like to
>> the DOM tree) for the security benefit and possible perf win.
>> https://bugs.webkit.org/show_bug.cgi?id=101087
>
> Having dealt with the specific technical question of RenderArena, I'd like to briefly discuss the meta-level of how the WebKit project works.
>
> Sam Weinig and I both provided review feedback saying that using RenderArena more was a bad idea (https://bugs.webkit.org/show_bug.cgi?id=101087#c9, https://bugs.webkit.org/show_bug.cgi?id=101087#c18). Nonetheless, you r+ed a patch to move in that direction, and you describe it here as the "current plan" for WebKit.

One thing that I notice about these two comments is that they say
"please don't make this change" but they don't explain why.  I realize
that it takes more time and energy to explain the "why," but it's
helpful for folks like me (and I suspect Chris and Cris as well) who
don't know these issues as well as you.

Thanks for taking the time to explain the issue in your previous
email.  That helped me understand this issue much better than before.

Adam