Re: [PATCH v2 1/1] netfilter: nf_tables: fib: Drop IPV6 packages if IPv6 is disabled on boot
On Thu, 2019-08-29 at 22:58 +0200, Florian Westphal wrote: > Ah, it was the latter. > Making bridge netfilter not pass packets up with ipv6 off closes > the problem for fib_ipv6 and inet, so only _netdev.c needs fixing. Ok then, preparing a v4. https://lkml.org/lkml/2019/8/30/843 signature.asc Description: This is a digitally signed message part
Re: [PATCH v2 1/1] netfilter: nf_tables: fib: Drop IPV6 packages if IPv6 is disabled on boot
Leonardo Bras wrote: > On Thu, 2019-08-29 at 22:58 +0200, Florian Westphal wrote: > [...] > > 1. add a patch to BREAK in nft_fib_netdev.c for !ipv6_mod_enabled() > [...] > > But this is still needed? I mean, in nft_fib_netdev_eval there are only > 2 functions being called for IPv6 protocol : nft_fib6_eval and > nft_fib6_eval_type. Both are already protected by this current patch. > > Is your 1st suggestion about this patch, or you think it's better to > move this change to nft_fib_netdev_eval ? Ah, it was the latter. Making bridge netfilter not pass packets up with ipv6 off closes the problem for fib_ipv6 and inet, so only _netdev.c needs fixing.
Re: [PATCH v2 1/1] netfilter: nf_tables: fib: Drop IPV6 packages if IPv6 is disabled on boot
On Thu, 2019-08-29 at 22:58 +0200, Florian Westphal wrote: [...] > 1. add a patch to BREAK in nft_fib_netdev.c for !ipv6_mod_enabled() [...] But this is still needed? I mean, in nft_fib_netdev_eval there are only 2 functions being called for IPv6 protocol : nft_fib6_eval and nft_fib6_eval_type. Both are already protected by this current patch. Is your 1st suggestion about this patch, or you think it's better to move this change to nft_fib_netdev_eval ? Best regards, Leonardo Bras signature.asc Description: This is a digitally signed message part
Re: [PATCH v2 1/1] netfilter: nf_tables: fib: Drop IPV6 packages if IPv6 is disabled on boot
On Thu, 2019-08-29 at 22:58 +0200, Florian Westphal wrote: > In any case your patch looks ok to me. Great! Please give your feedback on v3: http://patchwork.ozlabs.org/patch/1154040/ [...] > > Even if we disable call-ip6tables in br_netfilter we will at least > in addition need a patch for nft_fib_netdev.c. > > From a "avoid calls to ipv6 stack when its disabled" standpoint, > the safest fix is to disable call-ip6tables functionality if ipv6 > module is off *and* fix nft_fib_netdev.c to BREAK in ipv6 is off case. > > I started to place a list of suspicous modules here, but that got out > of hand quickly. > > So, given I don't want to plaster ipv6_mod_enabled() everywhere, I > would suggest this course of action: > > 1. add a patch to BREAK in nft_fib_netdev.c for !ipv6_mod_enabled() > 2. change net/bridge/br_netfilter_hooks.c, br_nf_pre_routing() to >make sure ipv6_mod_enabled() is true before doing the ipv6 stack >"emulation". > > Makes sense? IMHO sure. Shortly, I will send a couple patches proposing the above changes. (Or my best understanding about them :) ) > > Thanks, > Florian Thank you, Leonardo Bras signature.asc Description: This is a digitally signed message part
Re: [PATCH v2 1/1] netfilter: nf_tables: fib: Drop IPV6 packages if IPv6 is disabled on boot
Leonardo Bras wrote: > On Thu, 2019-08-29 at 17:04 -0300, Leonardo Bras wrote: > > > Thats a good point -- Leonardo, is the > > > "net.bridge.bridge-nf-call-ip6tables" sysctl on? > > > > Running > > # sudo sysctl -a > > I can see: > > net.bridge.bridge-nf-call-ip6tables = 1 > > Also, doing > # echo 0 > /proc/sys/net/bridge/bridge-nf-call-ip6tables > And then trying to boot the guest will not crash the host. > > Which would make sense, since host iptables is not dealing with guest > IPv6 packets. Yes. > So, the real cause of this bug is the bridge making host ip6tables deal > with guest IPv6 packets ? > If so, would it be ok if write a patch testing ipv6_mod_enabled() > before passing guest ipv6 packets to host ip6tables? I'm not sure. This switch is very old, it was added 10 years ago in v2.6.31-rc1. Even if we disable call-ip6tables in br_netfilter we will at least in addition need a patch for nft_fib_netdev.c. >From a "avoid calls to ipv6 stack when its disabled" standpoint, the safest fix is to disable call-ip6tables functionality if ipv6 module is off *and* fix nft_fib_netdev.c to BREAK in ipv6 is off case. I started to place a list of suspicous modules here, but that got out of hand quickly. So, given I don't want to plaster ipv6_mod_enabled() everywhere, I would suggest this course of action: 1. add a patch to BREAK in nft_fib_netdev.c for !ipv6_mod_enabled() 2. change net/bridge/br_netfilter_hooks.c, br_nf_pre_routing() to make sure ipv6_mod_enabled() is true before doing the ipv6 stack "emulation". Makes sense? Thanks, Florian
Re: [PATCH v2 1/1] netfilter: nf_tables: fib: Drop IPV6 packages if IPv6 is disabled on boot
On Thu, 2019-08-29 at 17:04 -0300, Leonardo Bras wrote: > > Thats a good point -- Leonardo, is the > > "net.bridge.bridge-nf-call-ip6tables" sysctl on? > > Running > # sudo sysctl -a > I can see: > net.bridge.bridge-nf-call-ip6tables = 1 Also, doing # echo 0 > /proc/sys/net/bridge/bridge-nf-call-ip6tables And then trying to boot the guest will not crash the host. Which would make sense, since host iptables is not dealing with guest IPv6 packets. So, the real cause of this bug is the bridge making host ip6tables deal with guest IPv6 packets ? If so, would it be ok if write a patch testing ipv6_mod_enabled() before passing guest ipv6 packets to host ip6tables? Best regards, > > So this packets are sent to host iptables for processing? > > > (Sorry for the delay, I did not received the previous e-mails. > Please include me in to/cc.) signature.asc Description: This is a digitally signed message part
Re: [PATCH v2 1/1] netfilter: nf_tables: fib: Drop IPV6 packages if IPv6 is disabled on boot
Leonardo Bras wrote: > > Thats a good point -- Leonardo, is the > > "net.bridge.bridge-nf-call-ip6tables" sysctl on? > > Running > # sudo sysctl -a > I can see: > net.bridge.bridge-nf-call-ip6tables = 1 > > So this packets are sent to host iptables for processing? Yes, this is an hold hack that was made because ebtables is very feature-limited. However, as I mentioned before I don't think there is anything we can do here except audit all affected nft expressions and ip6tables matches and add this check where needed. ip6t_rpfilter.c comes to mind. In any case your patch looks ok to me. > (Sorry for the delay, I did not received the previous e-mails. > Please include me in to/cc.) Sorry about that.
Re: [PATCH v2 1/1] netfilter: nf_tables: fib: Drop IPV6 packages if IPv6 is disabled on boot
> Thats a good point -- Leonardo, is the > "net.bridge.bridge-nf-call-ip6tables" sysctl on? Running # sudo sysctl -a I can see: net.bridge.bridge-nf-call-ip6tables = 1 So this packets are sent to host iptables for processing? (Sorry for the delay, I did not received the previous e-mails. Please include me in to/cc.) signature.asc Description: This is a digitally signed message part
Re: [PATCH v2 1/1] netfilter: nf_tables: fib: Drop IPV6 packages if IPv6 is disabled on boot
From: Leonardo Bras
Date: Tue, 27 Aug 2019 14:34:14 -0300
> I could reproduce this bug on a host ('ipv6.disable=1') starting a
> guest with a virtio-net interface with 'filterref' over a virtual
> bridge. It crashes the host during guest boot (just before login).
>
> By that I could understand that a guest IPv6 network traffic
> (viavirtio-net) may cause this kernel panic.
Really this is bad and I suspected bridging to be involved somehow.
If ipv6 is disabled ipv6 traffic should not pass through the machine
by any means whatsoever. Otherwise there is no point to the knob
and we will keep having to add hack checks all over the tree instead
of fixing the fundamental issue.
Re: [PATCH v2 1/1] netfilter: nf_tables: fib: Drop IPV6 packages if IPv6 is disabled on boot
On Tue, 2019-08-27 at 20:51 +0200, Pablo Neira Ayuso wrote: > > > The drop case at the bottom of the fib eval function never actually > > > never happens. > > > > Which one do you mean? > > Line 31 of net/netfilter/nft_fib_inet.c. Oh, yeah, I was thinking about that when I wrote the patch. Thanks for explaining :) I will send the v3 in a few minutes. Best regards, signature.asc Description: This is a digitally signed message part
Re: [PATCH v2 1/1] netfilter: nf_tables: fib: Drop IPV6 packages if IPv6 is disabled on boot
On Tue, Aug 27, 2019 at 02:34:14PM -0300, Leonardo Bras wrote: > On Tue, 2019-08-27 at 12:35 +0200, Pablo Neira Ayuso wrote: [...] > > NFT_BREAK instead to stop evaluating this rule, this results in a > > mismatch, so you let the user decide what to do with packets that do > > not match your policy. > > Ok, I will replace for v3. Thanks. > > The drop case at the bottom of the fib eval function never actually > > never happens. > > Which one do you mean? Line 31 of net/netfilter/nft_fib_inet.c.
Re: [PATCH v2 1/1] netfilter: nf_tables: fib: Drop IPV6 packages if IPv6 is disabled on boot
On Tue, 2019-08-27 at 12:35 +0200, Pablo Neira Ayuso wrote:
> On Wed, Aug 21, 2019 at 11:15:06AM -0300, Leonardo Bras wrote:
> > If IPv6 is disabled on boot (ipv6.disable=1), but nft_fib_inet ends up
> > dealing with a IPv6 package, it causes a kernel panic in
> > fib6_node_lookup_1(), crashing in bad_page_fault.
>
> Q: How do you get to see IPv6 packets if IPv6 module is disable?
I could reproduce this bug on a host ('ipv6.disable=1') starting a
guest with a virtio-net interface with 'filterref' over a virtual
bridge. It crashes the host during guest boot (just before login).
By that I could understand that a guest IPv6 network traffic (viavirtio-net)
may cause this kernel panic.
>
> > The panic is caused by trying to deference a very low address (0x38
> > in ppc64le), due to ipv6.fib6_main_tbl = NULL.
> > BUG: Kernel NULL pointer dereference at 0x0038
> >
> > Fix this behavior by dropping IPv6 packages if !ipv6_mod_enabled().
>
> I'd suggest: s/package/packet/
Sure, I will make sure to put it on v3.
(Sorry, I am not very used to net subsystem.)
>
> [...]
> > diff --git a/net/ipv6/netfilter/nft_fib_ipv6.c
> > b/net/ipv6/netfilter/nft_fib_ipv6.c
> > index 7ece86afd079..75acc417e2ff 100644
> > --- a/net/ipv6/netfilter/nft_fib_ipv6.c
> > +++ b/net/ipv6/netfilter/nft_fib_ipv6.c
> > @@ -125,6 +125,11 @@ void nft_fib6_eval_type(const struct nft_expr *expr,
> > struct nft_regs *regs,
> > u32 *dest = >data[priv->dreg];
> > struct ipv6hdr *iph, _iph;
> >
> > + if (!ipv6_mod_enabled()) {
> > + regs->verdict.code = NF_DROP;
>
> NFT_BREAK instead to stop evaluating this rule, this results in a
> mismatch, so you let the user decide what to do with packets that do
> not match your policy.
Ok, I will replace for v3.
>
> The drop case at the bottom of the fib eval function never actually
> never happens.
Which one do you mean?
signature.asc
Description: This is a digitally signed message part
Re: [PATCH v2 1/1] netfilter: nf_tables: fib: Drop IPV6 packages if IPv6 is disabled on boot
On Wed, Aug 21, 2019 at 11:15:06AM -0300, Leonardo Bras wrote:
> If IPv6 is disabled on boot (ipv6.disable=1), but nft_fib_inet ends up
> dealing with a IPv6 package, it causes a kernel panic in
> fib6_node_lookup_1(), crashing in bad_page_fault.
Q: How do you get to see IPv6 packets if IPv6 module is disable?
> The panic is caused by trying to deference a very low address (0x38
> in ppc64le), due to ipv6.fib6_main_tbl = NULL.
> BUG: Kernel NULL pointer dereference at 0x0038
>
> Fix this behavior by dropping IPv6 packages if !ipv6_mod_enabled().
I'd suggest: s/package/packet/
[...]
> diff --git a/net/ipv6/netfilter/nft_fib_ipv6.c
> b/net/ipv6/netfilter/nft_fib_ipv6.c
> index 7ece86afd079..75acc417e2ff 100644
> --- a/net/ipv6/netfilter/nft_fib_ipv6.c
> +++ b/net/ipv6/netfilter/nft_fib_ipv6.c
> @@ -125,6 +125,11 @@ void nft_fib6_eval_type(const struct nft_expr *expr,
> struct nft_regs *regs,
> u32 *dest = >data[priv->dreg];
> struct ipv6hdr *iph, _iph;
>
> + if (!ipv6_mod_enabled()) {
> + regs->verdict.code = NF_DROP;
NFT_BREAK instead to stop evaluating this rule, this results in a
mismatch, so you let the user decide what to do with packets that do
not match your policy.
The drop case at the bottom of the fib eval function never actually
never happens.
