From 4610476d89d53714ca94aae081fa035908bc137a Mon Sep 17 00:00:00 2001 From: Pablo Neira Ayuso Date: Thu, 10 Jan 2013 12:42:15 +0100 Subject: [PATCH 1/3] netfilter: xt_CT: fix unset return value if conntrack zone are disabled MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit net/netfilter/xt_CT.c: In function ‘xt_ct_tg_check_v1’: net/netfilter/xt_CT.c:250:6: warning: ‘ret’ may be used uninitialized in this function [-Wmaybe-uninitialized] net/netfilter/xt_CT.c: In function ‘xt_ct_tg_check_v0’: net/netfilter/xt_CT.c:112:6: warning: ‘ret’ may be used uninitialized in this function [-Wmaybe-uninitialized] Reported-by: Borislav Petkov Acked-by: Borislav Petkov Signed-off-by: Pablo Neira Ayuso --- net/netfilter/xt_CT.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/net/netfilter/xt_CT.c b/net/netfilter/xt_CT.c index 2a0843081840..bde009ed8d3b 100644 --- a/net/netfilter/xt_CT.c +++ b/net/netfilter/xt_CT.c @@ -109,7 +109,7 @@ static int xt_ct_tg_check_v0(const struct xt_tgchk_param *par) struct xt_ct_target_info *info = par->targinfo; struct nf_conntrack_tuple t; struct nf_conn *ct; - int ret; + int ret = -EOPNOTSUPP; if (info->flags & ~XT_CT_NOTRACK) return -EINVAL; @@ -247,7 +247,7 @@ static int xt_ct_tg_check_v1(const struct xt_tgchk_param *par) struct xt_ct_target_info_v1 *info = par->targinfo; struct nf_conntrack_tuple t; struct nf_conn *ct; - int ret; + int ret = -EOPNOTSUPP; if (info->flags & ~XT_CT_NOTRACK) return -EINVAL; From 1e47ee8367babe6a5e8adf44a714c7086657b87e Mon Sep 17 00:00:00 2001 From: Pablo Neira Ayuso Date: Thu, 10 Jan 2013 16:12:01 +0100 Subject: [PATCH 2/3] netfilter: nf_conntrack: fix BUG_ON while removing nf_conntrack with netns canqun zhang reported that we're hitting BUG_ON in the nf_conntrack_destroy path when calling kfree_skb while rmmod'ing the nf_conntrack module. Currently, the nf_ct_destroy hook is being set to NULL in the destroy path of conntrack.init_net. However, this is a problem since init_net may be destroyed before any other existing netns (we cannot assume any specific ordering while releasing existing netns according to what I read in recent emails). Thanks to Gao feng for initial patch to address this issue. Reported-by: canqun zhang Acked-by: Gao feng Signed-off-by: Pablo Neira Ayuso --- include/net/netfilter/nf_conntrack_core.h | 2 ++ net/netfilter/nf_conntrack_core.c | 9 +++++---- net/netfilter/nf_conntrack_standalone.c | 1 + 3 files changed, 8 insertions(+), 4 deletions(-) diff --git a/include/net/netfilter/nf_conntrack_core.h b/include/net/netfilter/nf_conntrack_core.h index d8f5b9f52169..e98aeb3da033 100644 --- a/include/net/netfilter/nf_conntrack_core.h +++ b/include/net/netfilter/nf_conntrack_core.h @@ -31,6 +31,8 @@ extern void nf_conntrack_cleanup(struct net *net); extern int nf_conntrack_proto_init(struct net *net); extern void nf_conntrack_proto_fini(struct net *net); +extern void nf_conntrack_cleanup_end(void); + extern bool nf_ct_get_tuple(const struct sk_buff *skb, unsigned int nhoff, diff --git a/net/netfilter/nf_conntrack_core.c b/net/netfilter/nf_conntrack_core.c index 016d95ead930..e4a0c4fb3a7c 100644 --- a/net/netfilter/nf_conntrack_core.c +++ b/net/netfilter/nf_conntrack_core.c @@ -1376,11 +1376,12 @@ void nf_conntrack_cleanup(struct net *net) synchronize_net(); nf_conntrack_proto_fini(net); nf_conntrack_cleanup_net(net); +} - if (net_eq(net, &init_net)) { - RCU_INIT_POINTER(nf_ct_destroy, NULL); - nf_conntrack_cleanup_init_net(); - } +void nf_conntrack_cleanup_end(void) +{ + RCU_INIT_POINTER(nf_ct_destroy, NULL); + nf_conntrack_cleanup_init_net(); } void *nf_ct_alloc_hashtable(unsigned int *sizep, int nulls) diff --git a/net/netfilter/nf_conntrack_standalone.c b/net/netfilter/nf_conntrack_standalone.c index 363285d544a1..e7185c684816 100644 --- a/net/netfilter/nf_conntrack_standalone.c +++ b/net/netfilter/nf_conntrack_standalone.c @@ -575,6 +575,7 @@ static int __init nf_conntrack_standalone_init(void) static void __exit nf_conntrack_standalone_fini(void) { unregister_pernet_subsys(&nf_conntrack_net_ops); + nf_conntrack_cleanup_end(); } module_init(nf_conntrack_standalone_init); From 5b76c4948fe6977bead2359c2054f3e6a2dcf3d0 Mon Sep 17 00:00:00 2001 From: Jan Engelhardt Date: Thu, 10 Jan 2013 12:30:05 +0000 Subject: [PATCH 3/3] netfilter: x_tables: print correct hook names for ARP arptables 0.0.4 (released on 10th Jan 2013) supports calling the CLASSIFY target, but on adding a rule to the wrong chain, the diagnostic is as follows: # arptables -A INPUT -j CLASSIFY --set-class 0:0 arptables: Invalid argument # dmesg | tail -n1 x_tables: arp_tables: CLASSIFY target: used from hooks PREROUTING, but only usable from INPUT/FORWARD This is incorrect, since xt_CLASSIFY.c does specify (1 << NF_ARP_OUT) | (1 << NF_ARP_FORWARD). This patch corrects the x_tables diagnostic message to print the proper hook names for the NFPROTO_ARP case. Affects all kernels down to and including v2.6.31. Signed-off-by: Jan Engelhardt Signed-off-by: Pablo Neira Ayuso --- net/netfilter/x_tables.c | 28 ++++++++++++++++++++-------- 1 file changed, 20 insertions(+), 8 deletions(-) diff --git a/net/netfilter/x_tables.c b/net/netfilter/x_tables.c index 8d987c3573fd..7b3a9e5999c0 100644 --- a/net/netfilter/x_tables.c +++ b/net/netfilter/x_tables.c @@ -345,19 +345,27 @@ int xt_find_revision(u8 af, const char *name, u8 revision, int target, } EXPORT_SYMBOL_GPL(xt_find_revision); -static char *textify_hooks(char *buf, size_t size, unsigned int mask) +static char * +textify_hooks(char *buf, size_t size, unsigned int mask, uint8_t nfproto) { - static const char *const names[] = { + static const char *const inetbr_names[] = { "PREROUTING", "INPUT", "FORWARD", "OUTPUT", "POSTROUTING", "BROUTING", }; - unsigned int i; + static const char *const arp_names[] = { + "INPUT", "FORWARD", "OUTPUT", + }; + const char *const *names; + unsigned int i, max; char *p = buf; bool np = false; int res; + names = (nfproto == NFPROTO_ARP) ? arp_names : inetbr_names; + max = (nfproto == NFPROTO_ARP) ? ARRAY_SIZE(arp_names) : + ARRAY_SIZE(inetbr_names); *p = '\0'; - for (i = 0; i < ARRAY_SIZE(names); ++i) { + for (i = 0; i < max; ++i) { if (!(mask & (1 << i))) continue; res = snprintf(p, size, "%s%s", np ? "/" : "", names[i]); @@ -402,8 +410,10 @@ int xt_check_match(struct xt_mtchk_param *par, pr_err("%s_tables: %s match: used from hooks %s, but only " "valid from %s\n", xt_prefix[par->family], par->match->name, - textify_hooks(used, sizeof(used), par->hook_mask), - textify_hooks(allow, sizeof(allow), par->match->hooks)); + textify_hooks(used, sizeof(used), par->hook_mask, + par->family), + textify_hooks(allow, sizeof(allow), par->match->hooks, + par->family)); return -EINVAL; } if (par->match->proto && (par->match->proto != proto || inv_proto)) { @@ -575,8 +585,10 @@ int xt_check_target(struct xt_tgchk_param *par, pr_err("%s_tables: %s target: used from hooks %s, but only " "usable from %s\n", xt_prefix[par->family], par->target->name, - textify_hooks(used, sizeof(used), par->hook_mask), - textify_hooks(allow, sizeof(allow), par->target->hooks)); + textify_hooks(used, sizeof(used), par->hook_mask, + par->family), + textify_hooks(allow, sizeof(allow), par->target->hooks, + par->family)); return -EINVAL; } if (par->target->proto && (par->target->proto != proto || inv_proto)) {