netdev.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Shaochun Chen <cscnull@gmail.com>
To: pablo@netfilter.org
Cc: kadlec@blackhole.kfki.hu, fw@strlen.de, davem@davemloft.net,
	johannes.berg@intel.com, Jason@zx2c4.com, ktkhai@virtuozzo.com,
	lucien.xin@gmail.com, xiyou.wangcong@gmail.com,
	dsahern@gmail.com, netfilter-devel@vger.kernel.org,
	coreteam@netfilter.org, netdev@vger.kernel.org,
	linux-kernel@vger.kernel.org, Shaochun Chen <cscnull@gmail.com>
Subject: [PATCH] netlink: fix memory leak of dump
Date: Sun, 22 Jul 2018 22:33:54 +0800	[thread overview]
Message-ID: <20180722143354.23722-1-cscnull@gmail.com> (raw)

1) if netlink_dump_start start fail, the memory of c->data will leak.
so free manually after netlink_dump_start return error.

2) In netlink_dump_start, ignore the return of netlink_dump.
Because if cb_running is set to true, cb->dump will be call in anyway.
so if netlink_dump_start start successfully, just return -EINTR.

Signed-off-by: Shaochun Chen <cscnull@gmail.com>
---
 net/netfilter/nf_conntrack_netlink.c |  8 ++++--
 net/netfilter/nf_tables_api.c        | 41 ++++++++++++++++++++--------
 net/netfilter/nfnetlink_acct.c       |  8 ++++--
 net/netlink/af_netlink.c             |  5 +---
 4 files changed, 41 insertions(+), 21 deletions(-)

diff --git a/net/netfilter/nf_conntrack_netlink.c b/net/netfilter/nf_conntrack_netlink.c
index 20a2e37c76d1..31db758bdb7a 100644
--- a/net/netfilter/nf_conntrack_netlink.c
+++ b/net/netfilter/nf_conntrack_netlink.c
@@ -1243,17 +1243,19 @@ static int ctnetlink_get_conntrack(struct net *net, struct sock *ctnl,
 			.dump = ctnetlink_dump_table,
 			.done = ctnetlink_done,
 		};
+		struct ctnetlink_filter *filter = NULL;
 
 		if (cda[CTA_MARK] && cda[CTA_MARK_MASK]) {
-			struct ctnetlink_filter *filter;
-
 			filter = ctnetlink_alloc_filter(cda);
 			if (IS_ERR(filter))
 				return PTR_ERR(filter);
 
 			c.data = filter;
 		}
-		return netlink_dump_start(ctnl, skb, nlh, &c);
+		err = netlink_dump_start(ctnl, skb, nlh, &c);
+		if (err != -EINTR && filter)
+			kfree(filter);
+		return err;
 	}
 
 	err = ctnetlink_parse_zone(cda[CTA_ZONE], &zone);
diff --git a/net/netfilter/nf_tables_api.c b/net/netfilter/nf_tables_api.c
index 896d4a36081d..4635a841f5f2 100644
--- a/net/netfilter/nf_tables_api.c
+++ b/net/netfilter/nf_tables_api.c
@@ -2287,10 +2287,9 @@ static int nf_tables_getrule(struct net *net, struct sock *nlsk,
 			.done = nf_tables_dump_rules_done,
 			.module = THIS_MODULE,
 		};
+		struct nft_rule_dump_ctx *ctx = NULL;
 
 		if (nla[NFTA_RULE_TABLE] || nla[NFTA_RULE_CHAIN]) {
-			struct nft_rule_dump_ctx *ctx;
-
 			ctx = kzalloc(sizeof(*ctx), GFP_ATOMIC);
 			if (!ctx)
 				return -ENOMEM;
@@ -2315,7 +2314,13 @@ static int nf_tables_getrule(struct net *net, struct sock *nlsk,
 			c.data = ctx;
 		}
 
-		return nft_netlink_dump_start_rcu(nlsk, skb, nlh, &c);
+		err = nft_netlink_dump_start_rcu(nlsk, skb, nlh, &c);
+		if (err != -EINTR && ctx) {
+			kfree(ctx->table);
+			kfree(ctx->chain);
+			kfree(ctx);
+		}
+		return err;
 	}
 
 	table = nft_table_lookup(net, nla[NFTA_RULE_TABLE], family, genmask);
@@ -3201,7 +3206,10 @@ static int nf_tables_getset(struct net *net, struct sock *nlsk,
 		*ctx_dump = ctx;
 		c.data = ctx_dump;
 
-		return nft_netlink_dump_start_rcu(nlsk, skb, nlh, &c);
+		err = nft_netlink_dump_start_rcu(nlsk, skb, nlh, &c);
+		if (err != -EINTR)
+			kfree(ctx_dump);
+		return err;
 	}
 
 	/* Only accept unspec with dump */
@@ -4016,7 +4024,10 @@ static int nf_tables_getsetelem(struct net *net, struct sock *nlsk,
 		dump_ctx->ctx = ctx;
 
 		c.data = dump_ctx;
-		return nft_netlink_dump_start_rcu(nlsk, skb, nlh, &c);
+		err = nft_netlink_dump_start_rcu(nlsk, skb, nlh, &c);
+		if (err != -EINTR)
+			kfree(dump_ctx);
+		return err;
 	}
 
 	if (!nla[NFTA_SET_ELEM_LIST_ELEMENTS])
