[PATCH] netfilter: fix mangle tables back

[PATCH] netfilter: fix mangle tables back

[Alexey Dobriyan]

Calling POST_ROUTING hook with NULL input device is not going to work.

Signed-off-by: Alexey Dobriyan <adobriyan@gmail.com>
---

 net/ipv4/netfilter/iptable_mangle.c  |    2 +-
 net/ipv6/netfilter/ip6table_mangle.c |    2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

--- a/net/ipv4/netfilter/iptable_mangle.c
+++ b/net/ipv4/netfilter/iptable_mangle.c
@@ -85,7 +85,7 @@ iptable_mangle_hook(unsigned int hook,
 		     const struct net_device *out,
 		     int (*okfn)(struct sk_buff *))
 {
-	if (hook == NF_INET_LOCAL_OUT)
+	if (hook == NF_INET_LOCAL_OUT || hook == NF_INET_POST_ROUTING)
 		return ipt_local_hook(hook, skb, in, out, okfn);
 
 	/* PREROUTING/INPUT/FORWARD: */
--- a/net/ipv6/netfilter/ip6table_mangle.c
+++ b/net/ipv6/netfilter/ip6table_mangle.c
@@ -79,7 +79,7 @@ ip6table_mangle_hook(unsigned int hook, struct sk_buff *skb,
 		     const struct net_device *in, const struct net_device *out,
 		     int (*okfn)(struct sk_buff *))
 {
-	if (hook == NF_INET_LOCAL_OUT)
+	if (hook == NF_INET_LOCAL_OUT || hook == NF_INET_POST_ROUTING)
 		return ip6t_local_out_hook(hook, skb, out, okfn);
 
 	/* INPUT/FORWARD */

Re: [PATCH] netfilter: fix mangle tables back

[Jan Engelhardt]

On Thursday 2010-02-11 17:12, Alexey Dobriyan wrote:

>Calling POST_ROUTING hook with NULL input device is not going to work.
>
>--- a/net/ipv4/netfilter/iptable_mangle.c
>+++ b/net/ipv4/netfilter/iptable_mangle.c
>@@ -85,7 +85,7 @@ iptable_mangle_hook(unsigned int hook,
> 		     const struct net_device *out,
> 		     int (*okfn)(struct sk_buff *))
> {
>-	if (hook == NF_INET_LOCAL_OUT)
>+	if (hook == NF_INET_LOCAL_OUT || hook == NF_INET_POST_ROUTING)
> 		return ipt_local_hook(hook, skb, in, out, okfn);
> 
> 	/* PREROUTING/INPUT/FORWARD: */

postrouting did not call ipt_local_hook before, so why now?


>--- a/net/ipv6/netfilter/ip6table_mangle.c
>+++ b/net/ipv6/netfilter/ip6table_mangle.c
>@@ -79,7 +79,7 @@ ip6table_mangle_hook(unsigned int hook, struct sk_buff *skb,
> 		     const struct net_device *in, const struct net_device *out,
> 		     int (*okfn)(struct sk_buff *))
> {
>-	if (hook == NF_INET_LOCAL_OUT)
>+	if (hook == NF_INET_LOCAL_OUT || hook == NF_INET_POST_ROUTING)
> 		return ip6t_local_out_hook(hook, skb, out, okfn);
> 
> 	/* INPUT/FORWARD */
>

Re: [PATCH] netfilter: fix mangle tables back

[Patrick McHardy]

Jan Engelhardt wrote:
> On Thursday 2010-02-11 17:12, Alexey Dobriyan wrote:
> 
>> Calling POST_ROUTING hook with NULL input device is not going to work.
>>
>> --- a/net/ipv4/netfilter/iptable_mangle.c
>> +++ b/net/ipv4/netfilter/iptable_mangle.c
>> @@ -85,7 +85,7 @@ iptable_mangle_hook(unsigned int hook,
>> 		     const struct net_device *out,
>> 		     int (*okfn)(struct sk_buff *))
>> {
>> -	if (hook == NF_INET_LOCAL_OUT)
>> +	if (hook == NF_INET_LOCAL_OUT || hook == NF_INET_POST_ROUTING)
>> 		return ipt_local_hook(hook, skb, in, out, okfn);
>>
>> 	/* PREROUTING/INPUT/FORWARD: */
> 
> postrouting did not call ipt_local_hook before, so why now?

What Alexey meant is that

	/* PREROUTING/INPUT/FORWARD: */
	return ipt_do_table(skb, hook, in, out,
			    dev_net(in)->ipv4.iptable_mangle);

dev_net(in) for a NULL device won't work. Passing them to the local
hook won't work either however since we perform rerouting there.
I'm confused now why this didn't crash here so far ...

Re: [PATCH] netfilter: fix mangle tables back

[Jan Engelhardt]

On Thursday 2010-02-11 17:34, Patrick McHardy wrote:
>Jan Engelhardt wrote:
>> On Thursday 2010-02-11 17:12, Alexey Dobriyan wrote:
>> 
>>> Calling POST_ROUTING hook with NULL input device is not going to work.
>>>
>>> --- a/net/ipv4/netfilter/iptable_mangle.c
>>> +++ b/net/ipv4/netfilter/iptable_mangle.c
>>> @@ -85,7 +85,7 @@ iptable_mangle_hook(unsigned int hook,
>>> 		     const struct net_device *out,
>>> 		     int (*okfn)(struct sk_buff *))
>>> {
>>> -	if (hook == NF_INET_LOCAL_OUT)
>>> +	if (hook == NF_INET_LOCAL_OUT || hook == NF_INET_POST_ROUTING)
>>> 		return ipt_local_hook(hook, skb, in, out, okfn);
>>>
>>> 	/* PREROUTING/INPUT/FORWARD: */
>> 
>> postrouting did not call ipt_local_hook before, so why now?
>
>What Alexey meant is that
>
>	/* PREROUTING/INPUT/FORWARD: */
>	return ipt_do_table(skb, hook, in, out,
>			    dev_net(in)->ipv4.iptable_mangle);
>
>dev_net(in) for a NULL device won't work. Passing them to the local
>hook won't work either however since we perform rerouting there.
>I'm confused now why this didn't crash here so far ...

Before, ipt_post_routing_hook just called

	return ipt_do_table(skb, hook, in, out,
		dev_net(out)->ipv4.iptable_mangle);

Not caring about whether in and out are NULL or not,
because ipt_do_table checks for NULL.
Now, iptable_mangle_hook still just calls ipt_do_table, so
nothing really changed, thus nothing broke.

Thus the confusion this patch introduces.

Re: [PATCH] netfilter: fix mangle tables back

[Patrick McHardy]

Jan Engelhardt wrote:
> On Thursday 2010-02-11 17:34, Patrick McHardy wrote:
>> Jan Engelhardt wrote:
>>> On Thursday 2010-02-11 17:12, Alexey Dobriyan wrote:
>>>
>>>> Calling POST_ROUTING hook with NULL input device is not going to work.
>>>>
>>>> --- a/net/ipv4/netfilter/iptable_mangle.c
>>>> +++ b/net/ipv4/netfilter/iptable_mangle.c
>>>> @@ -85,7 +85,7 @@ iptable_mangle_hook(unsigned int hook,
>>>> 		     const struct net_device *out,
>>>> 		     int (*okfn)(struct sk_buff *))
>>>> {
>>>> -	if (hook == NF_INET_LOCAL_OUT)
>>>> +	if (hook == NF_INET_LOCAL_OUT || hook == NF_INET_POST_ROUTING)
>>>> 		return ipt_local_hook(hook, skb, in, out, okfn);
>>>>
>>>> 	/* PREROUTING/INPUT/FORWARD: */
>>> postrouting did not call ipt_local_hook before, so why now?
>> What Alexey meant is that
>>
>> 	/* PREROUTING/INPUT/FORWARD: */
>> 	return ipt_do_table(skb, hook, in, out,
>> 			    dev_net(in)->ipv4.iptable_mangle);
>>
>> dev_net(in) for a NULL device won't work. Passing them to the local
>> hook won't work either however since we perform rerouting there.
>> I'm confused now why this didn't crash here so far ...
> 
> Before, ipt_post_routing_hook just called
> 
> 	return ipt_do_table(skb, hook, in, out,
> 		dev_net(out)->ipv4.iptable_mangle);
> 
> Not caring about whether in and out are NULL or not,
> because ipt_do_table checks for NULL.

Its using the device to get to the table:

dev_net(in)->ipv4.iptable_mangle

The only reason why it doesn't crash for me is that the
machine I'm testing this on doesn't use network namespaces.

Please fix this up.

Re: [PATCH] netfilter: fix mangle tables back

[Alexey Dobriyan]

On Thu, Feb 11, 2010 at 05:34:30PM +0100, Patrick McHardy wrote:
> Jan Engelhardt wrote:
> > On Thursday 2010-02-11 17:12, Alexey Dobriyan wrote:
> > 
> >> Calling POST_ROUTING hook with NULL input device is not going to work.
> >>
> >> --- a/net/ipv4/netfilter/iptable_mangle.c
> >> +++ b/net/ipv4/netfilter/iptable_mangle.c
> >> @@ -85,7 +85,7 @@ iptable_mangle_hook(unsigned int hook,
> >> 		     const struct net_device *out,
> >> 		     int (*okfn)(struct sk_buff *))
> >> {
> >> -	if (hook == NF_INET_LOCAL_OUT)
> >> +	if (hook == NF_INET_LOCAL_OUT || hook == NF_INET_POST_ROUTING)
> >> 		return ipt_local_hook(hook, skb, in, out, okfn);
> >>
> >> 	/* PREROUTING/INPUT/FORWARD: */
> > 
> > postrouting did not call ipt_local_hook before, so why now?
> 
> What Alexey meant is that
> 
> 	/* PREROUTING/INPUT/FORWARD: */
> 	return ipt_do_table(skb, hook, in, out,
> 			    dev_net(in)->ipv4.iptable_mangle);
> 
> dev_net(in) for a NULL device won't work. Passing them to the local
> hook won't work either however since we perform rerouting there.
> I'm confused now why this didn't crash here so far ...

It did crashed, that's why I noticed it.
But now I can't reproduce it too. Hopefully this patch is correct.


[PATCH] netfilter: fix mangle tables

In POST_ROUTING hook, calling dev_net(in) is going to oops.

Signed-off-by: Alexey Dobriyan <adobriyan@gmail.com>
---

 net/ipv4/netfilter/iptable_mangle.c  |    4 +++-
 net/ipv6/netfilter/ip6table_mangle.c |    4 +++-
 2 files changed, 6 insertions(+), 2 deletions(-)

--- a/net/ipv4/netfilter/iptable_mangle.c
+++ b/net/ipv4/netfilter/iptable_mangle.c
@@ -87,7 +87,9 @@ iptable_mangle_hook(unsigned int hook,
 {
 	if (hook == NF_INET_LOCAL_OUT)
 		return ipt_local_hook(hook, skb, in, out, okfn);
-
+	if (hook == NF_INET_POST_ROUTING)
+		return ipt_do_table(skb, hook, in, out,
+				    dev_net(out)->ipv4.iptable_mangle);
 	/* PREROUTING/INPUT/FORWARD: */
 	return ipt_do_table(skb, hook, in, out,
 			    dev_net(in)->ipv4.iptable_mangle);
--- a/net/ipv6/netfilter/ip6table_mangle.c
+++ b/net/ipv6/netfilter/ip6table_mangle.c
@@ -81,7 +81,9 @@ ip6table_mangle_hook(unsigned int hook, struct sk_buff *skb,
 {
 	if (hook == NF_INET_LOCAL_OUT)
 		return ip6t_local_out_hook(hook, skb, out, okfn);
-
+	if (hook == NF_INET_POST_ROUTING)
+		return ip6t_do_table(skb, hook, in, out,
+				     dev_net(out)->ipv6.ip6table_mangle);
 	/* INPUT/FORWARD */
 	return ip6t_do_table(skb, hook, in, out,
 			     dev_net(in)->ipv6.ip6table_mangle);

Re: [PATCH] netfilter: fix mangle tables back

[Patrick McHardy]

Alexey Dobriyan wrote:
> On Thu, Feb 11, 2010 at 05:34:30PM +0100, Patrick McHardy wrote:
>> Jan Engelhardt wrote:
>>> On Thursday 2010-02-11 17:12, Alexey Dobriyan wrote:
>>>
>>>> Calling POST_ROUTING hook with NULL input device is not going to work.
>>>>
>>>> --- a/net/ipv4/netfilter/iptable_mangle.c
>>>> +++ b/net/ipv4/netfilter/iptable_mangle.c
>>>> @@ -85,7 +85,7 @@ iptable_mangle_hook(unsigned int hook,
>>>> 		     const struct net_device *out,
>>>> 		     int (*okfn)(struct sk_buff *))
>>>> {
>>>> -	if (hook == NF_INET_LOCAL_OUT)
>>>> +	if (hook == NF_INET_LOCAL_OUT || hook == NF_INET_POST_ROUTING)
>>>> 		return ipt_local_hook(hook, skb, in, out, okfn);
>>>>
>>>> 	/* PREROUTING/INPUT/FORWARD: */
>>> postrouting did not call ipt_local_hook before, so why now?
>> What Alexey meant is that
>>
>> 	/* PREROUTING/INPUT/FORWARD: */
>> 	return ipt_do_table(skb, hook, in, out,
>> 			    dev_net(in)->ipv4.iptable_mangle);
>>
>> dev_net(in) for a NULL device won't work. Passing them to the local
>> hook won't work either however since we perform rerouting there.
>> I'm confused now why this didn't crash here so far ...
> 
> It did crashed, that's why I noticed it.
> But now I can't reproduce it too. Hopefully this patch is correct.

It looks correct to me. Will try to reproduce the crash
just to make sure.

Re: [PATCH] netfilter: fix mangle tables back

[Alexey Dobriyan]

On Thu, Feb 11, 2010 at 06:24:48PM +0100, Patrick McHardy wrote:
> It looks correct to me. Will try to reproduce the crash
> just to make sure.

Just CONFIG_NET_NS=y and it will crash.
Arrgh!

Re: [PATCH] netfilter: fix mangle tables back

[Patrick McHardy]

Alexey Dobriyan wrote:
> On Thu, Feb 11, 2010 at 06:24:48PM +0100, Patrick McHardy wrote:
>> It looks correct to me. Will try to reproduce the crash
>> just to make sure.
> 
> Just CONFIG_NET_NS=y and it will crash.

Indeed. Patch applied, thanks Alexey.