2019-08-30 18:15:43

by Leonardo Bras

[permalink] [raw]
Subject: [PATCH v4 1/2] netfilter: Terminate rule eval if protocol=IPv6 and ipv6 module is disabled

If IPv6 is disabled on boot (ipv6.disable=1), but nft_fib_inet ends up
dealing with a IPv6 packet, it causes a kernel panic in
fib6_node_lookup_1(), crashing in bad_page_fault.

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 0x00000038

The kernel panic was reproduced in a host that disabled IPv6 on boot and
have to process guest packets (coming from a bridge) using it's ip6tables.

Terminate rule evaluation when packet protocol is IPv6 but the ipv6 module
is not loaded.

Signed-off-by: Leonardo Bras <[email protected]>
---
net/netfilter/nft_fib_netdev.c | 3 +++
1 file changed, 3 insertions(+)

diff --git a/net/netfilter/nft_fib_netdev.c b/net/netfilter/nft_fib_netdev.c
index 2cf3f32fe6d2..a2e726ae7f07 100644
--- a/net/netfilter/nft_fib_netdev.c
+++ b/net/netfilter/nft_fib_netdev.c
@@ -14,6 +14,7 @@
#include <linux/netfilter/nf_tables.h>
#include <net/netfilter/nf_tables_core.h>
#include <net/netfilter/nf_tables.h>
+#include <net/ipv6.h>

#include <net/netfilter/nft_fib.h>

