* [PATCH 1/6] netfilter: ebtables: fix wrong name length while copying to user-space
2012-03-06 11:22 [PATCH 0/6] netfilter fixes for 3.3-rc6 pablo
@ 2012-03-06 11:22 ` pablo
2012-03-06 20:15 ` David Miller
2012-03-06 11:22 ` [PATCH 2/6] netfilter: ctnetlink: remove incorrect spin_[un]lock_bh on NAT module autoload pablo
` (4 subsequent siblings)
5 siblings, 1 reply; 18+ messages in thread
From: pablo @ 2012-03-06 11:22 UTC (permalink / raw)
To: netfilter-devel; +Cc: davem, netdev
From: Santosh Nayak <santoshprasadnayak@gmail.com>
user-space ebtables expects 32 bytes-long names, but xt_match names
use 29 bytes. We have to copy less 29 bytes and then, make sure we
fill the remaining bytes with zeroes.
Signed-off-by: Santosh Nayak <santoshprasadnayak@gmail.com>
Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
---
net/bridge/netfilter/ebtables.c | 16 +++++++++++++---
1 files changed, 13 insertions(+), 3 deletions(-)
diff --git a/net/bridge/netfilter/ebtables.c b/net/bridge/netfilter/ebtables.c
index 8aa4ad0..15e9575 100644
--- a/net/bridge/netfilter/ebtables.c
+++ b/net/bridge/netfilter/ebtables.c
@@ -1335,7 +1335,12 @@ static inline int ebt_make_matchname(const struct ebt_entry_match *m,
const char *base, char __user *ubase)
{
char __user *hlp = ubase + ((char *)m - base);
- if (copy_to_user(hlp, m->u.match->name, EBT_FUNCTION_MAXNAMELEN))
+ char name[EBT_FUNCTION_MAXNAMELEN] = {};
+
+ /* ebtables expects 32 bytes long names but xt_match names are 29 bytes
+ long. Copy 29 bytes and fill remaining bytes with zeroes. */
+ strncpy(name, m->u.match->name, sizeof(name));
+ if (copy_to_user(hlp, name, EBT_FUNCTION_MAXNAMELEN))
return -EFAULT;
return 0;
}
@@ -1344,7 +1349,10 @@ static inline int ebt_make_watchername(const struct ebt_entry_watcher *w,
const char *base, char __user *ubase)
{
char __user *hlp = ubase + ((char *)w - base);
- if (copy_to_user(hlp , w->u.watcher->name, EBT_FUNCTION_MAXNAMELEN))
+ char name[EBT_FUNCTION_MAXNAMELEN] = {};
+
+ strncpy(name, w->u.watcher->name, sizeof(name));
+ if (copy_to_user(hlp , name, EBT_FUNCTION_MAXNAMELEN))
return -EFAULT;
return 0;
}
@@ -1355,10 +1363,12 @@ ebt_make_names(struct ebt_entry *e, const char *base, char __user *ubase)
int ret;
char __user *hlp;
const struct ebt_entry_target *t;
+ char name[EBT_FUNCTION_MAXNAMELEN] = {};
if (e->bitmask == 0)
return 0;
+ strncpy(name, t->u.target->name, sizeof(name));
hlp = ubase + (((char *)e + e->target_offset) - base);
t = (struct ebt_entry_target *)(((char *)e) + e->target_offset);
@@ -1368,7 +1378,7 @@ ebt_make_names(struct ebt_entry *e, const char *base, char __user *ubase)
ret = EBT_WATCHER_ITERATE(e, ebt_make_watchername, base, ubase);
if (ret != 0)
return ret;
- if (copy_to_user(hlp, t->u.target->name, EBT_FUNCTION_MAXNAMELEN))
+ if (copy_to_user(hlp, name, EBT_FUNCTION_MAXNAMELEN))
return -EFAULT;
return 0;
}
--
1.7.7.3
^ permalink raw reply related [flat|nested] 18+ messages in thread
* [PATCH 2/6] netfilter: ctnetlink: remove incorrect spin_[un]lock_bh on NAT module autoload
2012-03-06 11:22 [PATCH 0/6] netfilter fixes for 3.3-rc6 pablo
2012-03-06 11:22 ` [PATCH 1/6] netfilter: ebtables: fix wrong name length while copying to user-space pablo
@ 2012-03-06 11:22 ` pablo
2012-03-06 20:15 ` David Miller
2012-03-06 11:22 ` [PATCH 3/6] netfilter: ctnetlink: use GFP_ATOMIC in all allocations pablo
` (3 subsequent siblings)
5 siblings, 1 reply; 18+ messages in thread
From: pablo @ 2012-03-06 11:22 UTC (permalink / raw)
To: netfilter-devel; +Cc: davem, netdev
From: Pablo Neira Ayuso <pablo@netfilter.org>
Since 7d367e0, ctnetlink_new_conntrack is called without holding
the nf_conntrack_lock spinlock. Thus, ctnetlink_parse_nat_setup
does not require to release that spinlock anymore in the NAT module
autoload case.
Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
---
net/netfilter/nf_conntrack_netlink.c | 3 ---
1 files changed, 0 insertions(+), 3 deletions(-)
diff --git a/net/netfilter/nf_conntrack_netlink.c b/net/netfilter/nf_conntrack_netlink.c
index 30c9d4c..1068769 100644
--- a/net/netfilter/nf_conntrack_netlink.c
+++ b/net/netfilter/nf_conntrack_netlink.c
@@ -1041,16 +1041,13 @@ ctnetlink_parse_nat_setup(struct nf_conn *ct,
if (!parse_nat_setup) {
#ifdef CONFIG_MODULES
rcu_read_unlock();
- spin_unlock_bh(&nf_conntrack_lock);
nfnl_unlock();
if (request_module("nf-nat-ipv4") < 0) {
nfnl_lock();
- spin_lock_bh(&nf_conntrack_lock);
rcu_read_lock();
return -EOPNOTSUPP;
}
nfnl_lock();
- spin_lock_bh(&nf_conntrack_lock);
rcu_read_lock();
if (nfnetlink_parse_nat_setup_hook)
return -EAGAIN;
--
1.7.7.3
^ permalink raw reply related [flat|nested] 18+ messages in thread
* [PATCH 3/6] netfilter: ctnetlink: use GFP_ATOMIC in all allocations
2012-03-06 11:22 [PATCH 0/6] netfilter fixes for 3.3-rc6 pablo
2012-03-06 11:22 ` [PATCH 1/6] netfilter: ebtables: fix wrong name length while copying to user-space pablo
2012-03-06 11:22 ` [PATCH 2/6] netfilter: ctnetlink: remove incorrect spin_[un]lock_bh on NAT module autoload pablo
@ 2012-03-06 11:22 ` pablo
2012-03-06 12:50 ` Eric Dumazet
2012-03-06 11:22 ` [PATCH 4/6] netfilter: bridge: fix wrong pointer dereference pablo
` (2 subsequent siblings)
5 siblings, 1 reply; 18+ messages in thread
From: pablo @ 2012-03-06 11:22 UTC (permalink / raw)
To: netfilter-devel; +Cc: davem, netdev
From: Pablo Neira Ayuso <pablo@netfilter.org>
All ctnetlink operations are invoked inside rcu_read_lock
(see net/netfilter/nfnetlink.c).
Allocations have to be atomic, as RCU requires.
Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
---
net/netfilter/nf_conntrack_netlink.c | 4 ++--
1 files changed, 2 insertions(+), 2 deletions(-)
diff --git a/net/netfilter/nf_conntrack_netlink.c b/net/netfilter/nf_conntrack_netlink.c
index 1068769..867843f 100644
--- a/net/netfilter/nf_conntrack_netlink.c
+++ b/net/netfilter/nf_conntrack_netlink.c
@@ -1002,7 +1002,7 @@ ctnetlink_get_conntrack(struct sock *ctnl, struct sk_buff *skb,
ct = nf_ct_tuplehash_to_ctrack(h);
err = -ENOMEM;
- skb2 = nlmsg_new(NLMSG_DEFAULT_SIZE, GFP_KERNEL);
+ skb2 = nlmsg_new(NLMSG_DEFAULT_SIZE, GFP_ATOMIC);
if (skb2 == NULL) {
nf_ct_put(ct);
return -ENOMEM;
@@ -1865,7 +1865,7 @@ ctnetlink_get_expect(struct sock *ctnl, struct sk_buff *skb,
}
err = -ENOMEM;
- skb2 = nlmsg_new(NLMSG_DEFAULT_SIZE, GFP_KERNEL);
+ skb2 = nlmsg_new(NLMSG_DEFAULT_SIZE, GFP_ATOMIC);
if (skb2 == NULL) {
nf_ct_expect_put(exp);
goto out;
--
1.7.7.3
^ permalink raw reply related [flat|nested] 18+ messages in thread
* Re: [PATCH 3/6] netfilter: ctnetlink: use GFP_ATOMIC in all allocations
2012-03-06 11:22 ` [PATCH 3/6] netfilter: ctnetlink: use GFP_ATOMIC in all allocations pablo
@ 2012-03-06 12:50 ` Eric Dumazet
2012-03-06 14:48 ` Pablo Neira Ayuso
0 siblings, 1 reply; 18+ messages in thread
From: Eric Dumazet @ 2012-03-06 12:50 UTC (permalink / raw)
To: pablo; +Cc: netfilter-devel, davem, netdev
On Tue, 2012-03-06 at 12:22 +0100, pablo@netfilter.org wrote:
> From: Pablo Neira Ayuso <pablo@netfilter.org>
>
> All ctnetlink operations are invoked inside rcu_read_lock
> (see net/netfilter/nfnetlink.c).
>
> Allocations have to be atomic, as RCU requires.
>
> Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
> ---
> net/netfilter/nf_conntrack_netlink.c | 4 ++--
> 1 files changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/net/netfilter/nf_conntrack_netlink.c b/net/netfilter/nf_conntrack_netlink.c
> index 1068769..867843f 100644
> --- a/net/netfilter/nf_conntrack_netlink.c
> +++ b/net/netfilter/nf_conntrack_netlink.c
> @@ -1002,7 +1002,7 @@ ctnetlink_get_conntrack(struct sock *ctnl, struct sk_buff *skb,
> ct = nf_ct_tuplehash_to_ctrack(h);
>
> err = -ENOMEM;
> - skb2 = nlmsg_new(NLMSG_DEFAULT_SIZE, GFP_KERNEL);
> + skb2 = nlmsg_new(NLMSG_DEFAULT_SIZE, GFP_ATOMIC);
> if (skb2 == NULL) {
> nf_ct_put(ct);
> return -ENOMEM;
> @@ -1865,7 +1865,7 @@ ctnetlink_get_expect(struct sock *ctnl, struct sk_buff *skb,
> }
>
> err = -ENOMEM;
> - skb2 = nlmsg_new(NLMSG_DEFAULT_SIZE, GFP_KERNEL);
> + skb2 = nlmsg_new(NLMSG_DEFAULT_SIZE, GFP_ATOMIC);
> if (skb2 == NULL) {
> nf_ct_expect_put(exp);
> goto out;
This cant be right.
Really this must be kept as GFP_KERNEL allocations.
Only if .call_rcu member is used in place of .call rcu_read_lock() is
held instead of nfnl_lock().
You should take a look at all GFP_ATOMIC uses in
net/netfilter/nf_conntrack_netlink.c and check if they can be GFP_KERNEL
instead.
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH 3/6] netfilter: ctnetlink: use GFP_ATOMIC in all allocations
2012-03-06 12:50 ` Eric Dumazet
@ 2012-03-06 14:48 ` Pablo Neira Ayuso
2012-03-06 15:09 ` Eric Dumazet
2012-03-06 20:20 ` David Miller
0 siblings, 2 replies; 18+ messages in thread
From: Pablo Neira Ayuso @ 2012-03-06 14:48 UTC (permalink / raw)
To: Eric Dumazet; +Cc: netfilter-devel, davem, netdev
On Tue, Mar 06, 2012 at 04:50:21AM -0800, Eric Dumazet wrote:
> On Tue, 2012-03-06 at 12:22 +0100, pablo@netfilter.org wrote:
> > From: Pablo Neira Ayuso <pablo@netfilter.org>
> >
> > All ctnetlink operations are invoked inside rcu_read_lock
> > (see net/netfilter/nfnetlink.c).
> >
> > Allocations have to be atomic, as RCU requires.
> >
> > Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
> > ---
> > net/netfilter/nf_conntrack_netlink.c | 4 ++--
> > 1 files changed, 2 insertions(+), 2 deletions(-)
> >
> > diff --git a/net/netfilter/nf_conntrack_netlink.c b/net/netfilter/nf_conntrack_netlink.c
> > index 1068769..867843f 100644
> > --- a/net/netfilter/nf_conntrack_netlink.c
> > +++ b/net/netfilter/nf_conntrack_netlink.c
> > @@ -1002,7 +1002,7 @@ ctnetlink_get_conntrack(struct sock *ctnl, struct sk_buff *skb,
> > ct = nf_ct_tuplehash_to_ctrack(h);
> >
> > err = -ENOMEM;
> > - skb2 = nlmsg_new(NLMSG_DEFAULT_SIZE, GFP_KERNEL);
> > + skb2 = nlmsg_new(NLMSG_DEFAULT_SIZE, GFP_ATOMIC);
> > if (skb2 == NULL) {
> > nf_ct_put(ct);
> > return -ENOMEM;
> > @@ -1865,7 +1865,7 @@ ctnetlink_get_expect(struct sock *ctnl, struct sk_buff *skb,
> > }
> >
> > err = -ENOMEM;
> > - skb2 = nlmsg_new(NLMSG_DEFAULT_SIZE, GFP_KERNEL);
> > + skb2 = nlmsg_new(NLMSG_DEFAULT_SIZE, GFP_ATOMIC);
> > if (skb2 == NULL) {
> > nf_ct_expect_put(exp);
> > goto out;
>
> This cant be right.
>
> Really this must be kept as GFP_KERNEL allocations.
>
> Only if .call_rcu member is used in place of .call rcu_read_lock() is
> held instead of nfnl_lock().
I thought we couldn't sleep while holding rcu_read_lock.
> You should take a look at all GFP_ATOMIC uses in
> net/netfilter/nf_conntrack_netlink.c and check if they can be GFP_KERNEL
> instead.
David, can you take all patches except this one?
I'll have to rebase my tree after this, sorry.
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH 3/6] netfilter: ctnetlink: use GFP_ATOMIC in all allocations
2012-03-06 14:48 ` Pablo Neira Ayuso
@ 2012-03-06 15:09 ` Eric Dumazet
2012-03-06 15:35 ` Pablo Neira Ayuso
2012-03-06 20:20 ` David Miller
1 sibling, 1 reply; 18+ messages in thread
From: Eric Dumazet @ 2012-03-06 15:09 UTC (permalink / raw)
To: Pablo Neira Ayuso; +Cc: netfilter-devel, davem, netdev
Le mardi 06 mars 2012 à 15:48 +0100, Pablo Neira Ayuso a écrit :
> On Tue, Mar 06, 2012 at 04:50:21AM -0800, Eric Dumazet wrote:
> > This cant be right.
> >
> > Really this must be kept as GFP_KERNEL allocations.
> >
> > Only if .call_rcu member is used in place of .call rcu_read_lock() is
> > held instead of nfnl_lock().
>
> I thought we couldn't sleep while holding rcu_read_lock.
True, but as far as I can see we dont hold rcu_read_lock() at this
point, only a mutex.
I added the .call_rcu() mechanism in struct nfnl_callback only for very
specific needs, namely performance improvements in commit 84a797dd0
(netfilter: nfnetlink_queue: provide rcu enabled callbacks)
--
To unsubscribe from this list: send the line "unsubscribe netfilter-devel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH 3/6] netfilter: ctnetlink: use GFP_ATOMIC in all allocations
2012-03-06 15:09 ` Eric Dumazet
@ 2012-03-06 15:35 ` Pablo Neira Ayuso
0 siblings, 0 replies; 18+ messages in thread
From: Pablo Neira Ayuso @ 2012-03-06 15:35 UTC (permalink / raw)
To: Eric Dumazet; +Cc: netfilter-devel, davem, netdev
On Tue, Mar 06, 2012 at 07:09:17AM -0800, Eric Dumazet wrote:
> Le mardi 06 mars 2012 à 15:48 +0100, Pablo Neira Ayuso a écrit :
> > On Tue, Mar 06, 2012 at 04:50:21AM -0800, Eric Dumazet wrote:
>
> > > This cant be right.
> > >
> > > Really this must be kept as GFP_KERNEL allocations.
> > >
> > > Only if .call_rcu member is used in place of .call rcu_read_lock() is
> > > held instead of nfnl_lock().
> >
> > I thought we couldn't sleep while holding rcu_read_lock.
>
> True, but as far as I can see we dont hold rcu_read_lock() at this
> point, only a mutex.
>
> I added the .call_rcu() mechanism in struct nfnl_callback only for very
> specific needs, namely performance improvements in commit 84a797dd0
> (netfilter: nfnetlink_queue: provide rcu enabled callbacks)
Sorry, I overlooked that changed, I still thought that we were calling
these under rcu_read_lock.
This patch has to be kept out indeed. Thanks for spotting this Eric.
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH 3/6] netfilter: ctnetlink: use GFP_ATOMIC in all allocations
2012-03-06 14:48 ` Pablo Neira Ayuso
2012-03-06 15:09 ` Eric Dumazet
@ 2012-03-06 20:20 ` David Miller
1 sibling, 0 replies; 18+ messages in thread
From: David Miller @ 2012-03-06 20:20 UTC (permalink / raw)
To: pablo; +Cc: eric.dumazet, netfilter-devel, netdev
From: Pablo Neira Ayuso <pablo@netfilter.org>
Date: Tue, 6 Mar 2012 15:48:38 +0100
> David, can you take all patches except this one?
That's what I did, thanks.
^ permalink raw reply [flat|nested] 18+ messages in thread
* [PATCH 4/6] netfilter: bridge: fix wrong pointer dereference
2012-03-06 11:22 [PATCH 0/6] netfilter fixes for 3.3-rc6 pablo
` (2 preceding siblings ...)
2012-03-06 11:22 ` [PATCH 3/6] netfilter: ctnetlink: use GFP_ATOMIC in all allocations pablo
@ 2012-03-06 11:22 ` pablo
2012-03-06 20:15 ` David Miller
2012-03-06 11:22 ` [PATCH 5/6] bridge: netfilter: don't call iptables on vlan packets if sysctl is off pablo
2012-03-06 11:22 ` [PATCH 6/6] netfilter: nf_conntrack: fix early_drop with reliable event delivery pablo
5 siblings, 1 reply; 18+ messages in thread
From: pablo @ 2012-03-06 11:22 UTC (permalink / raw)
To: netfilter-devel; +Cc: davem, netdev
From: Pablo Neira Ayuso <pablo@netfilter.org>
In adf7ff8, a invalid dereference was added in ebt_make_names.
CC [M] net/bridge/netfilter/ebtables.o
net/bridge/netfilter/ebtables.c: In function `ebt_make_names':
net/bridge/netfilter/ebtables.c:1371:20: warning: `t' may be used uninitialized in this function [-Wuninitialized]
Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
---
net/bridge/netfilter/ebtables.c | 2 +-
1 files changed, 1 insertions(+), 1 deletions(-)
diff --git a/net/bridge/netfilter/ebtables.c b/net/bridge/netfilter/ebtables.c
index 15e9575..5fe2ff3 100644
--- a/net/bridge/netfilter/ebtables.c
+++ b/net/bridge/netfilter/ebtables.c
@@ -1368,7 +1368,6 @@ ebt_make_names(struct ebt_entry *e, const char *base, char __user *ubase)
if (e->bitmask == 0)
return 0;
- strncpy(name, t->u.target->name, sizeof(name));
hlp = ubase + (((char *)e + e->target_offset) - base);
t = (struct ebt_entry_target *)(((char *)e) + e->target_offset);
@@ -1378,6 +1377,7 @@ ebt_make_names(struct ebt_entry *e, const char *base, char __user *ubase)
ret = EBT_WATCHER_ITERATE(e, ebt_make_watchername, base, ubase);
if (ret != 0)
return ret;
+ strncpy(name, t->u.target->name, sizeof(name));
if (copy_to_user(hlp, name, EBT_FUNCTION_MAXNAMELEN))
return -EFAULT;
return 0;
--
1.7.7.3
^ permalink raw reply related [flat|nested] 18+ messages in thread
* Re: [PATCH 4/6] netfilter: bridge: fix wrong pointer dereference
2012-03-06 11:22 ` [PATCH 4/6] netfilter: bridge: fix wrong pointer dereference pablo
@ 2012-03-06 20:15 ` David Miller
0 siblings, 0 replies; 18+ messages in thread
From: David Miller @ 2012-03-06 20:15 UTC (permalink / raw)
To: pablo; +Cc: netfilter-devel, netdev
From: pablo@netfilter.org
Date: Tue, 6 Mar 2012 12:22:53 +0100
> From: Pablo Neira Ayuso <pablo@netfilter.org>
>
> In adf7ff8, a invalid dereference was added in ebt_make_names.
>
> CC [M] net/bridge/netfilter/ebtables.o
> net/bridge/netfilter/ebtables.c: In function `ebt_make_names':
> net/bridge/netfilter/ebtables.c:1371:20: warning: `t' may be used uninitialized in this function [-Wuninitialized]
>
> Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
Applied.
^ permalink raw reply [flat|nested] 18+ messages in thread
* [PATCH 5/6] bridge: netfilter: don't call iptables on vlan packets if sysctl is off
2012-03-06 11:22 [PATCH 0/6] netfilter fixes for 3.3-rc6 pablo
` (3 preceding siblings ...)
2012-03-06 11:22 ` [PATCH 4/6] netfilter: bridge: fix wrong pointer dereference pablo
@ 2012-03-06 11:22 ` pablo
2012-03-06 20:15 ` David Miller
2012-03-06 11:22 ` [PATCH 6/6] netfilter: nf_conntrack: fix early_drop with reliable event delivery pablo
5 siblings, 1 reply; 18+ messages in thread
From: pablo @ 2012-03-06 11:22 UTC (permalink / raw)
To: netfilter-devel; +Cc: davem, netdev
From: Florian Westphal <fw@strlen.de>
When net.bridge.bridge-nf-filter-vlan-tagged is 0 (default), vlan packets
arriving should not be sent to ip(6)tables by bridge netfilter.
However, it turns out that we currently always send VLAN packets to
netfilter, if ..
a), CONFIG_VLAN_8021Q is enabled ; or
b), CONFIG_VLAN_8021Q is not set but rx vlan offload is enabled
on the bridge port.
This is because bridge netfilter treats skb with
skb->protocol == ETH_P_IP{V6} as "non-vlan packet".
With rx vlan offload on or CONFIG_VLAN_8021Q=y, the vlan header has
already been removed here, and we cannot rely on skb->protocol alone.
Fix this by only using skb->protocol if the skb has no vlan tag,
or if a vlan tag is present and filter-vlan-tagged bridge netfilter
sysctl is enabled.
We cannot remove the skb->protocol == htons(ETH_P_8021Q) test
because the vlan tag is still around in the CONFIG_VLAN_8021Q=n &&
"ethtool -K $itf rxvlan off" case.
reproducer:
iptables -t raw -I PREROUTING -i br0
iptables -t raw -I PREROUTING -i br0.1
Then send packets to an ip address configured on br0.1 interface.
Even with net.bridge.bridge-nf-filter-vlan-tagged=0, the 1st rule
will match instead of the 2nd one.
With this patch applied, the 2nd rule will match instead.
In the non-local address case, netfilter won't be consulted after
this patch unless the sysctl is switched on.
Signed-off-by: Florian Westphal <fw@strlen.de>
Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
---
net/bridge/br_netfilter.c | 32 ++++++++++++++++++--------------
1 files changed, 18 insertions(+), 14 deletions(-)
diff --git a/net/bridge/br_netfilter.c b/net/bridge/br_netfilter.c
index 8412247..dec4f38 100644
--- a/net/bridge/br_netfilter.c
+++ b/net/bridge/br_netfilter.c
@@ -62,6 +62,15 @@ static int brnf_filter_pppoe_tagged __read_mostly = 0;
#define brnf_filter_pppoe_tagged 0
#endif
+#define IS_IP(skb) \
+ (!vlan_tx_tag_present(skb) && skb->protocol == htons(ETH_P_IP))
+
+#define IS_IPV6(skb) \
+ (!vlan_tx_tag_present(skb) && skb->protocol == htons(ETH_P_IPV6))
+
+#define IS_ARP(skb) \
+ (!vlan_tx_tag_present(skb) && skb->protocol == htons(ETH_P_ARP))
+
static inline __be16 vlan_proto(const struct sk_buff *skb)
{
if (vlan_tx_tag_present(skb))
@@ -639,8 +648,7 @@ static unsigned int br_nf_pre_routing(unsigned int hook, struct sk_buff *skb,
return NF_DROP;
br = p->br;
- if (skb->protocol == htons(ETH_P_IPV6) || IS_VLAN_IPV6(skb) ||
- IS_PPPOE_IPV6(skb)) {
+ if (IS_IPV6(skb) || IS_VLAN_IPV6(skb) || IS_PPPOE_IPV6(skb)) {
if (!brnf_call_ip6tables && !br->nf_call_ip6tables)
return NF_ACCEPT;
@@ -651,8 +659,7 @@ static unsigned int br_nf_pre_routing(unsigned int hook, struct sk_buff *skb,
if (!brnf_call_iptables && !br->nf_call_iptables)
return NF_ACCEPT;
- if (skb->protocol != htons(ETH_P_IP) && !IS_VLAN_IP(skb) &&
- !IS_PPPOE_IP(skb))
+ if (!IS_IP(skb) && !IS_VLAN_IP(skb) && !IS_PPPOE_IP(skb))
return NF_ACCEPT;
nf_bridge_pull_encap_header_rcsum(skb);
@@ -701,7 +708,7 @@ static int br_nf_forward_finish(struct sk_buff *skb)
struct nf_bridge_info *nf_bridge = skb->nf_bridge;
struct net_device *in;
- if (skb->protocol != htons(ETH_P_ARP) && !IS_VLAN_ARP(skb)) {
+ if (!IS_ARP(skb) && !IS_VLAN_ARP(skb)) {
in = nf_bridge->physindev;
if (nf_bridge->mask & BRNF_PKT_TYPE) {
skb->pkt_type = PACKET_OTHERHOST;
@@ -718,6 +725,7 @@ static int br_nf_forward_finish(struct sk_buff *skb)
return 0;
}
+
/* This is the 'purely bridged' case. For IP, we pass the packet to
* netfilter with indev and outdev set to the bridge device,
* but we are still able to filter on the 'real' indev/outdev
@@ -744,11 +752,9 @@ static unsigned int br_nf_forward_ip(unsigned int hook, struct sk_buff *skb,
if (!parent)
return NF_DROP;
- if (skb->protocol == htons(ETH_P_IP) || IS_VLAN_IP(skb) ||
- IS_PPPOE_IP(skb))
+ if (IS_IP(skb) || IS_VLAN_IP(skb) || IS_PPPOE_IP(skb))
pf = PF_INET;
- else if (skb->protocol == htons(ETH_P_IPV6) || IS_VLAN_IPV6(skb) ||
- IS_PPPOE_IPV6(skb))
+ else if (IS_IPV6(skb) || IS_VLAN_IPV6(skb) || IS_PPPOE_IPV6(skb))
pf = PF_INET6;
else
return NF_ACCEPT;
@@ -795,7 +801,7 @@ static unsigned int br_nf_forward_arp(unsigned int hook, struct sk_buff *skb,
if (!brnf_call_arptables && !br->nf_call_arptables)
return NF_ACCEPT;
- if (skb->protocol != htons(ETH_P_ARP)) {
+ if (!IS_ARP(skb)) {
if (!IS_VLAN_ARP(skb))
return NF_ACCEPT;
nf_bridge_pull_encap_header(skb);
@@ -853,11 +859,9 @@ static unsigned int br_nf_post_routing(unsigned int hook, struct sk_buff *skb,
if (!realoutdev)
return NF_DROP;
- if (skb->protocol == htons(ETH_P_IP) || IS_VLAN_IP(skb) ||
- IS_PPPOE_IP(skb))
+ if (IS_IP(skb) || IS_VLAN_IP(skb) || IS_PPPOE_IP(skb))
pf = PF_INET;
- else if (skb->protocol == htons(ETH_P_IPV6) || IS_VLAN_IPV6(skb) ||
- IS_PPPOE_IPV6(skb))
+ else if (IS_IPV6(skb) || IS_VLAN_IPV6(skb) || IS_PPPOE_IPV6(skb))
pf = PF_INET6;
else
return NF_ACCEPT;
--
1.7.7.3
^ permalink raw reply related [flat|nested] 18+ messages in thread
* [PATCH 6/6] netfilter: nf_conntrack: fix early_drop with reliable event delivery
2012-03-06 11:22 [PATCH 0/6] netfilter fixes for 3.3-rc6 pablo
` (4 preceding siblings ...)
2012-03-06 11:22 ` [PATCH 5/6] bridge: netfilter: don't call iptables on vlan packets if sysctl is off pablo
@ 2012-03-06 11:22 ` pablo
2012-03-06 20:16 ` David Miller
5 siblings, 1 reply; 18+ messages in thread
From: pablo @ 2012-03-06 11:22 UTC (permalink / raw)
To: netfilter-devel; +Cc: davem, netdev
From: Pablo Neira Ayuso <pablo@netfilter.org>
If reliable event delivery is enabled and ctnetlink fails to deliver
the destroy event in early_drop, the conntrack subsystem cannot
drop any the candidate flow that was planned to be evicted.
Reported-by: Kerin Millar <kerframil@gmail.com>
Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
---
net/netfilter/nf_conntrack_core.c | 8 ++++++--
1 files changed, 6 insertions(+), 2 deletions(-)
diff --git a/net/netfilter/nf_conntrack_core.c b/net/netfilter/nf_conntrack_core.c
index ed86a3b..fa4b82c 100644
--- a/net/netfilter/nf_conntrack_core.c
+++ b/net/netfilter/nf_conntrack_core.c
@@ -635,8 +635,12 @@ static noinline int early_drop(struct net *net, unsigned int hash)
if (del_timer(&ct->timeout)) {
death_by_timeout((unsigned long)ct);
- dropped = 1;
- NF_CT_STAT_INC_ATOMIC(net, early_drop);
+ /* Check if we indeed killed this entry. Reliable event
+ delivery may have inserted it into the dying list. */
+ if (test_bit(IPS_DYING_BIT, &ct->status)) {
+ dropped = 1;
+ NF_CT_STAT_INC_ATOMIC(net, early_drop);
+ }
}
nf_ct_put(ct);
return dropped;
--
1.7.7.3
^ permalink raw reply related [flat|nested] 18+ messages in thread
* Re: [PATCH 6/6] netfilter: nf_conntrack: fix early_drop with reliable event delivery
2012-03-06 11:22 ` [PATCH 6/6] netfilter: nf_conntrack: fix early_drop with reliable event delivery pablo
@ 2012-03-06 20:16 ` David Miller
2012-03-07 13:19 ` Pablo Neira Ayuso
0 siblings, 1 reply; 18+ messages in thread
From: David Miller @ 2012-03-06 20:16 UTC (permalink / raw)
To: pablo; +Cc: netfilter-devel, netdev
From: pablo@netfilter.org
Date: Tue, 6 Mar 2012 12:22:55 +0100
> From: Pablo Neira Ayuso <pablo@netfilter.org>
>
> If reliable event delivery is enabled and ctnetlink fails to deliver
> the destroy event in early_drop, the conntrack subsystem cannot
> drop any the candidate flow that was planned to be evicted.
>
> Reported-by: Kerin Millar <kerframil@gmail.com>
> Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
Applied.
> + /* Check if we indeed killed this entry. Reliable event
> + delivery may have inserted it into the dying list. */
> + if (test_bit(IPS_DYING_BIT, &ct->status)) {
Please don't allow misformatted comments like this into your
tree next time, this should be:
/* Check if we indeed killed this entry. Reliable event
* delivery may have inserted it into the dying list.
*/
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH 6/6] netfilter: nf_conntrack: fix early_drop with reliable event delivery
2012-03-06 20:16 ` David Miller
@ 2012-03-07 13:19 ` Pablo Neira Ayuso
0 siblings, 0 replies; 18+ messages in thread
From: Pablo Neira Ayuso @ 2012-03-07 13:19 UTC (permalink / raw)
To: David Miller; +Cc: netfilter-devel, netdev
On Tue, Mar 06, 2012 at 03:16:18PM -0500, David Miller wrote:
> From: pablo@netfilter.org
> Date: Tue, 6 Mar 2012 12:22:55 +0100
>
> > From: Pablo Neira Ayuso <pablo@netfilter.org>
> >
> > If reliable event delivery is enabled and ctnetlink fails to deliver
> > the destroy event in early_drop, the conntrack subsystem cannot
> > drop any the candidate flow that was planned to be evicted.
> >
> > Reported-by: Kerin Millar <kerframil@gmail.com>
> > Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
>
> Applied.
>
> > + /* Check if we indeed killed this entry. Reliable event
> > + delivery may have inserted it into the dying list. */
> > + if (test_bit(IPS_DYING_BIT, &ct->status)) {
>
> Please don't allow misformatted comments like this into your
> tree next time, this should be:
>
> /* Check if we indeed killed this entry. Reliable event
> * delivery may have inserted it into the dying list.
> */
Thanks for spotting this.
Will in the future (probably some patches that I took for net-next
may not follow this format either, but for upcoming patches I will).
^ permalink raw reply [flat|nested] 18+ messages in thread