* [PATCH] netfilter: fix mangle tables back @ 2010-02-11 16:12 Alexey Dobriyan 2010-02-11 16:25 ` Jan Engelhardt 0 siblings, 1 reply; 9+ messages in thread From: Alexey Dobriyan @ 2010-02-11 16:12 UTC (permalink / raw) To: kaber, jengelh; +Cc: netfilter-devel 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 */ ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH] netfilter: fix mangle tables back 2010-02-11 16:12 [PATCH] netfilter: fix mangle tables back Alexey Dobriyan @ 2010-02-11 16:25 ` Jan Engelhardt 2010-02-11 16:34 ` Patrick McHardy 0 siblings, 1 reply; 9+ messages in thread From: Jan Engelhardt @ 2010-02-11 16:25 UTC (permalink / raw) To: Alexey Dobriyan; +Cc: kaber, netfilter-devel 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 */ > ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH] netfilter: fix mangle tables back 2010-02-11 16:25 ` Jan Engelhardt @ 2010-02-11 16:34 ` Patrick McHardy 2010-02-11 17:07 ` Jan Engelhardt 2010-02-11 17:15 ` Alexey Dobriyan 0 siblings, 2 replies; 9+ messages in thread From: Patrick McHardy @ 2010-02-11 16:34 UTC (permalink / raw) To: Jan Engelhardt; +Cc: Alexey Dobriyan, netfilter-devel 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 ... ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH] netfilter: fix mangle tables back 2010-02-11 16:34 ` Patrick McHardy @ 2010-02-11 17:07 ` Jan Engelhardt 2010-02-11 17:13 ` Patrick McHardy 2010-02-11 17:15 ` Alexey Dobriyan 1 sibling, 1 reply; 9+ messages in thread From: Jan Engelhardt @ 2010-02-11 17:07 UTC (permalink / raw) To: Patrick McHardy; +Cc: Alexey Dobriyan, netfilter-devel 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. ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH] netfilter: fix mangle tables back 2010-02-11 17:07 ` Jan Engelhardt @ 2010-02-11 17:13 ` Patrick McHardy 0 siblings, 0 replies; 9+ messages in thread From: Patrick McHardy @ 2010-02-11 17:13 UTC (permalink / raw) To: Jan Engelhardt; +Cc: Alexey Dobriyan, netfilter-devel 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. ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH] netfilter: fix mangle tables back 2010-02-11 16:34 ` Patrick McHardy 2010-02-11 17:07 ` Jan Engelhardt @ 2010-02-11 17:15 ` Alexey Dobriyan 2010-02-11 17:24 ` Patrick McHardy 1 sibling, 1 reply; 9+ messages in thread From: Alexey Dobriyan @ 2010-02-11 17:15 UTC (permalink / raw) To: Patrick McHardy; +Cc: Jan Engelhardt, netfilter-devel 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); ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH] netfilter: fix mangle tables back 2010-02-11 17:15 ` Alexey Dobriyan @ 2010-02-11 17:24 ` Patrick McHardy 2010-02-11 17:27 ` Alexey Dobriyan 0 siblings, 1 reply; 9+ messages in thread From: Patrick McHardy @ 2010-02-11 17:24 UTC (permalink / raw) To: Alexey Dobriyan; +Cc: Jan Engelhardt, netfilter-devel 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. ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH] netfilter: fix mangle tables back 2010-02-11 17:24 ` Patrick McHardy @ 2010-02-11 17:27 ` Alexey Dobriyan 2010-02-11 17:42 ` Patrick McHardy 0 siblings, 1 reply; 9+ messages in thread From: Alexey Dobriyan @ 2010-02-11 17:27 UTC (permalink / raw) To: Patrick McHardy; +Cc: Jan Engelhardt, netfilter-devel 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! ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH] netfilter: fix mangle tables back 2010-02-11 17:27 ` Alexey Dobriyan @ 2010-02-11 17:42 ` Patrick McHardy 0 siblings, 0 replies; 9+ messages in thread From: Patrick McHardy @ 2010-02-11 17:42 UTC (permalink / raw) To: Alexey Dobriyan; +Cc: Jan Engelhardt, netfilter-devel 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. ^ permalink raw reply [flat|nested] 9+ messages in thread
end of thread, other threads:[~2010-02-11 17:42 UTC | newest] Thread overview: 9+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2010-02-11 16:12 [PATCH] netfilter: fix mangle tables back Alexey Dobriyan 2010-02-11 16:25 ` Jan Engelhardt 2010-02-11 16:34 ` Patrick McHardy 2010-02-11 17:07 ` Jan Engelhardt 2010-02-11 17:13 ` Patrick McHardy 2010-02-11 17:15 ` Alexey Dobriyan 2010-02-11 17:24 ` Patrick McHardy 2010-02-11 17:27 ` Alexey Dobriyan 2010-02-11 17:42 ` Patrick McHardy
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).