From c0ea1bcb39352b57ac5c4b6da8acd65bddeee2c5 Mon Sep 17 00:00:00 2001 From: Pablo Neira Ayuso Date: Tue, 6 Feb 2018 13:22:44 +0100 Subject: netfilter: nft_flow_offload: move flowtable cleanup routines to nf_flow_table Move the flowtable cleanup routines to nf_flow_table and expose the nf_flow_table_cleanup() helper function. Signed-off-by: Pablo Neira Ayuso --- include/net/netfilter/nf_flow_table.h | 3 +++ 1 file changed, 3 insertions(+) (limited to 'include') diff --git a/include/net/netfilter/nf_flow_table.h b/include/net/netfilter/nf_flow_table.h index b22b22082733..ed49cd169ecf 100644 --- a/include/net/netfilter/nf_flow_table.h +++ b/include/net/netfilter/nf_flow_table.h @@ -95,6 +95,9 @@ struct flow_offload_tuple_rhash *flow_offload_lookup(struct nf_flowtable *flow_t int nf_flow_table_iterate(struct nf_flowtable *flow_table, void (*iter)(struct flow_offload *flow, void *data), void *data); + +void nf_flow_table_cleanup(struct net *net, struct net_device *dev); + void nf_flow_offload_work_gc(struct work_struct *work); extern const struct rhashtable_params nf_flow_offload_rhash_params; -- cgit v1.2.3-59-g8ed1b From b408c5b04f82fe4e20bceb8e4f219453d4f21f02 Mon Sep 17 00:00:00 2001 From: Pablo Neira Ayuso Date: Tue, 6 Feb 2018 13:22:47 +0100 Subject: netfilter: nf_tables: fix flowtable free Every flow_offload entry is added into the table twice. Because of this, rhashtable_free_and_destroy can't be used, since it would call kfree for each flow_offload object twice. This patch cleans up the flowtable via nf_flow_table_iterate() to schedule removal of entries by setting on the dying bit, then there is an explicitly invocation of the garbage collector to release resources. Based on patch from Felix Fietkau. Signed-off-by: Pablo Neira Ayuso --- include/net/netfilter/nf_flow_table.h | 2 ++ net/ipv4/netfilter/nf_flow_table_ipv4.c | 1 + net/ipv6/netfilter/nf_flow_table_ipv6.c | 1 + net/netfilter/nf_flow_table.c | 25 +++++++++++++++++++------ net/netfilter/nf_flow_table_inet.c | 1 + net/netfilter/nf_tables_api.c | 9 ++------- 6 files changed, 26 insertions(+), 13 deletions(-) (limited to 'include') diff --git a/include/net/netfilter/nf_flow_table.h b/include/net/netfilter/nf_flow_table.h index ed49cd169ecf..020ae903066f 100644 --- a/include/net/netfilter/nf_flow_table.h +++ b/include/net/netfilter/nf_flow_table.h @@ -14,6 +14,7 @@ struct nf_flowtable_type { struct list_head list; int family; void (*gc)(struct work_struct *work); + void (*free)(struct nf_flowtable *ft); const struct rhashtable_params *params; nf_hookfn *hook; struct module *owner; @@ -98,6 +99,7 @@ int nf_flow_table_iterate(struct nf_flowtable *flow_table, void nf_flow_table_cleanup(struct net *net, struct net_device *dev); +void nf_flow_table_free(struct nf_flowtable *flow_table); void nf_flow_offload_work_gc(struct work_struct *work); extern const struct rhashtable_params nf_flow_offload_rhash_params; diff --git a/net/ipv4/netfilter/nf_flow_table_ipv4.c b/net/ipv4/netfilter/nf_flow_table_ipv4.c index b2d01eb25f2c..25d2975da156 100644 --- a/net/ipv4/netfilter/nf_flow_table_ipv4.c +++ b/net/ipv4/netfilter/nf_flow_table_ipv4.c @@ -260,6 +260,7 @@ static struct nf_flowtable_type flowtable_ipv4 = { .family = NFPROTO_IPV4, .params = &nf_flow_offload_rhash_params, .gc = nf_flow_offload_work_gc, + .free = nf_flow_table_free, .hook = nf_flow_offload_ip_hook, .owner = THIS_MODULE, }; diff --git a/net/ipv6/netfilter/nf_flow_table_ipv6.c b/net/ipv6/netfilter/nf_flow_table_ipv6.c index fff21602875a..d346705d6ee6 100644 --- a/net/ipv6/netfilter/nf_flow_table_ipv6.c +++ b/net/ipv6/netfilter/nf_flow_table_ipv6.c @@ -253,6 +253,7 @@ static struct nf_flowtable_type flowtable_ipv6 = { .family = NFPROTO_IPV6, .params = &nf_flow_offload_rhash_params, .gc = nf_flow_offload_work_gc, + .free = nf_flow_table_free, .hook = nf_flow_offload_ipv6_hook, .owner = THIS_MODULE, }; diff --git a/net/netfilter/nf_flow_table.c b/net/netfilter/nf_flow_table.c index 04c08f6b9015..c17f1af42daa 100644 --- a/net/netfilter/nf_flow_table.c +++ b/net/netfilter/nf_flow_table.c @@ -232,19 +232,16 @@ static inline bool nf_flow_is_dying(const struct flow_offload *flow) return flow->flags & FLOW_OFFLOAD_DYING; } -void nf_flow_offload_work_gc(struct work_struct *work) +static int nf_flow_offload_gc_step(struct nf_flowtable *flow_table) { struct flow_offload_tuple_rhash *tuplehash; - struct nf_flowtable *flow_table; struct rhashtable_iter hti; struct flow_offload *flow; int err; - flow_table = container_of(work, struct nf_flowtable, gc_work.work); - err = rhashtable_walk_init(&flow_table->rhashtable, &hti, GFP_KERNEL); if (err) - goto schedule; + return 0; rhashtable_walk_start(&hti); @@ -270,7 +267,16 @@ void nf_flow_offload_work_gc(struct work_struct *work) out: rhashtable_walk_stop(&hti); rhashtable_walk_exit(&hti); -schedule: + + return 1; +} + +void nf_flow_offload_work_gc(struct work_struct *work) +{ + struct nf_flowtable *flow_table; + + flow_table = container_of(work, struct nf_flowtable, gc_work.work); + nf_flow_offload_gc_step(flow_table); queue_delayed_work(system_power_efficient_wq, &flow_table->gc_work, HZ); } EXPORT_SYMBOL_GPL(nf_flow_offload_work_gc); @@ -449,5 +455,12 @@ void nf_flow_table_cleanup(struct net *net, struct net_device *dev) } EXPORT_SYMBOL_GPL(nf_flow_table_cleanup); +void nf_flow_table_free(struct nf_flowtable *flow_table) +{ + nf_flow_table_iterate(flow_table, nf_flow_table_do_cleanup, NULL); + WARN_ON(!nf_flow_offload_gc_step(flow_table)); +} +EXPORT_SYMBOL_GPL(nf_flow_table_free); + MODULE_LICENSE("GPL"); MODULE_AUTHOR("Pablo Neira Ayuso "); diff --git a/net/netfilter/nf_flow_table_inet.c b/net/netfilter/nf_flow_table_inet.c index 281209aeba8f..375a1881d93d 100644 --- a/net/netfilter/nf_flow_table_inet.c +++ b/net/netfilter/nf_flow_table_inet.c @@ -24,6 +24,7 @@ static struct nf_flowtable_type flowtable_inet = { .family = NFPROTO_INET, .params = &nf_flow_offload_rhash_params, .gc = nf_flow_offload_work_gc, + .free = nf_flow_table_free, .hook = nf_flow_offload_inet_hook, .owner = THIS_MODULE, }; diff --git a/net/netfilter/nf_tables_api.c b/net/netfilter/nf_tables_api.c index 07dd1fac78a8..8b9fe30de0cd 100644 --- a/net/netfilter/nf_tables_api.c +++ b/net/netfilter/nf_tables_api.c @@ -5399,17 +5399,12 @@ err: nfnetlink_set_err(ctx->net, ctx->portid, NFNLGRP_NFTABLES, -ENOBUFS); } -static void nft_flowtable_destroy(void *ptr, void *arg) -{ - kfree(ptr); -} - static void nf_tables_flowtable_destroy(struct nft_flowtable *flowtable) { cancel_delayed_work_sync(&flowtable->data.gc_work); kfree(flowtable->name); - rhashtable_free_and_destroy(&flowtable->data.rhashtable, - nft_flowtable_destroy, NULL); + flowtable->data.type->free(&flowtable->data); + rhashtable_destroy(&flowtable->data.rhashtable); module_put(flowtable->data.type->owner); } -- cgit v1.2.3-59-g8ed1b From d8ed9600581d40d818ae417b3086a333841b0559 Mon Sep 17 00:00:00 2001 From: Taehee Yoo Date: Wed, 7 Feb 2018 11:50:41 +0900 Subject: netfilter: remove useless prototype prototype nf_ct_nat_offset is not used anymore. Signed-off-by: Taehee Yoo --- include/net/netfilter/nf_conntrack.h | 5 ----- 1 file changed, 5 deletions(-) (limited to 'include') diff --git a/include/net/netfilter/nf_conntrack.h b/include/net/netfilter/nf_conntrack.h index f5223bf2c420..062dc19b5840 100644 --- a/include/net/netfilter/nf_conntrack.h +++ b/include/net/netfilter/nf_conntrack.h @@ -213,11 +213,6 @@ static inline bool nf_ct_kill(struct nf_conn *ct) return nf_ct_delete(ct, 0, 0); } -/* These are for NAT. Icky. */ -extern s32 (*nf_ct_nat_offset)(const struct nf_conn *ct, - enum ip_conntrack_dir dir, - u32 seq); - /* Set all unconfirmed conntrack as dying */ void nf_ct_unconfirmed_destroy(struct net *); -- cgit v1.2.3-59-g8ed1b From 0ff90b6c20340e57616a51ae1a1bf18156d6638a Mon Sep 17 00:00:00 2001 From: Felix Fietkau Date: Wed, 7 Feb 2018 09:49:02 +0100 Subject: netfilter: nf_flow_offload: fix use-after-free and a resource leak flow_offload_del frees the flow, so all associated resource must be freed before. Since the ct entry in struct flow_offload_entry was allocated by flow_offload_alloc, it should be freed by flow_offload_free to take care of the error handling path when flow_offload_add fails. While at it, make flow_offload_del static, since it should never be called directly, only from the gc step Signed-off-by: Felix Fietkau Signed-off-by: Pablo Neira Ayuso --- include/net/netfilter/nf_flow_table.h | 1 - net/netfilter/nf_flow_table.c | 27 +++++++-------------------- 2 files changed, 7 insertions(+), 21 deletions(-) (limited to 'include') diff --git a/include/net/netfilter/nf_flow_table.h b/include/net/netfilter/nf_flow_table.h index 020ae903066f..833752dd0c58 100644 --- a/include/net/netfilter/nf_flow_table.h +++ b/include/net/netfilter/nf_flow_table.h @@ -90,7 +90,6 @@ struct flow_offload *flow_offload_alloc(struct nf_conn *ct, void flow_offload_free(struct flow_offload *flow); int flow_offload_add(struct nf_flowtable *flow_table, struct flow_offload *flow); -void flow_offload_del(struct nf_flowtable *flow_table, struct flow_offload *flow); struct flow_offload_tuple_rhash *flow_offload_lookup(struct nf_flowtable *flow_table, struct flow_offload_tuple *tuple); int nf_flow_table_iterate(struct nf_flowtable *flow_table, diff --git a/net/netfilter/nf_flow_table.c b/net/netfilter/nf_flow_table.c index c17f1af42daa..ec410cae9307 100644 --- a/net/netfilter/nf_flow_table.c +++ b/net/netfilter/nf_flow_table.c @@ -125,7 +125,9 @@ void flow_offload_free(struct flow_offload *flow) dst_release(flow->tuplehash[FLOW_OFFLOAD_DIR_ORIGINAL].tuple.dst_cache); dst_release(flow->tuplehash[FLOW_OFFLOAD_DIR_REPLY].tuple.dst_cache); e = container_of(flow, struct flow_offload_entry, flow); - kfree(e); + nf_ct_delete(e->ct, 0, 0); + nf_ct_put(e->ct); + kfree_rcu(e, rcu_head); } EXPORT_SYMBOL_GPL(flow_offload_free); @@ -149,11 +151,9 @@ int flow_offload_add(struct nf_flowtable *flow_table, struct flow_offload *flow) } EXPORT_SYMBOL_GPL(flow_offload_add); -void flow_offload_del(struct nf_flowtable *flow_table, - struct flow_offload *flow) +static void flow_offload_del(struct nf_flowtable *flow_table, + struct flow_offload *flow) { - struct flow_offload_entry *e; - rhashtable_remove_fast(&flow_table->rhashtable, &flow->tuplehash[FLOW_OFFLOAD_DIR_ORIGINAL].node, *flow_table->type->params); @@ -161,10 +161,8 @@ void flow_offload_del(struct nf_flowtable *flow_table, &flow->tuplehash[FLOW_OFFLOAD_DIR_REPLY].node, *flow_table->type->params); - e = container_of(flow, struct flow_offload_entry, flow); - kfree_rcu(e, rcu_head); + flow_offload_free(flow); } -EXPORT_SYMBOL_GPL(flow_offload_del); struct flow_offload_tuple_rhash * flow_offload_lookup(struct nf_flowtable *flow_table, @@ -175,15 +173,6 @@ flow_offload_lookup(struct nf_flowtable *flow_table, } EXPORT_SYMBOL_GPL(flow_offload_lookup); -static void nf_flow_release_ct(const struct flow_offload *flow) -{ - struct flow_offload_entry *e; - - e = container_of(flow, struct flow_offload_entry, flow); - nf_ct_delete(e->ct, 0, 0); - nf_ct_put(e->ct); -} - int nf_flow_table_iterate(struct nf_flowtable *flow_table, void (*iter)(struct flow_offload *flow, void *data), void *data) @@ -259,10 +248,8 @@ static int nf_flow_offload_gc_step(struct nf_flowtable *flow_table) flow = container_of(tuplehash, struct flow_offload, tuplehash[0]); if (nf_flow_has_expired(flow) || - nf_flow_is_dying(flow)) { + nf_flow_is_dying(flow)) flow_offload_del(flow_table, flow); - nf_flow_release_ct(flow); - } } out: rhashtable_walk_stop(&hti); -- cgit v1.2.3-59-g8ed1b