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

["Larry H." <research@subreption.com>]

On 19:34 Sat 30 May     , Ingo Molnar wrote:
> You need to provide a more sufficient and more constructive answer 
> than that, if you propose upstream patches that impact the SLAB 
> subsystem.

Impact? If you mean introducing changes, definitely. If the word has
negative connotations in this context, definitely not ;)

> FYI Pekka is one of the SLAB subsystem maintainers so you need to 
> convince him that your patches are the right approach. Trying to 
> teach Pekka about SLAB internals in a condescending tone will only 
> cause your patches to be ignored.

I've never tried to teach you anything but security matters, so far.
And I've been quite unsuccessful at it, apparently. That said, please
let me explain why kzfree was broken (as of 2.6.29.4, I've been told
30-rc2 already has users of it).

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.

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.

I just noticed Johannes Weiner's patch from February 16.

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.

	Larry

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>