linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v4 0/2] Drop IPV6 packets if IPv6 is disabled on boot
@ 2019-08-30 18:13 Leonardo Bras
  2019-08-30 18:13 ` [PATCH v4 1/2] netfilter: Terminate rule eval if protocol=IPv6 and ipv6 module is disabled Leonardo Bras
  2019-08-30 18:13 ` [PATCH v4 2/2] net: br_netfiler_hooks: Drops IPv6 packets if IPv6 module is not loaded Leonardo Bras
  0 siblings, 2 replies; 17+ messages in thread
From: Leonardo Bras @ 2019-08-30 18:13 UTC (permalink / raw)
  To: netfilter-devel, coreteam, bridge, netdev, linux-kernel
  Cc: Leonardo Bras, Pablo Neira Ayuso, Jozsef Kadlecsik,
	Florian Westphal, Roopa Prabhu, Nikolay Aleksandrov,
	David S. Miller

This patchset was prevously a single patch named:
- netfilter: nf_tables: fib: Drop IPV6 packets if IPv6 is disabled on boot

It fixes a bug where a host, with IPv6 disabled on boot, has to deal with
guest IPv6 packets, that comes from a bridge interface.
When these packets reach the host ip6tables they cause a kernel panic.

---
Changes from v3:
- Move drop logic from nft_fib6_eval{,_type} to nft_fib_netdev_eval
- Add another patch to drop ipv6 packets from bridge when ipv6 disabled  

Changes from v2:
- Replace veredict.code from NF_DROP to NFT_BREAK
- Updated commit message (s/package/packet)

Changes from v1:
- Move drop logic from nft_fib_inet_eval() to nft_fib6_eval{,_type}
so it can affect other usages of these functions.

Leonardo Bras (2):
  netfilter: Terminate rule eval if protocol=IPv6 and ipv6 module is
    disabled
  net: br_netfiler_hooks: Drops IPv6 packets if IPv6 module is not
    loaded

 net/bridge/br_netfilter_hooks.c | 2 ++
 net/netfilter/nft_fib_netdev.c  | 3 +++
 2 files changed, 5 insertions(+)

-- 
2.20.1


^ permalink raw reply	[flat|nested] 17+ messages in thread

* [PATCH v4 1/2] netfilter: Terminate rule eval if protocol=IPv6 and ipv6 module is disabled
  2019-08-30 18:13 [PATCH v4 0/2] Drop IPV6 packets if IPv6 is disabled on boot Leonardo Bras
@ 2019-08-30 18:13 ` Leonardo Bras
  2019-08-30 20:58   ` Florian Westphal
  2019-09-03 20:55   ` Pablo Neira Ayuso
  2019-08-30 18:13 ` [PATCH v4 2/2] net: br_netfiler_hooks: Drops IPv6 packets if IPv6 module is not loaded Leonardo Bras
  1 sibling, 2 replies; 17+ messages in thread
From: Leonardo Bras @ 2019-08-30 18:13 UTC (permalink / raw)
  To: netfilter-devel, coreteam, bridge, netdev, linux-kernel
  Cc: Leonardo Bras, Pablo Neira Ayuso, Jozsef Kadlecsik,
	Florian Westphal, Roopa Prabhu, Nikolay Aleksandrov,
	David S. Miller

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 <leonardo@linux.ibm.com>
---
 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


^ permalink raw reply related	[flat|nested] 17+ messages in thread

