| Message ID | 20190805214917.13747-1-fw@strlen.de |
|---|---|
| State | RFC |
| Delegated to: | Pablo Neira |
| Headers | show |
| Series | [RFC,nft] src: avoid re-initing core library when a second context struct is allocated | expand |
On Mon, Aug 05, 2019 at 11:49:17PM +0200, Florian Westphal wrote: > Calling nft_ctx_new() a second time leaks memory, and calling > nft_ctx_free a second time -- on a different context -- causes > double-free. > > This patch won't work in case we assume libnftables should be > thread-safe, in such case we either need a mutex or move all resources > under nft_ctx scope. These two should avoid the memleak / double free I think: https://patchwork.ozlabs.org/patch/1143742/ https://patchwork.ozlabs.org/patch/1143743/ Not thread-safe yet, there is a bunch global variables still in place.
Pablo Neira Ayuso <pablo@netfilter.org> wrote: > On Mon, Aug 05, 2019 at 11:49:17PM +0200, Florian Westphal wrote: > > Calling nft_ctx_new() a second time leaks memory, and calling > > nft_ctx_free a second time -- on a different context -- causes > > double-free. > > > > This patch won't work in case we assume libnftables should be > > thread-safe, in such case we either need a mutex or move all resources > > under nft_ctx scope. > > These two should avoid the memleak / double free I think: > > https://patchwork.ozlabs.org/patch/1143742/ > https://patchwork.ozlabs.org/patch/1143743/ Thanks, I will give them a try. > Not thread-safe yet, there is a bunch global variables still in place. I don't need thread-safety at the moment, I just found this double-free crash when creating another nft_ctx inside nftables (don't ask why, its fugly...)
diff --git a/src/libnftables.c b/src/libnftables.c index 4a139c58b2b3..880fd4284555 100644 --- a/src/libnftables.c +++ b/src/libnftables.c @@ -17,6 +17,8 @@ #include <stdlib.h> #include <string.h> +static unsigned int context_count; + static int nft_netlink(struct nft_ctx *nft, struct list_head *cmds, struct list_head *msgs, struct mnl_socket *nf_sock) @@ -86,6 +88,9 @@ out: static void nft_init(void) { + if (context_count++) + return; + mark_table_init(); realm_table_rt_init(); devgroup_table_init(); @@ -99,6 +104,9 @@ static void nft_init(void) static void nft_exit(void) { + if (--context_count) + return; + ct_label_table_exit(); realm_table_rt_exit(); devgroup_table_exit();
Calling nft_ctx_new() a second time leaks memory, and calling nft_ctx_free a second time -- on a different context -- causes double-free. This patch won't work in case we assume libnftables should be thread-safe, in such case we either need a mutex or move all resources under nft_ctx scope. Signed-off-by: Florian Westphal <fw@strlen.de> --- src/libnftables.c | 8 ++++++++ 1 file changed, 8 insertions(+)