linux-kernel - Re: [PATCH] netlink: fix memory leak of dump

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: <20180722.100755.19840167505550163.davem@davemloft.net>
Date:   Sun, 22 Jul 2018 10:07:55 -0700 (PDT)
From:   David Miller <davem@...emloft.net>
To:     fw@...len.de
Cc:     cscnull@...il.com, pablo@...filter.org, kadlec@...ckhole.kfki.hu,
        johannes.berg@...el.com, Jason@...c4.com, ktkhai@...tuozzo.com,
        lucien.xin@...il.com, xiyou.wangcong@...il.com, dsahern@...il.com,
        netfilter-devel@...r.kernel.org, tom@...ntonium.net,
        netdev@...r.kernel.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH] netlink: fix memory leak of dump

From: Florian Westphal <fw@...len.de>
Date: Sun, 22 Jul 2018 18:39:25 +0200

> 3. change meaning of ->done() so its always called once ->start()
>    was invoked (and returned 0), this requires audit of all
>    places that provide .done to make sure they won't trip.
> 
> 3) seems to be what Tom intended when he added .start, so probably
> best to investigate that first.

Hmmm...

Any time ->start() succeeds, we set cb_running to true.

>From that point forward, ->done() will be called at some point at all
of the locations that check if cb_running is true and set it to false.

This may be deferred all the way to socket destruction, but it will
eventually happens.

Nothing sets cb_running to false without invoking the ->done()
callback.

Just because the individual dump invocations don't call ->done() does
not mean it will not eventually happen.  The dump callbacks, and thus
the state data, is live across multiple rounds of recvmsg() calls by
the user and that is as-designed.