* [PATCH v4 2/2] net: br_netfiler_hooks: Drops IPv6 packets if IPv6 module is not loaded
  2019-08-30 18:13 [PATCH v4 0/2] Drop IPV6 packets if IPv6 is disabled on boot Leonardo Bras
  2019-08-30 18:13 ` [PATCH v4 1/2] netfilter: Terminate rule eval if protocol=IPv6 and ipv6 module is disabled Leonardo Bras
@ 2019-08-30 18:13 ` Leonardo Bras
  2019-08-30 20:55   ` Florian Westphal
  1 sibling, 1 reply; 17+ messages in thread
From: Leonardo Bras @ 2019-08-30 18:13 UTC (permalink / raw)
  To: netfilter-devel, coreteam, bridge, netdev, linux-kernel
  Cc: Leonardo Bras, Pablo Neira Ayuso, Jozsef Kadlecsik,
	Florian Westphal, Roopa Prabhu, Nikolay Aleksandrov,
	David S. Miller

A kernel panic can happen if a host has disabled IPv6 on boot and have to
process guest packets (coming from a bridge) using it's ip6tables.

IPv6 packets need to be dropped if the IPv6 module is not loaded.

Signed-off-by: Leonardo Bras <leonardo@linux.ibm.com>
---
 net/bridge/br_netfilter_hooks.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/net/bridge/br_netfilter_hooks.c b/net/bridge/br_netfilter_hooks.c
index d3f9592f4ff8..5e8693730df1 100644
--- a/net/bridge/br_netfilter_hooks.c
+++ b/net/bridge/br_netfilter_hooks.c
@@ -493,6 +493,8 @@ static unsigned int br_nf_pre_routing(void *priv,
 	brnet = net_generic(state->net, brnf_net_id);
 	if (IS_IPV6(skb) || is_vlan_ipv6(skb, state->net) ||
 	    is_pppoe_ipv6(skb, state->net)) {
+		if (!ipv6_mod_enabled())
+			return NF_DROP;
 		if (!brnet->call_ip6tables &&
 		    !br_opt_get(br, BROPT_NF_CALL_IP6TABLES))
 			return NF_ACCEPT;
-- 
2.20.1


^ permalink raw reply related	[flat|nested] 17+ messages in thread

* Re: [PATCH v4 2/2] net: br_netfiler_hooks: Drops IPv6 packets if IPv6 module is not loaded
  2019-08-30 18:13 ` [PATCH v4 2/2] net: br_netfiler_hooks: Drops IPv6 packets if IPv6 module is not loaded Leonardo Bras
@ 2019-08-30 20:55   ` Florian Westphal
  2019-08-31  4:42     ` Leonardo Bras
  0 siblings, 1 reply; 17+ messages in thread
From: Florian Westphal @ 2019-08-30 20:55 UTC (permalink / raw)
  To: Leonardo Bras
  Cc: netfilter-devel, coreteam, bridge, netdev, linux-kernel,
	Pablo Neira Ayuso, Jozsef Kadlecsik, Florian Westphal,
	Roopa Prabhu, Nikolay Aleksandrov, David S. Miller