@@ -5031,18 +5042,22 @@ static int nf_tables_getobj(struct net *net, struct sock *nlsk,
 			.done = nf_tables_dump_obj_done,
 			.module = THIS_MODULE,
 		};
+		struct nft_obj_filter *filter = NULL;
 
 		if (nla[NFTA_OBJ_TABLE] ||
 		    nla[NFTA_OBJ_TYPE]) {
-			struct nft_obj_filter *filter;
-
 			filter = nft_obj_filter_alloc(nla);
 			if (IS_ERR(filter))
 				return -ENOMEM;
 
 			c.data = filter;
 		}
-		return nft_netlink_dump_start_rcu(nlsk, skb, nlh, &c);
+		err = nft_netlink_dump_start_rcu(nlsk, skb, nlh, &c);
+		if (err != -EINTR && filter) {
+			kfree(filter->table);
+			kfree(filter);
+		}
+		return err;
 	}
 
 	if (!nla[NFTA_OBJ_NAME] ||
@@ -5704,17 +5719,21 @@ static int nf_tables_getflowtable(struct net *net, struct sock *nlsk,
 			.done = nf_tables_dump_flowtable_done,
 			.module = THIS_MODULE,
 		};
+		struct nft_flowtable_filter *filter = NULL;
 
 		if (nla[NFTA_FLOWTABLE_TABLE]) {
-			struct nft_flowtable_filter *filter;
-
 			filter = nft_flowtable_filter_alloc(nla);
 			if (IS_ERR(filter))
 				return -ENOMEM;
 
 			c.data = filter;
 		}
-		return nft_netlink_dump_start_rcu(nlsk, skb, nlh, &c);
+		err = nft_netlink_dump_start_rcu(nlsk, skb, nlh, &c);
+		if (err != -EINTR && filter) {
+			kfree(filter->table);
+			kfree(filter);
+		}
+		return err;
 	}
 
 	if (!nla[NFTA_FLOWTABLE_NAME])
diff --git a/net/netfilter/nfnetlink_acct.c b/net/netfilter/nfnetlink_acct.c
index a0e5adf0b3b6..aecdb9cc1c5d 100644
--- a/net/netfilter/nfnetlink_acct.c
+++ b/net/netfilter/nfnetlink_acct.c
@@ -277,17 +277,19 @@ static int nfnl_acct_get(struct net *net, struct sock *nfnl,
 			.dump = nfnl_acct_dump,
 			.done = nfnl_acct_done,
 		};
+		struct nfacct_filter *filter = NULL;
 
 		if (tb[NFACCT_FILTER]) {
-			struct nfacct_filter *filter;
-
 			filter = nfacct_filter_alloc(tb[NFACCT_FILTER]);
 			if (IS_ERR(filter))
 				return PTR_ERR(filter);
 
 			c.data = filter;
 		}
-		return netlink_dump_start(nfnl, skb, nlh, &c);
+		ret = netlink_dump_start(nfnl, skb, nlh, &c);
+		if (ret != -EINTR && filter)
+			kfree(filter);
+		return ret;
 	}
 
 	if (!tb[NFACCT_NAME])
diff --git a/net/netlink/af_netlink.c b/net/netlink/af_netlink.c
index 393573a99a5a..61450461378c 100644
--- a/net/netlink/af_netlink.c
+++ b/net/netlink/af_netlink.c
@@ -2320,13 +2320,10 @@ int __netlink_dump_start(struct sock *ssk, struct sk_buff *skb,
 
 	mutex_unlock(nlk->cb_mutex);
 
-	ret = netlink_dump(sk);
+	netlink_dump(sk);
 
 	sock_put(sk);
 
-	if (ret)
-		return ret;
-
 	/* We successfully started a dump, by returning -EINTR we
 	 * signal not to send ACK even if it was requested.
 	 */
-- 
2.17.1

             reply	other threads:[~2018-07-22 14:33 UTC|newest]

Thread overview: 15+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2018-07-22 14:33 Shaochun Chen [this message]
2018-07-22 16:39 ` [PATCH] netlink: fix memory leak of dump Florian Westphal
2018-07-22 17:07   ` David Miller
2018-07-22 18:09     ` Florian Westphal
2018-07-23  2:05       ` shaochun chen
2018-07-23  9:15       ` Pablo Neira Ayuso
2018-07-23  9:28         ` Florian Westphal
2018-07-23  9:38           ` Pablo Neira Ayuso
2018-07-23  9:42             ` Florian Westphal
2018-07-23  9:47               ` Pablo Neira Ayuso
2018-07-23 10:51                 ` Florian Westphal
2018-07-23  9:52           ` shaochun chen
2018-07-23 10:34             ` shaochun chen
2018-07-23 10:43               ` Pablo Neira Ayuso
2018-07-23 10:59               ` Florian Westphal

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20180722143354.23722-1-cscnull@gmail.com \
    --to=cscnull@gmail.com \
    --cc=Jason@zx2c4.com \
    --cc=coreteam@netfilter.org \
    --cc=davem@davemloft.net \
    --cc=dsahern@gmail.com \
    --cc=fw@strlen.de \
    --cc=johannes.berg@intel.com \
    --cc=kadlec@blackhole.kfki.hu \
    --cc=ktkhai@virtuozzo.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=lucien.xin@gmail.com \
    --cc=netdev@vger.kernel.org \
    --cc=netfilter-devel@vger.kernel.org \
    --cc=pablo@netfilter.org \
    --cc=xiyou.wangcong@gmail.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).