[PATCH net 0/2] fix two kernel panics when disabled IPv6 on boot up

[PATCH net 0/2] fix two kernel panics when disabled IPv6 on boot up

[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

[PATCH net 1/2] geneve: should not call rt6_lookup() when ipv6 was disabled

[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

[PATCH net 2/2] sit: check if IPv6 enabled before call ip6_err_gen_icmpv6_unreach()

[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

Re: [PATCH net 1/2] geneve: should not call rt6_lookup() when ipv6 was disabled

[Stefano Brivio]

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

Re: [PATCH net 2/2] sit: check if IPv6 enabled before call ip6_err_gen_icmpv6_unreach()

[Stefano Brivio]

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

Re: [PATCH net 1/2] geneve: should not call rt6_lookup() when ipv6 was disabled

[Eric Dumazet]

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
> 

Re: [PATCH net 2/2] sit: check if IPv6 enabled before call ip6_err_gen_icmpv6_unreach()

[Eric Dumazet]

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 ?

Re: [PATCH net 1/2] geneve: should not call rt6_lookup() when ipv6 was disabled

[David Ahern]

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

Re: [PATCH net 1/2] geneve: should not call rt6_lookup() when ipv6 was disabled

[Stefano Brivio]

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

Re: [PATCH net 1/2] geneve: should not call rt6_lookup() when ipv6 was disabled

[Eric Dumazet]

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

Re: [PATCH net 1/2] geneve: should not call rt6_lookup() when ipv6 was disabled

[Eric Dumazet]

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

Re: [PATCH net 1/2] geneve: should not call rt6_lookup() when ipv6 was disabled

[kbuild test robot]

[-- 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 --]

[PATCH v2 net 0/2] fix two kernel panics when disabled IPv6 on boot up

[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

[PATCH v2 net 1/2] geneve: should not call rt6_lookup() when ipv6 was disabled

[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 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

[PATCH v2 net 2/2] sit: check if IPv6 enabled before calling ip6_err_gen_icmpv6_unreach()

[Hangbin Liu]

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

Re: [PATCH v2 net 0/2] fix two kernel panics when disabled IPv6 on boot up

[David Miller]

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.