[lkml]   [2009]   [May]   [30]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
Messages in this thread
    SubjectRe: [patch 0/5] Support for sanitization flag in low-level page allocator
    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, 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, 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.


     \ /
      Last update: 2009-05-30 20:09    [W:0.027 / U:42.672 seconds]
    ©2003-2017 Jasper Spaans. hosted at Digital OceanAdvertise on this site