netdev - Re: [PATCH 2/2] netfilter: revert "conntrack: silent a memory leak warning"

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:   Tue, 8 Oct 2019 00:10:04 -0700
From:   Maciej Żenczykowski <zenczykowski@...il.com>
To:     Florian Westphal <fw@...len.de>
Cc:     "David S . Miller" <davem@...emloft.net>,
        Linux NetDev <netdev@...r.kernel.org>,
        Cong Wang <xiyou.wangcong@...il.com>,
        Eric Dumazet <edumazet@...gle.com>,
        Pablo Neira Ayuso <pablo@...filter.org>
Subject: Re: [PATCH 2/2] netfilter: revert "conntrack: silent a memory leak warning"

Here's my reasoning:

        old = ct->ext;

        //... stuff that doesn't change old.

        alloc = max(newlen, NF_CT_EXT_PREALLOC);  <-- will be >= 128,
so not zero
        kmemleak_not_leak(old);
        new = __krealloc(old, alloc, gfp);
        if (!new)
                return NULL;  <--- if we return here, ct->ext still
holds old, so no leak.

        if (!old) {
                memset(new->offset, 0, sizeof(new->offset));
                ct->ext = new;  <--- old is NULL so can't leak
        } else if (new != old) {
                kfree_rcu(old, rcu);  <-- we free old, so doesn't leak
                rcu_assign_pointer(ct->ext, new);
        } <--- else new == old && it's still in ct->ext, so it doesn't leak

Basically AFAICT our use of __krealloc() is exactly like krealloc()
except instead of kfree() we do kfree_rcu().

And thus I don't understand the need for kmemleak_not_leak(old).

So... what's my mistake?