* [PATCH net 0/2] fix two kernel panics when disabled IPv6 on boot up
@ 2019-02-06 12:51 Hangbin Liu
2019-02-06 12:51 ` [PATCH net 1/2] geneve: should not call rt6_lookup() when ipv6 was disabled Hangbin Liu
` (2 more replies)
0 siblings, 3 replies; 16+ messages in thread
From: Hangbin Liu @ 2019-02-06 12:51 UTC (permalink / raw)
To: netdev; +Cc: Stefano Brivio, David Miller, Hangbin Liu
When disabled IPv6 on boot up, since there is no ipv6 route tables, we should
not call rt6_lookup. Fix them by checking if we have inet6_dev pointer on
netdevice.
Hangbin Liu (2):
geneve: should not call rt6_lookup() when ipv6 was disabled
sit: check if IPv6 enabled before call ip6_err_gen_icmpv6_unreach()
drivers/net/geneve.c | 6 ++++++
net/ipv6/sit.c | 8 +++++++-
2 files changed, 13 insertions(+), 1 deletion(-)
--
2.19.2
^ permalink raw reply [flat|nested] 16+ messages in thread
* [PATCH net 1/2] geneve: should not call rt6_lookup() when ipv6 was disabled
2019-02-06 12:51 [PATCH net 0/2] fix two kernel panics when disabled IPv6 on boot up Hangbin Liu
@ 2019-02-06 12:51 ` Hangbin Liu
2019-02-06 14:58 ` Stefano Brivio
` (3 more replies)
2019-02-06 12:51 ` [PATCH net 2/2] sit: check if IPv6 enabled before call ip6_err_gen_icmpv6_unreach() Hangbin Liu
2019-02-07 10:36 ` [PATCH v2 net 0/2] fix two kernel panics when disabled IPv6 on boot up Hangbin Liu
2 siblings, 4 replies; 16+ messages in thread
From: Hangbin Liu @ 2019-02-06 12:51 UTC (permalink / raw)
To: netdev; +Cc: Stefano Brivio, David Miller, Hangbin Liu
When we add a new GENEVE device with IPv6 remote, checking only for
IS_ENABLED(CONFIG_IPV6) is not enough as we may disable IPv6 in kernel
cmd(ipv6.disable=1), which will cause a NULL pointer dereference.
Reported-by: Jianlin Shi <jishi@redhat.com>
Signed-off-by: Hangbin Liu <liuhangbin@gmail.com>
---
drivers/net/geneve.c | 6 ++++++
1 file changed, 6 insertions(+)
diff --git a/drivers/net/geneve.c b/drivers/net/geneve.c
index 58bbba8582b0..0658715581e3 100644
--- a/drivers/net/geneve.c
+++ b/drivers/net/geneve.c
@@ -1512,6 +1512,10 @@ static void geneve_link_config(struct net_device *dev,
}
#if IS_ENABLED(CONFIG_IPV6)
case AF_INET6: {
+ struct inet6_dev *idev = in6_dev_get(dev);
+ if (!idev)
+ break;
+
struct rt6_info *rt = rt6_lookup(geneve->net,
&info->key.u.ipv6.dst, NULL, 0,
NULL, 0);
@@ -1519,6 +1523,8 @@ static void geneve_link_config(struct net_device *dev,
if (rt && rt->dst.dev)
ldev_mtu = rt->dst.dev->mtu - GENEVE_IPV6_HLEN;
ip6_rt_put(rt);
+
+ in6_dev_put(idev);
break;
}
#endif
--
2.19.2
^ permalink raw reply related [flat|nested] 16+ messages in thread
* [PATCH net 2/2] sit: check if IPv6 enabled before call ip6_err_gen_icmpv6_unreach()
2019-02-06 12:51 [PATCH net 0/2] fix two kernel panics when disabled IPv6 on boot up Hangbin Liu
2019-02-06 12:51 ` [PATCH net 1/2] geneve: should not call rt6_lookup() when ipv6 was disabled Hangbin Liu
@ 2019-02-06 12:51 ` Hangbin Liu
2019-02-06 15:00 ` Stefano Brivio
2019-02-06 16:45 ` Eric Dumazet
2019-02-07 10:36 ` [PATCH v2 net 0/2] fix two kernel panics when disabled IPv6 on boot up Hangbin Liu
2 siblings, 2 replies; 16+ messages in thread
From: Hangbin Liu @ 2019-02-06 12:51 UTC (permalink / raw)
To: netdev; +Cc: Stefano Brivio, David Miller, Hangbin Liu
If we disabled IPv6 from kernel boot up cmd(ipv6.disable=1), we should not
call ip6_err_gen_icmpv6_unreach().
Reproducer:
ip link add sit1 type sit local 10.10.0.1 remote 10.10.1.1 ttl 1
ip link set sit1 up
ip addr add 192.168.0.1/24 dev sit1
ping 192.168.0.2
Reported-by: Jianlin Shi <jishi@redhat.com>
Signed-off-by: Hangbin Liu <liuhangbin@gmail.com>
---
net/ipv6/sit.c | 8 +++++++-
1 file changed, 7 insertions(+), 1 deletion(-)
diff --git a/net/ipv6/sit.c b/net/ipv6/sit.c
index 1e03305c0549..e43fbac0fd1a 100644
--- a/net/ipv6/sit.c
+++ b/net/ipv6/sit.c
@@ -493,6 +493,7 @@ static int ipip6_err(struct sk_buff *skb, u32 info)
const int type = icmp_hdr(skb)->type;
const int code = icmp_hdr(skb)->code;
unsigned int data_len = 0;
+ struct inet6_dev *idev;
struct ip_tunnel *t;
int sifindex;
int err;
@@ -546,8 +547,13 @@ static int ipip6_err(struct sk_buff *skb, u32 info)
}
err = 0;
- if (!ip6_err_gen_icmpv6_unreach(skb, iph->ihl * 4, type, data_len))
+
+ idev = in6_dev_get(skb->dev);
+ if (idev &&
+ !ip6_err_gen_icmpv6_unreach(skb, iph->ihl * 4, type, data_len)) {
+ in6_dev_put(idev);
goto out;
+ }
if (t->parms.iph.daddr == 0)
goto out;
--
2.19.2
^ permalink raw reply related [flat|nested] 16+ messages in thread
* Re: [PATCH net 1/2] geneve: should not call rt6_lookup() when ipv6 was disabled
2019-02-06 12:51 ` [PATCH net 1/2] geneve: should not call rt6_lookup() when ipv6 was disabled Hangbin Liu
@ 2019-02-06 14:58 ` Stefano Brivio
2019-02-06 16:43 ` Eric Dumazet
` (2 subsequent siblings)
3 siblings, 0 replies; 16+ messages in thread
From: Stefano Brivio @ 2019-02-06 14:58 UTC (permalink / raw)
To: Hangbin Liu; +Cc: netdev, David Miller
Hi Hangbin,
On Wed, 6 Feb 2019 20:51:10 +0800
Hangbin Liu <liuhangbin@gmail.com> wrote:
> When we add a new GENEVE device with IPv6 remote, checking only for
> IS_ENABLED(CONFIG_IPV6) is not enough as we may disable IPv6 in kernel
> cmd(ipv6.disable=1), which will cause a NULL pointer dereference.
>
> Reported-by: Jianlin Shi <jishi@redhat.com>
> Signed-off-by: Hangbin Liu <liuhangbin@gmail.com>
> ---
> drivers/net/geneve.c | 6 ++++++
> 1 file changed, 6 insertions(+)
>
> diff --git a/drivers/net/geneve.c b/drivers/net/geneve.c
> index 58bbba8582b0..0658715581e3 100644
> --- a/drivers/net/geneve.c
> +++ b/drivers/net/geneve.c
> @@ -1512,6 +1512,10 @@ static void geneve_link_config(struct net_device *dev,
> }
> #if IS_ENABLED(CONFIG_IPV6)
> case AF_INET6: {
> + struct inet6_dev *idev = in6_dev_get(dev);
> + if (!idev)
> + break;
> +
> struct rt6_info *rt = rt6_lookup(geneve->net,
> &info->key.u.ipv6.dst, NULL, 0,
> NULL, 0);
You're mixing declarations and code here, ISO C90 forbids it. You
could rather declare:
struct inet6_dev *idev = in6_dev_get(dev);
struct rt6_info *rt;
then check idev, and then call rt6_lookup().
> @@ -1519,6 +1523,8 @@ static void geneve_link_config(struct net_device *dev,
> if (rt && rt->dst.dev)
> ldev_mtu = rt->dst.dev->mtu - GENEVE_IPV6_HLEN;
> ip6_rt_put(rt);
> +
> + in6_dev_put(idev);
I think it would be better to put this right after the check on idev,
mostly for readability, but also, marginally, to reduce the scope of
the reference count bump.
--
Stefano
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH net 2/2] sit: check if IPv6 enabled before call ip6_err_gen_icmpv6_unreach()
2019-02-06 12:51 ` [PATCH net 2/2] sit: check if IPv6 enabled before call ip6_err_gen_icmpv6_unreach() Hangbin Liu
@ 2019-02-06 15:00 ` Stefano Brivio
2019-02-06 16:45 ` Eric Dumazet
1 sibling, 0 replies; 16+ messages in thread
From: Stefano Brivio @ 2019-02-06 15:00 UTC (permalink / raw)
To: Hangbin Liu; +Cc: netdev, David Miller
On Wed, 6 Feb 2019 20:51:11 +0800
Hangbin Liu <liuhangbin@gmail.com> wrote:
> If we disabled IPv6 from kernel boot up cmd(ipv6.disable=1), we should not
> call ip6_err_gen_icmpv6_unreach().
>
> Reproducer:
> ip link add sit1 type sit local 10.10.0.1 remote 10.10.1.1 ttl 1
> ip link set sit1 up
> ip addr add 192.168.0.1/24 dev sit1
> ping 192.168.0.2
>
> Reported-by: Jianlin Shi <jishi@redhat.com>
> Signed-off-by: Hangbin Liu <liuhangbin@gmail.com>
> ---
> net/ipv6/sit.c | 8 +++++++-
> 1 file changed, 7 insertions(+), 1 deletion(-)
>
> diff --git a/net/ipv6/sit.c b/net/ipv6/sit.c
> index 1e03305c0549..e43fbac0fd1a 100644
> --- a/net/ipv6/sit.c
> +++ b/net/ipv6/sit.c
> @@ -493,6 +493,7 @@ static int ipip6_err(struct sk_buff *skb, u32 info)
> const int type = icmp_hdr(skb)->type;
> const int code = icmp_hdr(skb)->code;
> unsigned int data_len = 0;
> + struct inet6_dev *idev;
> struct ip_tunnel *t;
> int sifindex;
> int err;
> @@ -546,8 +547,13 @@ static int ipip6_err(struct sk_buff *skb, u32 info)
> }
>
> err = 0;
> - if (!ip6_err_gen_icmpv6_unreach(skb, iph->ihl * 4, type, data_len))
> +
> + idev = in6_dev_get(skb->dev);
> + if (idev &&
> + !ip6_err_gen_icmpv6_unreach(skb, iph->ihl * 4, type, data_len)) {
> + in6_dev_put(idev);
> goto out;
> + }
You are leaking a reference if idev && ip6_err_gen_icmpv6_unreach(...).
You should call in6_dev_put(idev) whenever idev is not NULL instead.
--
Stefano
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH net 1/2] geneve: should not call rt6_lookup() when ipv6 was disabled
2019-02-06 12:51 ` [PATCH net 1/2] geneve: should not call rt6_lookup() when ipv6 was disabled Hangbin Liu
2019-02-06 14:58 ` Stefano Brivio
@ 2019-02-06 16:43 ` Eric Dumazet
2019-02-06 18:54 ` David Ahern
2019-02-07 0:55 ` kbuild test robot
3 siblings, 0 replies; 16+ messages in thread
From: Eric Dumazet @ 2019-02-06 16:43 UTC (permalink / raw)
To: Hangbin Liu, netdev; +Cc: Stefano Brivio, David Miller
On 02/06/2019 04:51 AM, Hangbin Liu wrote:
> When we add a new GENEVE device with IPv6 remote, checking only for
> IS_ENABLED(CONFIG_IPV6) is not enough as we may disable IPv6 in kernel
> cmd(ipv6.disable=1), which will cause a NULL pointer dereference.
>
> Reported-by: Jianlin Shi <jishi@redhat.com>
> Signed-off-by: Hangbin Liu <liuhangbin@gmail.com>
> ---
> drivers/net/geneve.c | 6 ++++++
> 1 file changed, 6 insertions(+)
>
> diff --git a/drivers/net/geneve.c b/drivers/net/geneve.c
> index 58bbba8582b0..0658715581e3 100644
> --- a/drivers/net/geneve.c
> +++ b/drivers/net/geneve.c
> @@ -1512,6 +1512,10 @@ static void geneve_link_config(struct net_device *dev,
> }
> #if IS_ENABLED(CONFIG_IPV6)
> case AF_INET6: {
> + struct inet6_dev *idev = in6_dev_get(dev);
> + if (!idev)
> + break;
Do not mix declarations and code.
Instead, use :
struct inet6_dev *idev = in6_dev_get(dev);
struct rt6_info *rt;
if (!idev)
break;
rt = rt6_lookup(geneve->net, ...);
> +
> struct rt6_info *rt = rt6_lookup(geneve->net,
> &info->key.u.ipv6.dst, NULL, 0,
> NULL, 0);
> @@ -1519,6 +1523,8 @@ static void geneve_link_config(struct net_device *dev,
> if (rt && rt->dst.dev)
> ldev_mtu = rt->dst.dev->mtu - GENEVE_IPV6_HLEN;
> ip6_rt_put(rt);
> +
> + in6_dev_put(idev);
> break;
> }
> #endif
>
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH net 2/2] sit: check if IPv6 enabled before call ip6_err_gen_icmpv6_unreach()
2019-02-06 12:51 ` [PATCH net 2/2] sit: check if IPv6 enabled before call ip6_err_gen_icmpv6_unreach() Hangbin Liu
2019-02-06 15:00 ` Stefano Brivio
@ 2019-02-06 16:45 ` Eric Dumazet
1 sibling, 0 replies; 16+ messages in thread
From: Eric Dumazet @ 2019-02-06 16:45 UTC (permalink / raw)
To: Hangbin Liu, netdev; +Cc: Stefano Brivio, David Miller
On 02/06/2019 04:51 AM, Hangbin Liu wrote:
> If we disabled IPv6 from kernel boot up cmd(ipv6.disable=1), we should not
> call ip6_err_gen_icmpv6_unreach().
>
> Reproducer:
> ip link add sit1 type sit local 10.10.0.1 remote 10.10.1.1 ttl 1
> ip link set sit1 up
> ip addr add 192.168.0.1/24 dev sit1
> ping 192.168.0.2
>
> Reported-by: Jianlin Shi <jishi@redhat.com>
> Signed-off-by: Hangbin Liu <liuhangbin@gmail.com>
> ---
> net/ipv6/sit.c | 8 +++++++-
> 1 file changed, 7 insertions(+), 1 deletion(-)
>
> diff --git a/net/ipv6/sit.c b/net/ipv6/sit.c
> index 1e03305c0549..e43fbac0fd1a 100644
> --- a/net/ipv6/sit.c
> +++ b/net/ipv6/sit.c
> @@ -493,6 +493,7 @@ static int ipip6_err(struct sk_buff *skb, u32 info)
> const int type = icmp_hdr(skb)->type;
> const int code = icmp_hdr(skb)->code;
> unsigned int data_len = 0;
> + struct inet6_dev *idev;
> struct ip_tunnel *t;
> int sifindex;
> int err;
> @@ -546,8 +547,13 @@ static int ipip6_err(struct sk_buff *skb, u32 info)
> }
>
> err = 0;
> - if (!ip6_err_gen_icmpv6_unreach(skb, iph->ihl * 4, type, data_len))
> +
> + idev = in6_dev_get(skb->dev);
> + if (idev &&
> + !ip6_err_gen_icmpv6_unreach(skb, iph->ihl * 4, type, data_len)) {
> + in6_dev_put(idev);
> goto out;
> + }
>
It seems there is a missing in6_dev_put(idev) depending on ip6_err_gen_icmpv6_unreach()() return value ?
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH net 1/2] geneve: should not call rt6_lookup() when ipv6 was disabled
2019-02-06 12:51 ` [PATCH net 1/2] geneve: should not call rt6_lookup() when ipv6 was disabled Hangbin Liu
2019-02-06 14:58 ` Stefano Brivio
2019-02-06 16:43 ` Eric Dumazet
@ 2019-02-06 18:54 ` David Ahern
2019-02-06 19:04 ` Stefano Brivio
2019-02-06 19:08 ` Eric Dumazet
2019-02-07 0:55 ` kbuild test robot
3 siblings, 2 replies; 16+ messages in thread
From: David Ahern @ 2019-02-06 18:54 UTC (permalink / raw)
To: Hangbin Liu, netdev; +Cc: Stefano Brivio, David Miller
On 2/6/19 4:51 AM, Hangbin Liu wrote:
> When we add a new GENEVE device with IPv6 remote, checking only for
> IS_ENABLED(CONFIG_IPV6) is not enough as we may disable IPv6 in kernel
> cmd(ipv6.disable=1), which will cause a NULL pointer dereference.
>
> Reported-by: Jianlin Shi <jishi@redhat.com>
> Signed-off-by: Hangbin Liu <liuhangbin@gmail.com>
> ---
> drivers/net/geneve.c | 6 ++++++
> 1 file changed, 6 insertions(+)
>
> diff --git a/drivers/net/geneve.c b/drivers/net/geneve.c
> index 58bbba8582b0..0658715581e3 100644
> --- a/drivers/net/geneve.c
> +++ b/drivers/net/geneve.c
> @@ -1512,6 +1512,10 @@ static void geneve_link_config(struct net_device *dev,
> }
> #if IS_ENABLED(CONFIG_IPV6)
> case AF_INET6: {
> + struct inet6_dev *idev = in6_dev_get(dev);
> + if (!idev)
> + break;
Since you don't need an idev reference rcu_access_pointer should be
enough to say v6 is enabled on this device. ie., add a helper to check
that rcu_access_pointer(dev->ip6_ptr) is non-NULL
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH net 1/2] geneve: should not call rt6_lookup() when ipv6 was disabled
2019-02-06 18:54 ` David Ahern
@ 2019-02-06 19:04 ` Stefano Brivio
2019-02-06 19:09 ` Eric Dumazet
2019-02-06 19:08 ` Eric Dumazet
1 sibling, 1 reply; 16+ messages in thread
From: Stefano Brivio @ 2019-02-06 19:04 UTC (permalink / raw)
To: David Ahern; +Cc: Hangbin Liu, netdev, David Miller
On Wed, 6 Feb 2019 10:54:01 -0800
David Ahern <dsahern@gmail.com> wrote:
> On 2/6/19 4:51 AM, Hangbin Liu wrote:
> > When we add a new GENEVE device with IPv6 remote, checking only for
> > IS_ENABLED(CONFIG_IPV6) is not enough as we may disable IPv6 in kernel
> > cmd(ipv6.disable=1), which will cause a NULL pointer dereference.
> >
> > Reported-by: Jianlin Shi <jishi@redhat.com>
> > Signed-off-by: Hangbin Liu <liuhangbin@gmail.com>
> > ---
> > drivers/net/geneve.c | 6 ++++++
> > 1 file changed, 6 insertions(+)
> >
> > diff --git a/drivers/net/geneve.c b/drivers/net/geneve.c
> > index 58bbba8582b0..0658715581e3 100644
> > --- a/drivers/net/geneve.c
> > +++ b/drivers/net/geneve.c
> > @@ -1512,6 +1512,10 @@ static void geneve_link_config(struct net_device *dev,
> > }
> > #if IS_ENABLED(CONFIG_IPV6)
> > case AF_INET6: {
> > + struct inet6_dev *idev = in6_dev_get(dev);
> > + if (!idev)
> > + break;
>
> Since you don't need an idev reference rcu_access_pointer should be
> enough to say v6 is enabled on this device. ie., add a helper to check
> that rcu_access_pointer(dev->ip6_ptr) is non-NULL
I guess if (__in6_dev_get(dev)) is already good, no? This is called
under RTNL.
--
Stefano
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH net 1/2] geneve: should not call rt6_lookup() when ipv6 was disabled
2019-02-06 18:54 ` David Ahern
2019-02-06 19:04 ` Stefano Brivio
@ 2019-02-06 19:08 ` Eric Dumazet
1 sibling, 0 replies; 16+ messages in thread
From: Eric Dumazet @ 2019-02-06 19:08 UTC (permalink / raw)
To: David Ahern, Hangbin Liu, netdev; +Cc: Stefano Brivio, David Miller
On 02/06/2019 10:54 AM, David Ahern wrote:
> On 2/6/19 4:51 AM, Hangbin Liu wrote:
>> When we add a new GENEVE device with IPv6 remote, checking only for
>> IS_ENABLED(CONFIG_IPV6) is not enough as we may disable IPv6 in kernel
>> cmd(ipv6.disable=1), which will cause a NULL pointer dereference.
>>
>> Reported-by: Jianlin Shi <jishi@redhat.com>
>> Signed-off-by: Hangbin Liu <liuhangbin@gmail.com>
>> ---
>> drivers/net/geneve.c | 6 ++++++
>> 1 file changed, 6 insertions(+)
>>
>> diff --git a/drivers/net/geneve.c b/drivers/net/geneve.c
>> index 58bbba8582b0..0658715581e3 100644
>> --- a/drivers/net/geneve.c
>> +++ b/drivers/net/geneve.c
>> @@ -1512,6 +1512,10 @@ static void geneve_link_config(struct net_device *dev,
>> }
>> #if IS_ENABLED(CONFIG_IPV6)
>> case AF_INET6: {
>> + struct inet6_dev *idev = in6_dev_get(dev);
>> + if (!idev)
>> + break;
>
> Since you don't need an idev reference rcu_access_pointer should be
> enough to say v6 is enabled on this device. ie., add a helper to check
> that rcu_access_pointer(dev->ip6_ptr) is non-NULL
>
I guess RTNL is held at this point, so we can use a lockdep enabled
accessor : __in6_dev_get()
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH net 1/2] geneve: should not call rt6_lookup() when ipv6 was disabled
2019-02-06 19:04 ` Stefano Brivio
@ 2019-02-06 19:09 ` Eric Dumazet
0 siblings, 0 replies; 16+ messages in thread
From: Eric Dumazet @ 2019-02-06 19:09 UTC (permalink / raw)
To: Stefano Brivio, David Ahern; +Cc: Hangbin Liu, netdev, David Miller
On 02/06/2019 11:04 AM, Stefano Brivio wrote:
> I guess if (__in6_dev_get(dev)) is already good, no? This is called
> under RTNL.
>
Exactly ;)
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH net 1/2] geneve: should not call rt6_lookup() when ipv6 was disabled
2019-02-06 12:51 ` [PATCH net 1/2] geneve: should not call rt6_lookup() when ipv6 was disabled Hangbin Liu
` (2 preceding siblings ...)
2019-02-06 18:54 ` David Ahern
@ 2019-02-07 0:55 ` kbuild test robot
3 siblings, 0 replies; 16+ messages in thread
From: kbuild test robot @ 2019-02-07 0:55 UTC (permalink / raw)
To: Hangbin Liu; +Cc: kbuild-all, netdev, Stefano Brivio, David Miller, Hangbin Liu
[-- Attachment #1: Type: text/plain, Size: 4504 bytes --]
Hi Hangbin,
Thank you for the patch! Perhaps something to improve:
[auto build test WARNING on net/master]
url: https://github.com/0day-ci/linux/commits/Hangbin-Liu/fix-two-kernel-panics-when-disabled-IPv6-on-boot-up/20190207-071954
config: m68k-sun3_defconfig (attached as .config)
compiler: m68k-linux-gnu-gcc (Debian 8.2.0-11) 8.2.0
reproduce:
wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
chmod +x ~/bin/make.cross
# save the attached .config to linux build tree
GCC_VERSION=8.2.0 make.cross ARCH=m68k
All warnings (new ones prefixed by >>):
drivers/net/geneve.c: In function 'geneve_link_config':
>> drivers/net/geneve.c:1519:3: warning: ISO C90 forbids mixed declarations and code [-Wdeclaration-after-statement]
struct rt6_info *rt = rt6_lookup(geneve->net,
^~~~~~
vim +1519 drivers/net/geneve.c
abe492b4f5 Tom Herbert 2015-12-10 1490
c40e89fd35 Alexey Kodanev 2018-04-19 1491 static void geneve_link_config(struct net_device *dev,
c40e89fd35 Alexey Kodanev 2018-04-19 1492 struct ip_tunnel_info *info, struct nlattr *tb[])
c40e89fd35 Alexey Kodanev 2018-04-19 1493 {
c40e89fd35 Alexey Kodanev 2018-04-19 1494 struct geneve_dev *geneve = netdev_priv(dev);
c40e89fd35 Alexey Kodanev 2018-04-19 1495 int ldev_mtu = 0;
c40e89fd35 Alexey Kodanev 2018-04-19 1496
c40e89fd35 Alexey Kodanev 2018-04-19 1497 if (tb[IFLA_MTU]) {
c40e89fd35 Alexey Kodanev 2018-04-19 1498 geneve_change_mtu(dev, nla_get_u32(tb[IFLA_MTU]));
c40e89fd35 Alexey Kodanev 2018-04-19 1499 return;
c40e89fd35 Alexey Kodanev 2018-04-19 1500 }
c40e89fd35 Alexey Kodanev 2018-04-19 1501
c40e89fd35 Alexey Kodanev 2018-04-19 1502 switch (ip_tunnel_info_af(info)) {
c40e89fd35 Alexey Kodanev 2018-04-19 1503 case AF_INET: {
c40e89fd35 Alexey Kodanev 2018-04-19 1504 struct flowi4 fl4 = { .daddr = info->key.u.ipv4.dst };
c40e89fd35 Alexey Kodanev 2018-04-19 1505 struct rtable *rt = ip_route_output_key(geneve->net, &fl4);
c40e89fd35 Alexey Kodanev 2018-04-19 1506
c40e89fd35 Alexey Kodanev 2018-04-19 1507 if (!IS_ERR(rt) && rt->dst.dev) {
c40e89fd35 Alexey Kodanev 2018-04-19 1508 ldev_mtu = rt->dst.dev->mtu - GENEVE_IPV4_HLEN;
c40e89fd35 Alexey Kodanev 2018-04-19 1509 ip_rt_put(rt);
c40e89fd35 Alexey Kodanev 2018-04-19 1510 }
c40e89fd35 Alexey Kodanev 2018-04-19 1511 break;
c40e89fd35 Alexey Kodanev 2018-04-19 1512 }
c40e89fd35 Alexey Kodanev 2018-04-19 1513 #if IS_ENABLED(CONFIG_IPV6)
c40e89fd35 Alexey Kodanev 2018-04-19 1514 case AF_INET6: {
55e942d018 Hangbin Liu 2019-02-06 1515 struct inet6_dev *idev = in6_dev_get(dev);
55e942d018 Hangbin Liu 2019-02-06 1516 if (!idev)
55e942d018 Hangbin Liu 2019-02-06 1517 break;
55e942d018 Hangbin Liu 2019-02-06 1518
c40e89fd35 Alexey Kodanev 2018-04-19 @1519 struct rt6_info *rt = rt6_lookup(geneve->net,
c40e89fd35 Alexey Kodanev 2018-04-19 1520 &info->key.u.ipv6.dst, NULL, 0,
c40e89fd35 Alexey Kodanev 2018-04-19 1521 NULL, 0);
c40e89fd35 Alexey Kodanev 2018-04-19 1522
c40e89fd35 Alexey Kodanev 2018-04-19 1523 if (rt && rt->dst.dev)
c40e89fd35 Alexey Kodanev 2018-04-19 1524 ldev_mtu = rt->dst.dev->mtu - GENEVE_IPV6_HLEN;
c40e89fd35 Alexey Kodanev 2018-04-19 1525 ip6_rt_put(rt);
55e942d018 Hangbin Liu 2019-02-06 1526
55e942d018 Hangbin Liu 2019-02-06 1527 in6_dev_put(idev);
c40e89fd35 Alexey Kodanev 2018-04-19 1528 break;
c40e89fd35 Alexey Kodanev 2018-04-19 1529 }
c40e89fd35 Alexey Kodanev 2018-04-19 1530 #endif
c40e89fd35 Alexey Kodanev 2018-04-19 1531 }
c40e89fd35 Alexey Kodanev 2018-04-19 1532
c40e89fd35 Alexey Kodanev 2018-04-19 1533 if (ldev_mtu <= 0)
c40e89fd35 Alexey Kodanev 2018-04-19 1534 return;
c40e89fd35 Alexey Kodanev 2018-04-19 1535
c40e89fd35 Alexey Kodanev 2018-04-19 1536 geneve_change_mtu(dev, ldev_mtu - info->options_len);
c40e89fd35 Alexey Kodanev 2018-04-19 1537 }
c40e89fd35 Alexey Kodanev 2018-04-19 1538
:::::: The code at line 1519 was first introduced by commit
:::::: c40e89fd358e94a55d6c1475afbea17b5580f601 geneve: configure MTU based on a lower device
:::::: TO: Alexey Kodanev <alexey.kodanev@oracle.com>
:::::: CC: David S. Miller <davem@davemloft.net>
---
0-DAY kernel test infrastructure Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all Intel Corporation
[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 12117 bytes --]
^ permalink raw reply [flat|nested] 16+ messages in thread
* [PATCH v2 net 0/2] fix two kernel panics when disabled IPv6 on boot up
2019-02-06 12:51 [PATCH net 0/2] fix two kernel panics when disabled IPv6 on boot up Hangbin Liu
2019-02-06 12:51 ` [PATCH net 1/2] geneve: should not call rt6_lookup() when ipv6 was disabled Hangbin Liu
2019-02-06 12:51 ` [PATCH net 2/2] sit: check if IPv6 enabled before call ip6_err_gen_icmpv6_unreach() Hangbin Liu
@ 2019-02-07 10:36 ` Hangbin Liu
2019-02-07 10:36 ` [PATCH v2 net 1/2] geneve: should not call rt6_lookup() when ipv6 was disabled Hangbin Liu
` (2 more replies)
2 siblings, 3 replies; 16+ messages in thread
From: Hangbin Liu @ 2019-02-07 10:36 UTC (permalink / raw)
To: netdev; +Cc: Stefano Brivio, David Miller, Eric Dumazet, Hangbin Liu
When disabled IPv6 on boot up, since there is no ipv6 route tables, we should
not call rt6_lookup. Fix them by checking if we have inet6_dev pointer on
netdevice.
v2: Fix idev reference leak, declarations and code mixing as Stefano,
Eric pointed. Since we only want to check if idev exists and not
reference it, use __in6_dev_get() insteand of in6_dev_get().
Hangbin Liu (2):
geneve: should not call rt6_lookup() when ipv6 was disabled
sit: check if IPv6 enabled before call ip6_err_gen_icmpv6_unreach()
drivers/net/geneve.c | 6 ++++++
net/ipv6/sit.c | 8 +++++++-
2 files changed, 13 insertions(+), 1 deletion(-)
--
2.19.2
^ permalink raw reply [flat|nested] 16+ messages in thread
* [PATCH v2 net 1/2] geneve: should not call rt6_lookup() when ipv6 was disabled
2019-02-07 10:36 ` [PATCH v2 net 0/2] fix two kernel panics when disabled IPv6 on boot up Hangbin Liu
@ 2019-02-07 10:36 ` Hangbin Liu
2019-02-07 10:36 ` [PATCH v2 net 2/2] sit: check if IPv6 enabled before calling ip6_err_gen_icmpv6_unreach() Hangbin Liu
2019-02-07 18:48 ` [PATCH v2 net 0/2] fix two kernel panics when disabled IPv6 on boot up David Miller
2 siblings, 0 replies; 16+ messages in thread
From: Hangbin Liu @ 2019-02-07 10:36 UTC (permalink / raw)
To: netdev
Cc: Stefano Brivio, David Miller, Eric Dumazet, Hangbin Liu, Alexey Kodanev
When we add a new GENEVE device with IPv6 remote, checking only for
IS_ENABLED(CONFIG_IPV6) is not enough as we may disable IPv6 in the
kernel command line (ipv6.disable=1), and calling rt6_lookup() would
cause a NULL pointer dereference.
v2:
- don't mix declarations and code (reported by Stefano Brivio, Eric Dumazet)
- there's no need to use in6_dev_get() as we only need to check that
idev exists (reported by David Ahern). This is under RTNL, so we can
simply use __in6_dev_get() instead (Stefano, Eric).
Reported-by: Jianlin Shi <jishi@redhat.com>
Fixes: c40e89fd358e9 ("geneve: configure MTU based on a lower device")
Cc: Alexey Kodanev <alexey.kodanev@oracle.com>
Signed-off-by: Hangbin Liu <liuhangbin@gmail.com>
Reviewed-by: Stefano Brivio <sbrivio@redhat.com>
---
drivers/net/geneve.c | 10 +++++++---
1 file changed, 7 insertions(+), 3 deletions(-)
diff --git a/drivers/net/geneve.c b/drivers/net/geneve.c
index 58bbba8582b0..3377ac66a347 100644
--- a/drivers/net/geneve.c
+++ b/drivers/net/geneve.c
@@ -1512,9 +1512,13 @@ static void geneve_link_config(struct net_device *dev,
}
#if IS_ENABLED(CONFIG_IPV6)
case AF_INET6: {
- struct rt6_info *rt = rt6_lookup(geneve->net,
- &info->key.u.ipv6.dst, NULL, 0,
- NULL, 0);
+ struct rt6_info *rt;
+
+ if (!__in6_dev_get(dev))
+ break;
+
+ rt = rt6_lookup(geneve->net, &info->key.u.ipv6.dst, NULL, 0,
+ NULL, 0);
if (rt && rt->dst.dev)
ldev_mtu = rt->dst.dev->mtu - GENEVE_IPV6_HLEN;
--
2.19.2
^ permalink raw reply related [flat|nested] 16+ messages in thread
* [PATCH v2 net 2/2] sit: check if IPv6 enabled before calling ip6_err_gen_icmpv6_unreach()
2019-02-07 10:36 ` [PATCH v2 net 0/2] fix two kernel panics when disabled IPv6 on boot up Hangbin Liu
2019-02-07 10:36 ` [PATCH v2 net 1/2] geneve: should not call rt6_lookup() when ipv6 was disabled Hangbin Liu
@ 2019-02-07 10:36 ` Hangbin Liu
2019-02-07 18:48 ` [PATCH v2 net 0/2] fix two kernel panics when disabled IPv6 on boot up David Miller
2 siblings, 0 replies; 16+ messages in thread
From: Hangbin Liu @ 2019-02-07 10:36 UTC (permalink / raw)
To: netdev
Cc: Stefano Brivio, David Miller, Eric Dumazet, Hangbin Liu, Oussama Ghorbel
If we disabled IPv6 from the kernel command line (ipv6.disable=1), we should
not call ip6_err_gen_icmpv6_unreach(). This:
ip link add sit1 type sit local 192.0.2.1 remote 192.0.2.2 ttl 1
ip link set sit1 up
ip addr add 198.51.100.1/24 dev sit1
ping 198.51.100.2
if IPv6 is disabled at boot time, will crash the kernel.
v2: there's no need to use in6_dev_get(), use __in6_dev_get() instead,
as we only need to check that idev exists and we are under
rcu_read_lock() (from netif_receive_skb_internal()).
Reported-by: Jianlin Shi <jishi@redhat.com>
Fixes: ca15a078bd90 ("sit: generate icmpv6 error when receiving icmpv4 error")
Cc: Oussama Ghorbel <ghorbel@pivasoftware.com>
Signed-off-by: Hangbin Liu <liuhangbin@gmail.com>
Reviewed-by: Stefano Brivio <sbrivio@redhat.com>
---
net/ipv6/sit.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/net/ipv6/sit.c b/net/ipv6/sit.c
index 1e03305c0549..e8a1dabef803 100644
--- a/net/ipv6/sit.c
+++ b/net/ipv6/sit.c
@@ -546,7 +546,8 @@ static int ipip6_err(struct sk_buff *skb, u32 info)
}
err = 0;
- if (!ip6_err_gen_icmpv6_unreach(skb, iph->ihl * 4, type, data_len))
+ if (__in6_dev_get(skb->dev) &&
+ !ip6_err_gen_icmpv6_unreach(skb, iph->ihl * 4, type, data_len))
goto out;
if (t->parms.iph.daddr == 0)
--
2.19.2
^ permalink raw reply related [flat|nested] 16+ messages in thread
* Re: [PATCH v2 net 0/2] fix two kernel panics when disabled IPv6 on boot up
2019-02-07 10:36 ` [PATCH v2 net 0/2] fix two kernel panics when disabled IPv6 on boot up Hangbin Liu
2019-02-07 10:36 ` [PATCH v2 net 1/2] geneve: should not call rt6_lookup() when ipv6 was disabled Hangbin Liu
2019-02-07 10:36 ` [PATCH v2 net 2/2] sit: check if IPv6 enabled before calling ip6_err_gen_icmpv6_unreach() Hangbin Liu
@ 2019-02-07 18:48 ` David Miller
2 siblings, 0 replies; 16+ messages in thread
From: David Miller @ 2019-02-07 18:48 UTC (permalink / raw)
To: liuhangbin; +Cc: netdev, sbrivio, eric.dumazet
From: Hangbin Liu <liuhangbin@gmail.com>
Date: Thu, 7 Feb 2019 18:36:09 +0800
> When disabled IPv6 on boot up, since there is no ipv6 route tables, we should
> not call rt6_lookup. Fix them by checking if we have inet6_dev pointer on
> netdevice.
>
> v2: Fix idev reference leak, declarations and code mixing as Stefano,
> Eric pointed. Since we only want to check if idev exists and not
> reference it, use __in6_dev_get() insteand of in6_dev_get().
Series applied and queued up for -stable, thanks.
^ permalink raw reply [flat|nested] 16+ messages in thread
end of thread, other threads:[~2019-02-07 18:49 UTC | newest]
Thread overview: 16+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-02-06 12:51 [PATCH net 0/2] fix two kernel panics when disabled IPv6 on boot up Hangbin Liu
2019-02-06 12:51 ` [PATCH net 1/2] geneve: should not call rt6_lookup() when ipv6 was disabled Hangbin Liu
2019-02-06 14:58 ` Stefano Brivio
2019-02-06 16:43 ` Eric Dumazet
2019-02-06 18:54 ` David Ahern
2019-02-06 19:04 ` Stefano Brivio
2019-02-06 19:09 ` Eric Dumazet
2019-02-06 19:08 ` Eric Dumazet
2019-02-07 0:55 ` kbuild test robot
2019-02-06 12:51 ` [PATCH net 2/2] sit: check if IPv6 enabled before call ip6_err_gen_icmpv6_unreach() Hangbin Liu
2019-02-06 15:00 ` Stefano Brivio
2019-02-06 16:45 ` Eric Dumazet
2019-02-07 10:36 ` [PATCH v2 net 0/2] fix two kernel panics when disabled IPv6 on boot up Hangbin Liu
2019-02-07 10:36 ` [PATCH v2 net 1/2] geneve: should not call rt6_lookup() when ipv6 was disabled Hangbin Liu
2019-02-07 10:36 ` [PATCH v2 net 2/2] sit: check if IPv6 enabled before calling ip6_err_gen_icmpv6_unreach() Hangbin Liu
2019-02-07 18:48 ` [PATCH v2 net 0/2] fix two kernel panics when disabled IPv6 on boot up David Miller
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.