lkml.org 
[lkml]   [2019]   [Oct]   [4]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    Date
    From
    SubjectRe: [PATCH] lib/generic-radix-tree.c: add kmemleak annotations
    On Thu, Oct 03, 2019 at 11:50:39PM -0700, Eric Biggers wrote:
    > From: Eric Biggers <ebiggers@google.com>
    >
    > Kmemleak is falsely reporting a leak of the slab allocation in
    > sctp_stream_init_ext():
    >
    > BUG: memory leak
    > unreferenced object 0xffff8881114f5d80 (size 96):
    > comm "syz-executor934", pid 7160, jiffies 4294993058 (age 31.950s)
    > hex dump (first 32 bytes):
    > 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 ................
    > 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 ................
    > backtrace:
    > [<00000000ce7a1326>] kmemleak_alloc_recursive include/linux/kmemleak.h:55 [inline]
    > [<00000000ce7a1326>] slab_post_alloc_hook mm/slab.h:439 [inline]
    > [<00000000ce7a1326>] slab_alloc mm/slab.c:3326 [inline]
    > [<00000000ce7a1326>] kmem_cache_alloc_trace+0x13d/0x280 mm/slab.c:3553
    > [<000000007abb7ac9>] kmalloc include/linux/slab.h:547 [inline]
    > [<000000007abb7ac9>] kzalloc include/linux/slab.h:742 [inline]
    > [<000000007abb7ac9>] sctp_stream_init_ext+0x2b/0xa0 net/sctp/stream.c:157
    > [<0000000048ecb9c1>] sctp_sendmsg_to_asoc+0x946/0xa00 net/sctp/socket.c:1882
    > [<000000004483ca2b>] sctp_sendmsg+0x2a8/0x990 net/sctp/socket.c:2102
    > [...]
    >
    > But it's freed later. Kmemleak misses the allocation because its
    > pointer is stored in the generic radix tree sctp_stream::out, and the
    > generic radix tree uses raw pages which aren't tracked by kmemleak.
    >
    > Fix this by adding the kmemleak hooks to the generic radix tree code.

    Nice, thanks Eric.

    Reviewed-by: Marcelo Ricardo Leitner <marcelo.leitner@gmail.com>

    >
    > Reported-by: syzbot+7f3b6b106be8dcdcdeec@syzkaller.appspotmail.com
    > Signed-off-by: Eric Biggers <ebiggers@google.com>
    > ---
    > lib/generic-radix-tree.c | 32 ++++++++++++++++++++++++++------
    > 1 file changed, 26 insertions(+), 6 deletions(-)
    >
    > diff --git a/lib/generic-radix-tree.c b/lib/generic-radix-tree.c
    > index ae25e2fa2187..f25eb111c051 100644
    > --- a/lib/generic-radix-tree.c
    > +++ b/lib/generic-radix-tree.c
    > @@ -2,6 +2,7 @@
    > #include <linux/export.h>
    > #include <linux/generic-radix-tree.h>
    > #include <linux/gfp.h>
    > +#include <linux/kmemleak.h>
    >
    > #define GENRADIX_ARY (PAGE_SIZE / sizeof(struct genradix_node *))
    > #define GENRADIX_ARY_SHIFT ilog2(GENRADIX_ARY)
    > @@ -75,6 +76,27 @@ void *__genradix_ptr(struct __genradix *radix, size_t offset)
    > }
    > EXPORT_SYMBOL(__genradix_ptr);
    >
    > +static inline struct genradix_node *genradix_alloc_node(gfp_t gfp_mask)
    > +{
    > + struct genradix_node *node;
    > +
    > + node = (struct genradix_node *)__get_free_page(gfp_mask|__GFP_ZERO);
    > +
    > + /*
    > + * We're using pages (not slab allocations) directly for kernel data
    > + * structures, so we need to explicitly inform kmemleak of them in order
    > + * to avoid false positive memory leak reports.
    > + */
    > + kmemleak_alloc(node, PAGE_SIZE, 1, gfp_mask);
    > + return node;
    > +}
    > +
    > +static inline void genradix_free_node(struct genradix_node *node)
    > +{
    > + kmemleak_free(node);
    > + free_page((unsigned long)node);
    > +}
    > +
    > /*
    > * Returns pointer to the specified byte @offset within @radix, allocating it if
    > * necessary - newly allocated slots are always zeroed out:
    > @@ -97,8 +119,7 @@ void *__genradix_ptr_alloc(struct __genradix *radix, size_t offset,
    > break;
    >
    > if (!new_node) {
    > - new_node = (void *)
    > - __get_free_page(gfp_mask|__GFP_ZERO);
    > + new_node = genradix_alloc_node(gfp_mask);
    > if (!new_node)
    > return NULL;
    > }
    > @@ -121,8 +142,7 @@ void *__genradix_ptr_alloc(struct __genradix *radix, size_t offset,
    > n = READ_ONCE(*p);
    > if (!n) {
    > if (!new_node) {
    > - new_node = (void *)
    > - __get_free_page(gfp_mask|__GFP_ZERO);
    > + new_node = genradix_alloc_node(gfp_mask);
    > if (!new_node)
    > return NULL;
    > }
    > @@ -133,7 +153,7 @@ void *__genradix_ptr_alloc(struct __genradix *radix, size_t offset,
    > }
    >
    > if (new_node)
    > - free_page((unsigned long) new_node);
    > + genradix_free_node(new_node);
    >
    > return &n->data[offset];
    > }
    > @@ -191,7 +211,7 @@ static void genradix_free_recurse(struct genradix_node *n, unsigned level)
    > genradix_free_recurse(n->children[i], level - 1);
    > }
    >
    > - free_page((unsigned long) n);
    > + genradix_free_node(n);
    > }
    >
    > int __genradix_prealloc(struct __genradix *radix, size_t size,
    > --
    > 2.23.0
    >

    \
     
     \ /
      Last update: 2019-10-04 14:22    [W:3.180 / U:0.464 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site