Possible memory leak in genl_ctrl_probe_by_name()

[Thomas Graf]

On 03/29/13 at 05:01pm, Дмитрий Дороговцев wrote:
> Hello.
> 
> This is a small program using genl_ctrl_resolve(copied from test-genl.c).
> Compile: gcc -o test main.c -I libnl-3.2.21/include/
> libnl-3.2.21/lib/.libs/libnl-3.a libnl-3.2.21/lib/.libs/libnl-genl-3.a
> -lpthread -lm
> Run memcheck: valgrind --leak-check=full --show-reachable=yes ./test
> 
> I got following output:
> ...
> ==24905== HEAP SUMMARY:
> ==24905==     in use at exit: 112 bytes in 1 blocks
> ==24905==   total heap usage: 148 allocs, 147 frees, 31,720 bytes allocated
> ==24905==
> ==24905== 112 bytes in 1 blocks are definitely lost in loss record 1 of 1
> ==24905==    at 0x402A5E6: calloc (in
> /usr/lib/valgrind/vgpreload_memcheck-x86-linux.so)
> ==24905==    by 0x804F172: nl_cb_alloc (handlers.c:208)
> ==24905==    by 0x40A84D2: (below main) (libc-start.c:226)
> ==24905==
> ==24905== LEAK SUMMARY:
> ==24905==    definitely lost: 112 bytes in 1 blocks
> ==24905==    indirectly lost: 0 bytes in 0 blocks
> ==24905==      possibly lost: 0 bytes in 0 blocks
> ==24905==    still reachable: 0 bytes in 0 blocks
> ==24905==         suppressed: 0 bytes in 0 blocks
> ...
> 
> I think it's a bug in genl_ctrl_probe_by_name():
> 
> ...
> if (!(cb = nl_cb_clone(nl_socket_get_cb(sk))))
>         goto out_msg_free;
> ...
> 
> Here we increase ref count on sk->s_cb, so i think it should be decremented
> later.
> With this patch i don't have valgrind memleak anymore:

Thanks for reporting this, I fixed it in a slightly different way to
fix the leak where it occurs rather than in the caller.

commit df66b0f267af636848d7f0301d3f5863c58fb313
Author: Thomas Graf <tgraf at suug.ch>
Date:   Tue Apr 2 11:51:53 2013 +0200

    genl: Fix cb reference leak in genl_ctrl_probe_by_name()
    
    nl_socket_get_cb() bumps the cb reference counter
    
    Signed-off-by: Thomas Graf <tgraf at suug.ch>

diff --git a/lib/genl/ctrl.c b/lib/genl/ctrl.c
index c6db742..ce07f1d 100644
--- a/lib/genl/ctrl.c
+++ b/lib/genl/ctrl.c
@@ -239,7 +239,7 @@ static struct genl_family *genl_ctrl_probe_by_name(struct nl_sock *sk,
 {
        struct nl_msg *msg;
        struct genl_family *ret;
-       struct nl_cb *cb;
+       struct nl_cb *cb, *orig;
        int rc;
 
        ret = genl_family_alloc();
@@ -252,7 +252,12 @@ static struct genl_family *genl_ctrl_probe_by_name(struct nl_sock *sk,
        if (!msg)
                goto out_fam_free;
 
-       if (!(cb = nl_cb_clone(nl_socket_get_cb(sk))))
+       if (!(orig = nl_socket_get_cb(sk)))
+               goto out_msg_free;
+
+       cb = nl_cb_clone(orig);
+       nl_cb_put(orig);
+       if (!cb)
                goto out_msg_free;
 
        if (!genlmsg_put(msg, NL_AUTO_PORT, NL_AUTO_SEQ, GENL_ID_CTRL,