Re: [patch 0/5] Support for sanitization flag in low-level page allocator

[Pekka Enberg <penberg@cs.helsinki.fi>]

Hi Larry,

On Sat, May 30, 2009 at 9:03 PM, Larry H. <research@subreption.com> wrote:
> The first issue is that SLOB has a broken ksize, which won't take into
> consideration compound pages AFAIK. To fix this you will need to
> introduce some changes in the way the slob_page structure is handled,
> and add real size tracking to it. You will find these problems if you
> try to implement a reliable kmem_ptr_validate for SLOB, too.

Does this mean that kzfree() isn't broken for SLAB/SLUB? Maybe I read
your emails wrong but you seemed to imply that.

As for SLOB ksize(), I am sure Matt Mackall would love to hear the
details how ksize() is broken there. I am having difficult time
understanding the bug you're pointing out here as SLOB does check for
is_slob_page() in ksize() and falls back to page.private if the page
is not PageSlobPage...

On Sat, May 30, 2009 at 9:03 PM, Larry H. <research@subreption.com> wrote:
> The second is that I've experienced issues with kzfree on 2.6.29.4, in
> which something (apparently the freelist pointer) is overwritten and
> leads to a NULL pointer deference in the next allocation in the affected
> cache. I didn't fully analyze what was broken, besides that for
> sanitizing the objects on kfree I needed to rely on the inuse size and
> not the one reported by ksize, if I wanted to avoid hitting that
> trailing meta-data.

Which allocator are you talking about here?

On Sat, May 30, 2009 at 9:03 PM, Larry H. <research@subreption.com> wrote:
> BTW, talking about branches and call depth, you are proposing using
> kzfree() which involves further test and call branches (including those
> inside the specific ksize implementation of the allocator being used)
> and it duplicates the check for ZERO_SIZE_PTR/NULL too. The function is
> so simple that it should be a static inline declared in slab.h. It also
> lacks any validation checks as performed in kfree (besides the zero
> size/null ptr one).
>
> Also, users of unconditional sanitization would see unnecessary
> duplication of the clearing, causing a real performance hit (which would
> be almost non existent otherwise). That will make kzfree unsuitable for
> most hot spots like the crypto api and the mac80211 wep code.
>
> Honestly your proposed approach seems a little weak.

Honestly, this seems like more hand-waving to me.

                                       Pekka