lkml.org 
[lkml]   [2011]   [Feb]   [15]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    Date
    SubjectRe: [Bugme-new] [Bug 27212] New: Warning kmemcheck: Caught 64-bit read from uninitialized memory in netlink_broadcast_filtered
    From
    On Mon, Feb 14, 2011 at 7:35 PM, Eric Dumazet <eric.dumazet@gmail.com> wrote:
    > Le vendredi 21 janvier 2011 à 09:49 +0200, Pekka Enberg a écrit :
    >
    >> It actually looks like a bug in SLUB+kmemcheck. The
    >> kmemcheck_slab_alloc() call in slab_post_alloc_hook() should use ksize()
    >> instead of s->objsize. SLAB seems to do the right thing already. Anyone
    >> care to send a patch my way?
    >>
    >
    > Hmm, what do you think of following patch ?
    >
    > Thanks, and sorry for the delay.

    Looks good to me. Christoph, David, any objections to the patch?

    > [PATCH] slub: fix kmemcheck calls to match ksize() hints
    >
    > Recent use of ksize() in network stack (commit ca44ac38 : net: don't
    > reallocate skb->head unless the current one hasn't the needed extra size
    > or is shared) triggers kmemcheck warnings, because ksize() can return
    > more space than kmemcheck is aware of.
    >
    > Pekka Enberg noticed SLAB+kmemcheck is doing the right thing, while SLUB
    > +kmemcheck doesnt.
    >
    > Bugzilla reference #27212
    >
    > Reported-by: Christian Casteyde <casteyde.christian@free.fr>
    > Suggested-by: Pekka Enberg <penberg@kernel.org>
    > Signed-off-by: Eric Dumazet <eric.dumazet@gmail.com>
    > CC: David Miller <davem@davemloft.net>
    > CC: Changli Gao <xiaosuo@gmail.com>
    > CC: Andrew Morton <akpm@linux-foundation.org>
    > ---
    >  mm/slub.c |   49 ++++++++++++++++++++++++++-----------------------
    >  1 file changed, 26 insertions(+), 23 deletions(-)
    >
    > diff --git a/mm/slub.c b/mm/slub.c
    > index e15aa7f..ee0aeb8 100644
    > --- a/mm/slub.c
    > +++ b/mm/slub.c
    > @@ -797,10 +797,34 @@ static inline int slab_pre_alloc_hook(struct kmem_cache *s, gfp_t flags)
    >        return should_failslab(s->objsize, flags, s->flags);
    >  }
    >
    > +static inline size_t slab_ksize(const struct kmem_cache *s)
    > +{
    > +#ifdef CONFIG_SLUB_DEBUG
    > +       /*
    > +        * Debugging requires use of the padding between object
    > +        * and whatever may come after it.
    > +        */
    > +       if (s->flags & (SLAB_RED_ZONE | SLAB_POISON))
    > +               return s->objsize;
    > +
    > +#endif
    > +       /*
    > +        * If we have the need to store the freelist pointer
    > +        * back there or track user information then we can
    > +        * only use the space before that information.
    > +        */
    > +       if (s->flags & (SLAB_DESTROY_BY_RCU | SLAB_STORE_USER))
    > +               return s->inuse;
    > +       /*
    > +        * Else we can use all the padding etc for the allocation
    > +        */
    > +       return s->size;
    > +}
    > +
    >  static inline void slab_post_alloc_hook(struct kmem_cache *s, gfp_t flags, void *object)
    >  {
    >        flags &= gfp_allowed_mask;
    > -       kmemcheck_slab_alloc(s, flags, object, s->objsize);
    > +       kmemcheck_slab_alloc(s, flags, object, slab_ksize(s));
    >        kmemleak_alloc_recursive(object, s->objsize, 1, s->flags, flags);
    >  }
    >
    > @@ -2696,7 +2720,6 @@ EXPORT_SYMBOL(__kmalloc_node);
    >  size_t ksize(const void *object)
    >  {
    >        struct page *page;
    > -       struct kmem_cache *s;
    >
    >        if (unlikely(object == ZERO_SIZE_PTR))
    >                return 0;
    > @@ -2707,28 +2730,8 @@ size_t ksize(const void *object)
    >                WARN_ON(!PageCompound(page));
    >                return PAGE_SIZE << compound_order(page);
    >        }
    > -       s = page->slab;
    >
    > -#ifdef CONFIG_SLUB_DEBUG
    > -       /*
    > -        * Debugging requires use of the padding between object
    > -        * and whatever may come after it.
    > -        */
    > -       if (s->flags & (SLAB_RED_ZONE | SLAB_POISON))
    > -               return s->objsize;
    > -
    > -#endif
    > -       /*
    > -        * If we have the need to store the freelist pointer
    > -        * back there or track user information then we can
    > -        * only use the space before that information.
    > -        */
    > -       if (s->flags & (SLAB_DESTROY_BY_RCU | SLAB_STORE_USER))
    > -               return s->inuse;
    > -       /*
    > -        * Else we can use all the padding etc for the allocation
    > -        */
    > -       return s->size;
    > +       return slab_ksize(page->slab);
    >  }
    >  EXPORT_SYMBOL(ksize);
    >
    >
    >
    > --
    > To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
    > the body of a message to majordomo@vger.kernel.org
    > More majordomo info at  http://vger.kernel.org/majordomo-info.html
    > Please read the FAQ at  http://www.tux.org/lkml/
    >
    --
    To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
    the body of a message to majordomo@vger.kernel.org
    More majordomo info at http://vger.kernel.org/majordomo-info.html
    Please read the FAQ at http://www.tux.org/lkml/

    \
     
     \ /
      Last update: 2011-02-15 06:51    [W:3.305 / U:0.676 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site