Leonardo Bras <leonardo@linux.ibm.com> wrote:
> A kernel panic can happen if a host has disabled IPv6 on boot and have to
> process guest packets (coming from a bridge) using it's ip6tables.
> 
> IPv6 packets need to be dropped if the IPv6 module is not loaded.
> 
> Signed-off-by: Leonardo Bras <leonardo@linux.ibm.com>
> ---
>  net/bridge/br_netfilter_hooks.c | 2 ++
>  1 file changed, 2 insertions(+)
> 
> diff --git a/net/bridge/br_netfilter_hooks.c b/net/bridge/br_netfilter_hooks.c
> index d3f9592f4ff8..5e8693730df1 100644
> --- a/net/bridge/br_netfilter_hooks.c
> +++ b/net/bridge/br_netfilter_hooks.c
> @@ -493,6 +493,8 @@ static unsigned int br_nf_pre_routing(void *priv,
>  	brnet = net_generic(state->net, brnf_net_id);
>  	if (IS_IPV6(skb) || is_vlan_ipv6(skb, state->net) ||
>  	    is_pppoe_ipv6(skb, state->net)) {
> +		if (!ipv6_mod_enabled())
> +			return NF_DROP;
>  		if (!brnet->call_ip6tables &&
>  		    !br_opt_get(br, BROPT_NF_CALL_IP6TABLES))
>  			return NF_ACCEPT;

No, thats too aggressive and turns the bridge into an ipv6 blackhole.

There are two solutions:
1. The above patch, but use NF_ACCEPT instead
2. keep the DROP, but move it below the call_ip6tables test,
   so that users can tweak call-ip6tables to accept packets.

Perhaps it would be good to also add a pr_warn_once() that
tells that ipv6 was disabled on command line and
call-ip6tables isn't supported in this configuration.

I would go with option two.

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH v4 1/2] netfilter: Terminate rule eval if protocol=IPv6 and ipv6 module is disabled
  2019-08-30 18:13 ` [PATCH v4 1/2] netfilter: Terminate rule eval if protocol=IPv6 and ipv6 module is disabled Leonardo Bras
@ 2019-08-30 20:58   ` Florian Westphal
  2019-09-03 16:46     ` Leonardo Bras
  2019-09-03 20:55   ` Pablo Neira Ayuso
  1 sibling, 1 reply; 17+ messages in thread
From: Florian Westphal @ 2019-08-30 20:58 UTC (permalink / raw)
  To: Leonardo Bras
  Cc: netfilter-devel, coreteam, bridge, netdev, linux-kernel,
	Pablo Neira Ayuso, Jozsef Kadlecsik, Florian Westphal,
	Roopa Prabhu, Nikolay Aleksandrov, David S. Miller

Leonardo Bras <leonardo@linux.ibm.com> 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 <leonardo@linux.ibm.com>

Acked-by: Florian Westphal <fw@strlen.de>

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH v4 2/2] net: br_netfiler_hooks: Drops IPv6 packets if IPv6 module is not loaded
  2019-08-30 20:55   ` Florian Westphal
@ 2019-08-31  4:42     ` Leonardo Bras
  2019-08-31  8:43       ` Florian Westphal
  0 siblings, 1 reply; 17+ messages in thread
From: Leonardo Bras @ 2019-08-31  4:42 UTC (permalink / raw)
  To: Florian Westphal
  Cc: netfilter-devel, coreteam, bridge, netdev, linux-kernel,
	Pablo Neira Ayuso, Jozsef Kadlecsik, Roopa Prabhu,
	Nikolay Aleksandrov, David S. Miller

[-- Attachment #1: Type: text/plain, Size: 1904 bytes --]

On Fri, 2019-08-30 at 22:55 +0200, Florian Westphal wrote:
> Leonardo Bras <leonardo@linux.ibm.com> wrote:
> > A kernel panic can happen if a host has disabled IPv6 on boot and have to
> > process guest packets (coming from a bridge) using it's ip6tables.
> > 
> > IPv6 packets need to be dropped if the IPv6 module is not loaded.
> > 
> > Signed-off-by: Leonardo Bras <leonardo@linux.ibm.com>
> > ---
> >  net/bridge/br_netfilter_hooks.c | 2 ++
> >  1 file changed, 2 insertions(+)
> > 
> > diff --git a/net/bridge/br_netfilter_hooks.c b/net/bridge/br_netfilter_hooks.c
> > index d3f9592f4ff8..5e8693730df1 100644
> > --- a/net/bridge/br_netfilter_hooks.c
> > +++ b/net/bridge/br_netfilter_hooks.c
> > @@ -493,6 +493,8 @@ static unsigned int br_nf_pre_routing(void *priv,
> >  	brnet = net_generic(state->net, brnf_net_id);
> >  	if (IS_IPV6(skb) || is_vlan_ipv6(skb, state->net) ||
> >  	    is_pppoe_ipv6(skb, state->net)) {
> > +		if (!ipv6_mod_enabled())
> > +			return NF_DROP;
> >  		if (!brnet->call_ip6tables &&
> >  		    !br_opt_get(br, BROPT_NF_CALL_IP6TABLES))
> >  			return NF_ACCEPT;
> 
> No, thats too aggressive and turns the bridge into an ipv6 blackhole.
> 
> There are two solutions:
> 1. The above patch, but use NF_ACCEPT instead
> 2. keep the DROP, but move it below the call_ip6tables test,
>    so that users can tweak call-ip6tables to accept packets.

Q: Does 2 mean that it will only be dropped if bridge intents to use
host's ip6tables? Else, it will be accepted by previous if?

> Perhaps it would be good to also add a pr_warn_once() that
> tells that ipv6 was disabled on command line and
> call-ip6tables isn't supported in this configuration.
> 
Good idea, added.

> I would go with option two.
I think it's better than 1 too.

I sent a v5 with these changes:
https://lkml.org/lkml/2019/8/31/4

Thanks!

Leonardo Bras

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH v4 2/2] net: br_netfiler_hooks: Drops IPv6 packets if IPv6 module is not loaded
  2019-08-31  4:42     ` Leonardo Bras