@@ -34,6 +35,8 @@ static void nft_fib_netdev_eval(const struct nft_expr *expr,
}
break;
case ETH_P_IPV6:
+ if (!ipv6_mod_enabled())
+ break;
switch (priv->result) {
case NFT_FIB_RESULT_OIF:
case NFT_FIB_RESULT_OIFNAME:
--
2.20.1


2019-08-30 20:59:13

by Florian Westphal

[permalink] [raw]
Subject: Re: [PATCH v4 1/2] netfilter: Terminate rule eval if protocol=IPv6 and ipv6 module is disabled

Leonardo Bras <[email protected]> wrote:
> If IPv6 is disabled on boot (ipv6.disable=1), but nft_fib_inet ends up
> dealing with a IPv6 packet, it causes a kernel panic in
> fib6_node_lookup_1(), crashing in bad_page_fault.
>
> 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 0x00000038
>
> The kernel panic was reproduced in a host that disabled IPv6 on boot and
> have to process guest packets (coming from a bridge) using it's ip6tables.
>
> Terminate rule evaluation when packet protocol is IPv6 but the ipv6 module
> is not loaded.
>
> Signed-off-by: Leonardo Bras <[email protected]>

Acked-by: Florian Westphal <[email protected]>

2019-09-03 16:48:22

by Leonardo Bras

[permalink] [raw]
Subject: Re: [PATCH v4 1/2] netfilter: Terminate rule eval if protocol=IPv6 and ipv6 module is disabled

On Fri, 2019-08-30 at 22:58 +0200, Florian Westphal wrote:
> Leonardo Bras <[email protected]> wrote:
> > If IPv6 is disabled on boot (ipv6.disable=1), but nft_fib_inet ends up
> > dealing with a IPv6 packet, it causes a kernel panic in
> > fib6_node_lookup_1(), crashing in bad_page_fault.
> >
> > 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 0x00000038
> >
> > The kernel panic was reproduced in a host that disabled IPv6 on boot and
> > have to process guest packets (coming from a bridge) using it's ip6tables.
> >
> > Terminate rule evaluation when packet protocol is IPv6 but the ipv6 module
> > is not loaded.
> >
> > Signed-off-by: Leonardo Bras <[email protected]>
>
> Acked-by: Florian Westphal <[email protected]>
>

Hello Pablo,

Any trouble with this patch?
I could see the other* one got applied, but not this one.
*(The other did not get acked, so i released it alone as v5)

Is there any fix I need to do in this one?

Best regards,
Leonardo Bras


Attachments:
signature.asc (849.00 B)
This is a digitally signed message part

2019-09-03 16:51:00

by Pablo Neira Ayuso

[permalink] [raw]
Subject: Re: [PATCH v4 1/2] netfilter: Terminate rule eval if protocol=IPv6 and ipv6 module is disabled

On Tue, Sep 03, 2019 at 01:46:50PM -0300, Leonardo Bras wrote:
> On Fri, 2019-08-30 at 22:58 +0200, Florian Westphal wrote:
> > Leonardo Bras <[email protected]> wrote:
> > > If IPv6 is disabled on boot (ipv6.disable=1), but nft_fib_inet ends up
> > > dealing with a IPv6 packet, it causes a kernel panic in
> > > fib6_node_lookup_1(), crashing in bad_page_fault.
> > >
> > > 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 0x00000038
> > >
> > > The kernel panic was reproduced in a host that disabled IPv6 on boot and
> > > have to process guest packets (coming from a bridge) using it's ip6tables.
> > >
> > > Terminate rule evaluation when packet protocol is IPv6 but the ipv6 module
> > > is not loaded.
> > >
> > > Signed-off-by: Leonardo Bras <[email protected]>
> >
> > Acked-by: Florian Westphal <[email protected]>
> >
>
> Hello Pablo,
>
> Any trouble with this patch?
> I could see the other* one got applied, but not this one.
> *(The other did not get acked, so i released it alone as v5)
>
> Is there any fix I need to do in this one?

Hm, I see, so this one:

https://patchwork.ozlabs.org/patch/1156100/

is not enough?

I was expecting we could find a way to handle this from br_netfilter
alone itself.

Thanks.

2019-09-03 16:57:41

by Leonardo Bras

[permalink] [raw]
Subject: Re: [PATCH v4 1/2] netfilter: Terminate rule eval if protocol=IPv6 and ipv6 module is disabled

On Tue, 2019-09-03 at 18:49 +0200, Pablo Neira Ayuso wrote:
> On Tue, Sep 03, 2019 at 01:46:50PM -0300, Leonardo Bras wrote:
> > On Fri, 2019-08-30 at 22:58 +0200, Florian Westphal wrote:
> > Hello Pablo,
> >
> > Any trouble with this patch?
> > I could see the other* one got applied, but not this one.
> > *(The other did not get acked, so i released it alone as v5)
> >
> > Is there any fix I need to do in this one?
>
> Hm, I see, so this one:
>
> https://patchwork.ozlabs.org/patch/1156100/
>
> is not enough?

By what I could understand of Florian e-mail, we would need both:

>> 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".

Is that ok?

>
> I was expecting we could find a way to handle this from br_netfilter
> alone itself.
>
> Thanks.
Thank you!


Attachments:
signature.asc (849.00 B)
This is a digitally signed message part

2019-09-03 17:07:10

by Florian Westphal

[permalink] [raw]
Subject: Re: [PATCH v4 1/2] netfilter: Terminate rule eval if protocol=IPv6 and ipv6 module is disabled

Pablo Neira Ayuso <[email protected]> wrote:
> On Tue, Sep 03, 2019 at 01:46:50PM -0300, Leonardo Bras wrote:
> > On Fri, 2019-08-30 at 22:58 +0200, Florian Westphal wrote:
> > > Leonardo Bras <[email protected]> wrote:
> > > > If IPv6 is disabled on boot (ipv6.disable=1), but nft_fib_inet ends up
> > > > dealing with a IPv6 packet, it causes a kernel panic in
> > > > fib6_node_lookup_1(), crashing in bad_page_fault.
> > > >
> > > > 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 0x00000038
> > > >
> > > > The kernel panic was reproduced in a host that disabled IPv6 on boot and
> > > > have to process guest packets (coming from a bridge) using it's ip6tables.
> > > >
> > > > Terminate rule evaluation when packet protocol is IPv6 but the ipv6 module
> > > > is not loaded.
> > > >
> > > > Signed-off-by: Leonardo Bras <[email protected]>
> > >
> > > Acked-by: Florian Westphal <[email protected]>
> > >
> >
> > Hello Pablo,
> >
> > Any trouble with this patch?
> > I could see the other* one got applied, but not this one.
> > *(The other did not get acked, so i released it alone as v5)
> >
> > Is there any fix I need to do in this one?
>
> Hm, I see, so this one:
>
> https://patchwork.ozlabs.org/patch/1156100/
>
> is not enough?

No, its not.

> I was expecting we could find a way to handle this from br_netfilter
> alone itself.

We can't because we support ipv6 fib lookups from the netdev family
as well.

Alternative is to auto-accept ipv6 packets from the nf_tables eval loop,
but I think its worse.

2019-09-03 19:34:32

by Pablo Neira Ayuso

[permalink] [raw]
Subject: Re: [PATCH v4 1/2] netfilter: Terminate rule eval if protocol=IPv6 and ipv6 module is disabled

On Tue, Sep 03, 2019 at 07:05:50PM +0200, Florian Westphal wrote:
> Pablo Neira Ayuso <[email protected]> wrote:
> > On Tue, Sep 03, 2019 at 01:46:50PM -0300, Leonardo Bras wrote:
> > > On Fri, 2019-08-30 at 22:58 +0200, Florian Westphal wrote:
> > > > Leonardo Bras <[email protected]> wrote:
> > > > > If IPv6 is disabled on boot (ipv6.disable=1), but nft_fib_inet ends up
> > > > > dealing with a IPv6 packet, it causes a kernel panic in
> > > > > fib6_node_lookup_1(), crashing in bad_page_fault.
> > > > >
> > > > > 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 0x00000038
> > > > >
> > > > > The kernel panic was reproduced in a host that disabled IPv6 on boot and
> > > > > have to process guest packets (coming from a bridge) using it's ip6tables.
> > > > >
> > > > > Terminate rule evaluation when packet protocol is IPv6 but the ipv6 module
> > > > > is not loaded.
> > > > >
> > > > > Signed-off-by: Leonardo Bras <[email protected]>
> > > >
> > > > Acked-by: Florian Westphal <[email protected]>
> > > >
> > >
> > > Hello Pablo,
> > >
> > > Any trouble with this patch?
> > > I could see the other* one got applied, but not this one.
> > > *(The other did not get acked, so i released it alone as v5)
> > >
> > > Is there any fix I need to do in this one?
> >
> > Hm, I see, so this one:
> >
> > https://patchwork.ozlabs.org/patch/1156100/
> >
> > is not enough?
>
> No, its not.
>
> > I was expecting we could find a way to handle this from br_netfilter
> > alone itself.
>
> We can't because we support ipv6 fib lookups from the netdev family
> as well.
>
> Alternative is to auto-accept ipv6 packets from the nf_tables eval loop,
> but I think its worse.

Could we add a restriction for nf_tables + br_netfilter + !ipv6. I
mean, if this is an IPv6 packet, nf_tables is on and IPv6 module if
off, then drop this packet?

By dropping packet, the user could diagnose that its setup is
incomplete. I mean, if nf_tables fib ipv6 is used, then this setup is
really wrong and the user forgots to load the ipv6 module.

2019-09-03 19:49:24

by Florian Westphal

[permalink] [raw]
Subject: Re: [PATCH v4 1/2] netfilter: Terminate rule eval if protocol=IPv6 and ipv6 module is disabled

Pablo Neira Ayuso <[email protected]> wrote:
> > > I was expecting we could find a way to handle this from br_netfilter
> > > alone itself.
> >
> > We can't because we support ipv6 fib lookups from the netdev family
> > as well.
> >
> > Alternative is to auto-accept ipv6 packets from the nf_tables eval loop,
> > but I think its worse.
>
> Could we add a restriction for nf_tables + br_netfilter + !ipv6. I
> mean, if this is an IPv6 packet, nf_tables is on and IPv6 module if
> off, then drop this packet?

We could do that from nft_do_chain_netdev().

2019-09-03 20:20:16

by Pablo Neira Ayuso

[permalink] [raw]
Subject: Re: [PATCH v4 1/2] netfilter: Terminate rule eval if protocol=IPv6 and ipv6 module is disabled

On Tue, Sep 03, 2019 at 09:48:09PM +0200, Florian Westphal wrote:
> Pablo Neira Ayuso <[email protected]> wrote:
> > > > I was expecting we could find a way to handle this from br_netfilter
> > > > alone itself.
> > >
> > > We can't because we support ipv6 fib lookups from the netdev family
> > > as well.
> > >
> > > Alternative is to auto-accept ipv6 packets from the nf_tables eval loop,
> > > but I think its worse.
> >
> > Could we add a restriction for nf_tables + br_netfilter + !ipv6. I
> > mean, if this is an IPv6 packet, nf_tables is on and IPv6 module if
> > off, then drop this packet?
>
> We could do that from nft_do_chain_netdev().

Indeed, this is all about the netdev case.

Probably add something similar to nf_ip6_route() to deal with
ip6_route_lookup() case? This is the one trigering the problem, right?

BTW, how does nft_fib_ipv6 module kicks in if ipv6 module is not
loaded? The symbol dependency would pull in the IPv6 module anyway.

2019-09-03 20:36:41

by Florian Westphal

[permalink] [raw]
Subject: Re: [PATCH v4 1/2] netfilter: Terminate rule eval if protocol=IPv6 and ipv6 module is disabled

Pablo Neira Ayuso <[email protected]> wrote:
> On Tue, Sep 03, 2019 at 09:48:09PM +0200, Florian Westphal wrote:
> > We could do that from nft_do_chain_netdev().
>
> Indeed, this is all about the netdev case.
>
> Probably add something similar to nf_ip6_route() to deal with
> ip6_route_lookup() case? This is the one trigering the problem, right?

Yes, this particular problem is caused by ipv6 fib not being
initialized due to ipv6.disable=1. I don't know if there are cases
other than FIB.

> BTW, how does nft_fib_ipv6 module kicks in if ipv6 module is not
> loaded? The symbol dependency would pull in the IPv6 module anyway.

ipv6.disabled=1 does load the ipv6 module, but its non-functional.

2019-09-03 20:56:56

by Pablo Neira Ayuso

[permalink] [raw]
Subject: Re: [PATCH v4 1/2] netfilter: Terminate rule eval if protocol=IPv6 and ipv6 module is disabled

On Fri, Aug 30, 2019 at 03:13:53PM -0300, Leonardo Bras wrote:
> If IPv6 is disabled on boot (ipv6.disable=1), but nft_fib_inet ends up
> dealing with a IPv6 packet, it causes a kernel panic in
> fib6_node_lookup_1(), crashing in bad_page_fault.
>
> 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 0x00000038
>
> The kernel panic was reproduced in a host that disabled IPv6 on boot and
> have to process guest packets (coming from a bridge) using it's ip6tables.
>
> Terminate rule evaluation when packet protocol is IPv6 but the ipv6 module
> is not loaded.

Patch is applied, thanks.

2019-09-03 20:57:42

by Pablo Neira Ayuso

[permalink] [raw]
Subject: Re: [PATCH v4 1/2] netfilter: Terminate rule eval if protocol=IPv6 and ipv6 module is disabled

On Tue, Sep 03, 2019 at 10:35:31PM +0200, Florian Westphal wrote:
> Pablo Neira Ayuso <[email protected]> wrote:
> > On Tue, Sep 03, 2019 at 09:48:09PM +0200, Florian Westphal wrote:
> > > We could do that from nft_do_chain_netdev().
> >
> > Indeed, this is all about the netdev case.
> >
> > Probably add something similar to nf_ip6_route() to deal with
> > ip6_route_lookup() case? This is the one trigering the problem, right?
>
> Yes, this particular problem is caused by ipv6 fib not being
> initialized due to ipv6.disable=1. I don't know if there are cases
> other than FIB.
>
> > BTW, how does nft_fib_ipv6 module kicks in if ipv6 module is not
> > loaded? The symbol dependency would pull in the IPv6 module anyway.
>
> ipv6.disabled=1 does load the ipv6 module, but its non-functional.

I see, thanks for explaining.