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]

Message-ID: <CAHo-OowZ2_2LkeREVua6PdTojam_AZQEa0OLL80+t+2xiKSCRQ@mail.gmail.com>
Date:   Mon, 7 Oct 2019 22:38:15 -0700
From:   Maciej Żenczykowski <zenczykowski@...il.com>
To:     Maciej Żenczykowski <maze@...gle.com>,
        "David S . Miller" <davem@...emloft.net>
Cc:     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"

Please think both these patches through.
I'm not going to claim I'm 100% certain of their correctness.

I'm confused by:
  include/net/netfilter/nf_conntrack.h:65:
  * beware nf_ct_get() is different and don't inc refcnt.

and maybe there's some subtlety to this krealloc+rcu+kmemleak thing I'm missing.

On Mon, Oct 7, 2019 at 10:35 PM Maciej Żenczykowski
<zenczykowski@...il.com> wrote:
>
> From: Maciej Żenczykowski <maze@...gle.com>
>
> This reverts commit 114aa35d06d4920c537b72f9fa935de5dd205260.
>
> By my understanding of kmemleak the reasoning for this patch
> is incorrect.  If kmemleak couldn't handle rcu we'd have it
> reporting leaks all over the place.  My belief is that this
> was instead papering over a real leak.
>
> Cc: Cong Wang <xiyou.wangcong@...il.com>
> Cc: Eric Dumazet <edumazet@...gle.com>
> Cc: Pablo Neira Ayuso <pablo@...filter.org>
> Signed-off-by: Maciej Żenczykowski <maze@...gle.com>
> ---
>  net/netfilter/nf_conntrack_extend.c | 1 -
>  1 file changed, 1 deletion(-)
>
> diff --git a/net/netfilter/nf_conntrack_extend.c b/net/netfilter/nf_conntrack_extend.c
> index d4ed1e197921..fb208877338a 100644
> --- a/net/netfilter/nf_conntrack_extend.c
> +++ b/net/netfilter/nf_conntrack_extend.c
> @@ -68,7 +68,6 @@ void *nf_ct_ext_add(struct nf_conn *ct, enum nf_ct_ext_id id, gfp_t gfp)
>         rcu_read_unlock();
>
>         alloc = max(newlen, NF_CT_EXT_PREALLOC);
> -       kmemleak_not_leak(old);
>         new = __krealloc(old, alloc, gfp);
>         if (!new)
>                 return NULL;
> --
> 2.23.0.581.g78d2f28ef7-goog
>