@ 2019-08-31  8:43       ` Florian Westphal
  0 siblings, 0 replies; 17+ messages in thread
From: Florian Westphal @ 2019-08-31  8:43 UTC (permalink / raw)
  To: Leonardo Bras
  Cc: Florian Westphal, netfilter-devel, coreteam, bridge, netdev,
	linux-kernel, Pablo Neira Ayuso, Jozsef Kadlecsik, Roopa Prabhu,
	Nikolay Aleksandrov, David S. Miller

Leonardo Bras <leonardo@linux.ibm.com> wrote:
> > There are two solutions:
> > 1. The above patch, but use NF_ACCEPT instead
> > 2. keep the DROP, but move it below the call_ip6tables test,
> >    so that users can tweak call-ip6tables to accept packets.
> 
> Q: Does 2 mean that it will only be dropped if bridge intents to use
> host's ip6tables? Else, it will be accepted by previous if?

Yes, thats the idea: Let users decide if ipv6.disable or call-ip6tables
is more important to them.

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH v4 1/2] netfilter: Terminate rule eval if protocol=IPv6 and ipv6 module is disabled
  2019-08-30 20:58   ` Florian Westphal
@ 2019-09-03 16:46     ` Leonardo Bras
  2019-09-03 16:49       ` Pablo Neira Ayuso
  0 siblings, 1 reply; 17+ messages in thread
From: Leonardo Bras @ 2019-09-03 16:46 UTC (permalink / raw)
  To: Pablo Neira Ayuso
  Cc: netfilter-devel, coreteam, bridge, netdev, linux-kernel,
	FlorianWestphal, Jozsef Kadlecsik, Roopa Prabhu,
	Nikolay Aleksandrov, David S. Miller

[-- Attachment #1: Type: text/plain, Size: 1124 bytes --]

On Fri, 2019-08-30 at 22:58 +0200, Florian Westphal wrote:
> Leonardo Bras <leonardo@linux.ibm.com> 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 <leonardo@linux.ibm.com>
> 
> Acked-by: Florian Westphal <fw@strlen.de>
> 

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

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH v4 1/2] netfilter: Terminate rule eval if protocol=IPv6 and ipv6 module is disabled
  2019-09-03 16:46     ` Leonardo Bras
@ 2019-09-03 16:49       ` Pablo Neira Ayuso
  2019-09-03 16:56         ` Leonardo Bras
  2019-09-03 17:05         ` Florian Westphal
  0 siblings, 2 replies; 17+ messages in thread
From: Pablo Neira Ayuso @ 2019-09-03 16:49 UTC (permalink / raw)
  To: Leonardo Bras
  Cc: netfilter-devel, coreteam, bridge, netdev, linux-kernel,
	FlorianWestphal, Jozsef Kadlecsik, Roopa Prabhu,
	Nikolay Aleksandrov, David S. Miller

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 <leonardo@linux.ibm.com> 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 <leonardo@linux.ibm.com>
> > 
> > Acked-by: Florian Westphal <fw@strlen.de>
> > 
> 
> 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.

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH v4 1/2] netfilter: Terminate rule eval if protocol=IPv6 and ipv6 module is disabled
  2019-09-03 16:49       ` Pablo Neira Ayuso
@ 2019-09-03 16:56         ` Leonardo Bras
  2019-09-03 17:05         ` Florian Westphal
  1 sibling, 0 replies; 17+ messages in thread
From: Leonardo Bras @ 2019-09-03 16:56 UTC (permalink / raw)
  To: Pablo Neira Ayuso
  Cc: netfilter-devel, coreteam, bridge, netdev, linux-kernel,
	FlorianWestphal, Jozsef Kadlecsik, Roopa Prabhu,
	Nikolay Aleksandrov, David S. Miller

[-- Attachment #1: Type: text/plain, Size: 1095 bytes --]

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!

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH v4 1/2] netfilter: Terminate rule eval if protocol=IPv6 and ipv6 module is disabled
  2019-09-03 16:49       ` Pablo Neira Ayuso
  2019-09-03 16:56         ` Leonardo Bras
@ 2019-09-03 17:05         ` Florian Westphal
  2019-09-03 19:31           ` Pablo Neira Ayuso
  1 sibling, 1 reply; 17+ messages in thread
From: Florian Westphal @ 2019-09-03 17:05 UTC (permalink / raw)
  To: Pablo Neira Ayuso
  Cc: Leonardo Bras, netfilter-devel, coreteam, bridge, netdev,
	linux-kernel, FlorianWestphal, Jozsef Kadlecsik, Roopa Prabhu,
	Nikolay Aleksandrov, David S. Miller

Pablo Neira Ayuso <pablo@netfilter.org> 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 <leonardo@linux.ibm.com> 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 <leonardo@linux.ibm.com>
> > > 
> > > Acked-by: Florian Westphal <fw@strlen.de>
> > > 
> > 
> > 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.

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH v4 1/2] netfilter: Terminate rule eval if protocol=IPv6 and ipv6 module is disabled
  2019-09-03 17:05         ` Florian Westphal
@ 2019-09-03 19:31           ` Pablo Neira Ayuso
  2019-09-03 19:48             ` Florian Westphal
  0 siblings, 1 reply; 17+ messages in thread
From: Pablo Neira Ayuso @ 2019-09-03 19:31 UTC (permalink / raw)
  To: Florian Westphal
  Cc: Leonardo Bras, netfilter-devel, coreteam, bridge, netdev,
	linux-kernel, Jozsef Kadlecsik, Roopa Prabhu,
	Nikolay Aleksandrov, David S. Miller

On Tue, Sep 03, 2019 at 07:05:50PM +0200, Florian Westphal wrote:
> Pablo Neira Ayuso <pablo@netfilter.org> 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 <leonardo@linux.ibm.com> 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 <leonardo@linux.ibm.com>
> > > > 
> > > > Acked-by: Florian Westphal <fw@strlen.de>
> > > > 
> > > 
> > > 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.

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH v4 1/2] netfilter: Terminate rule eval if protocol=IPv6 and ipv6 module is disabled
  2019-09-03 19:31           ` Pablo Neira Ayuso
@ 2019-09-03 19:48             ` Florian Westphal
  2019-09-03 20:19               ` Pablo Neira Ayuso
  0 siblings, 1 reply; 17+ messages in thread
From: Florian Westphal @ 2019-09-03 19:48 UTC (permalink / raw)
  To: Pablo Neira Ayuso
  Cc: Florian Westphal, Leonardo Bras, netfilter-devel, coreteam,
	bridge, netdev, linux-kernel, Jozsef Kadlecsik, Roopa Prabhu,
	Nikolay Aleksandrov, David S. Miller

Pablo Neira Ayuso <pablo@netfilter.org> 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().

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH v4 1/2] netfilter: Terminate rule eval if protocol=IPv6 and ipv6 module is disabled
  2019-09-03 19:48             ` Florian Westphal
@ 2019-09-03 20:19               ` Pablo Neira Ayuso
  2019-09-03 20:35                 ` Florian Westphal
  0 siblings, 1 reply; 17+ messages in thread
From: Pablo Neira Ayuso @ 2019-09-03 20:19 UTC (permalink / raw)
  To: Florian Westphal
  Cc: Leonardo Bras, netfilter-devel, coreteam, bridge, netdev,
	linux-kernel, Jozsef Kadlecsik, Roopa Prabhu,
	Nikolay Aleksandrov, David S. Miller

On Tue, Sep 03, 2019 at 09:48:09PM +0200, Florian Westphal wrote:
> Pablo Neira Ayuso <pablo@netfilter.org> 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.

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH v4 1/2] netfilter: Terminate rule eval if protocol=IPv6 and ipv6 module is disabled
  2019-09-03 20:19               ` Pablo Neira Ayuso
@ 2019-09-03 20:35                 ` Florian Westphal
  2019-09-03 20:55                   ` Pablo Neira Ayuso
  0 siblings, 1 reply; 17+ messages in thread
From: Florian Westphal @ 2019-09-03 20:35 UTC (permalink / raw)
  To: Pablo Neira Ayuso
  Cc: Florian Westphal, Leonardo Bras, netfilter-devel, coreteam,
	bridge, netdev, linux-kernel, Jozsef Kadlecsik, Roopa Prabhu,
	Nikolay Aleksandrov, David S. Miller

Pablo Neira Ayuso <pablo@netfilter.org> 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.

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH v4 1/2] netfilter: Terminate rule eval if protocol=IPv6 and ipv6 module is disabled
  2019-09-03 20:35                 ` Florian Westphal
@ 2019-09-03 20:55                   ` Pablo Neira Ayuso
  0 siblings, 0 replies; 17+ messages in thread
From: Pablo Neira Ayuso @ 2019-09-03 20:55 UTC (permalink / raw)
  To: Florian Westphal
  Cc: Leonardo Bras, netfilter-devel, coreteam, bridge, netdev,
	linux-kernel, Jozsef Kadlecsik, Roopa Prabhu,
	Nikolay Aleksandrov, David S. Miller

On Tue, Sep 03, 2019 at 10:35:31PM +0200, Florian Westphal wrote:
> Pablo Neira Ayuso <pablo@netfilter.org> 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.

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH v4 1/2] netfilter: Terminate rule eval if protocol=IPv6 and ipv6 module is disabled
  2019-08-30 18:13 ` [PATCH v4 1/2] netfilter: Terminate rule eval if protocol=IPv6 and ipv6 module is disabled Leonardo Bras
  2019-08-30 20:58   ` Florian Westphal
@ 2019-09-03 20:55   ` Pablo Neira Ayuso
  1 sibling, 0 replies; 17+ messages in thread
From: Pablo Neira Ayuso @ 2019-09-03 20:55 UTC (permalink / raw)
  To: Leonardo Bras
  Cc: netfilter-devel, coreteam, bridge, netdev, linux-kernel,
	Jozsef Kadlecsik, Florian Westphal, Roopa Prabhu,
	Nikolay Aleksandrov, David S. Miller

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.

^ permalink raw reply	[flat|nested] 17+ messages in thread

end of thread, other threads:[~2019-09-03 20:55 UTC | newest]

Thread overview: 17+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-08-30 18:13 [PATCH v4 0/2] Drop IPV6 packets if IPv6 is disabled on boot Leonardo Bras
2019-08-30 18:13 ` [PATCH v4 1/2] netfilter: Terminate rule eval if protocol=IPv6 and ipv6 module is disabled Leonardo Bras
2019-08-30 20:58   ` Florian Westphal
2019-09-03 16:46     ` Leonardo Bras
2019-09-03 16:49       ` Pablo Neira Ayuso
2019-09-03 16:56         ` Leonardo Bras
2019-09-03 17:05         ` Florian Westphal
2019-09-03 19:31           ` Pablo Neira Ayuso
2019-09-03 19:48             ` Florian Westphal
2019-09-03 20:19               ` Pablo Neira Ayuso
2019-09-03 20:35                 ` Florian Westphal
2019-09-03 20:55                   ` Pablo Neira Ayuso
2019-09-03 20:55   ` Pablo Neira Ayuso
2019-08-30 18:13 ` [PATCH v4 2/2] net: br_netfiler_hooks: Drops IPv6 packets if IPv6 module is not loaded Leonardo Bras
2019-08-30 20:55   ` Florian Westphal
2019-08-31  4:42     ` Leonardo Bras
2019-08-31  8:43       ` Florian Westphal

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