[PATCH] ipv6:fix crash when idev is NULL

[PATCH] ipv6:fix crash when idev is NULL

[kongweibin]

When the remote device uses tc command to construct exception packages,
and send it to the local device, which acts as a forwarding device, it
will crash.

the tc cmd such as:
tc qdisc del dev vxlan100 root
tc qdisc add dev vxlan100 root netem corrupt 5%

When using dev_get_by_index_rcu to get net_device struct, once the 
package is abnormal, the corresponding net_device can't be found 
according with error device index, then return a null value, which 
value will be directly used in the policy check below, resulting in 
system crash.

Anyway, we can't directly use the idev variable. We need to ensure 
that it is a valid value.

kernel version is base on kernel-5.10.0, and the stack information 
of the crash is as follows:

[ 4484.161259] IPVS: __ip_vs_del_service: enter
[ 4484.162263] IPVS: __ip_vs_del_service: enter
[ 4686.564468] Unable to handle kernel NULL pointer dereference at virtual address 0000000000000240
[ 4686.565109] Mem abort info:
[ 4686.565328]   ESR = 0x96000004
[ 4686.565564]   EC = 0x25: DABT (current EL), IL = 32 bits
[ 4686.565948]   SET = 0, FnV = 0
[ 4686.566184]   EA = 0, S1PTW = 0
[ 4686.566427] Data abort info:
[ 4686.566651]   ISV = 0, ISS = 0x00000004
[ 4686.567024]   CM = 0, WnR = 0
[ 4686.567261] user pgtable: 4k pages, 48-bit VAs, pgdp=0000000102daa000
[ 4686.567708] [0000000000000240] pgd=0000000000000000, p4d=0000000000000000
[ 4686.568182] Internal error: Oops: 96000004 [#1] SMP
[ 4686.568530] CPU: 1 PID: 0 Comm: swapper/1 Kdump: loaded Tainted: G        W  O      5.10.0-xxxxxx.aarch64 #1
[ 4686.569316] Hardware name: QEMU KVM Virtual Machine, BIOS 0.0.0 02/06/2015
[ 4686.569787] pstate: 40400005 (nZcv daif +PAN -UAO -TCO BTYPE=--)
[ 4686.570214] pc : ip6_forward+0xb4/0x744
[ 4686.570499] lr : ip6_forward+0x5c/0x744
[ 4686.570782] sp : ffff80008800ba00
[ 4686.571098] x29: ffff80008800ba00 x28: ffff0000c02e39c0
[ 4686.571560] x27: ffff0000f6e97000 x26: ffff800089cfa500
[ 4686.572021] x25: ffff80008800bc98 x24: ffff80008800bc08
[ 4686.572487] x23: ffff800089cfa500 x22: ffff0000cbfd6c94
[ 4686.572953] x21: 0000000000000000 x20: ffff80008800bb38
[ 4686.573416] x19: ffff0000c995fc00 x18: 0000000000000000
[ 4686.573882] x17: 0000000000000000 x16: ffff8000881b65c0
[ 4686.574350] x15: 0000000000000000 x14: 0000000000000000
[ 4686.574816] x13: 0000000065f01475 x12: 0000000002cc68fd
[ 4686.575298] x11: 00000000d44127a3 x10: b181f30000000000
[ 4686.575760] x9 : ffff800088d5d9cc x8 : ffff0000c02e39c0
[ 4686.576224] x7 : 0000000000000000 x6 : 0000000000000000
[ 4686.576686] x5 : ffff0000c995fc00 x4 : ffff80008800bb38
[ 4686.577148] x3 : 0000000000000000 x2 : ffff0000cbfd6ec0
[ 4686.577609] x1 : 0000000000000000 x0 : 0000000000000000
[ 4686.578079] Call trace:
[ 4686.578323]  ip6_forward+0xb4/0x744
[ 4686.578646]  ip6_sublist_rcv_finish+0x6c/0x90
[ 4686.579051]  ip6_list_rcv_finish.constprop.0+0x198/0x260
[ 4686.579512]  ip6_sublist_rcv+0x40/0xb0
[ 4686.579852]  ipv6_list_rcv+0x144/0x180
[ 4686.580197]  __netif_receive_skb_list_core+0x154/0x28c
[ 4686.580643]  __netif_receive_skb_list+0x120/0x1a0
[ 4686.581057]  netif_receive_skb_list_internal+0xe4/0x1f0
[ 4686.581508]  napi_complete_done+0x70/0x1f0
[ 4686.581883]  virtnet_poll+0x214/0x2b0 [virtio_net]
[ 4686.582309]  napi_poll+0xcc/0x264
[ 4686.582617]  net_rx_action+0xd4/0x21c
[ 4686.582969]  __do_softirq+0x130/0x358
[ 4686.583308]  irq_exit+0x12c/0x150
[ 4686.583621]  __handle_domain_irq+0x88/0xf0
[ 4686.583991]  gic_handle_irq+0x78/0x2c0
[ 4686.584332]  el1_irq+0xc8/0x180
[ 4686.584628]  arch_cpu_idle+0x18/0x40
[ 4686.584960]  default_idle_call+0x5c/0x1c0
[ 4686.585323]  cpuidle_idle_call+0x174/0x1b0
[ 4686.585690]  do_idle+0xc8/0x160
[ 4686.585989]  cpu_startup_entry+0x30/0x10c
[ 4686.586351]  secondary_start_kernel+0x158/0x1e4
[ 4686.586754] Code: b9401842 34002ce2 b940d021 35000281 (b94242a1)
[ 4686.587301] kernel fault(0x1) notification starting on CPU 1
[ 4686.587787] kernel fault(0x1) notification finished on CPU 1

Signed-off-by: kongweibin <kongweibin2@huawei.com>
---
 net/ipv6/ip6_output.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/net/ipv6/ip6_output.c b/net/ipv6/ip6_output.c
index 54cabf1c2..347b5600d 100644
--- a/net/ipv6/ip6_output.c
+++ b/net/ipv6/ip6_output.c
@@ -495,6 +495,9 @@ int ip6_forward(struct sk_buff *skb)
 	u32 mtu;
 
 	idev = __in6_dev_get_safely(dev_get_by_index_rcu(net, IP6CB(skb)->iif));
+	if (!idev)
+		goto drop;
+
 	if (net->ipv6.devconf_all->forwarding == 0)
 		goto error;
 
-- 
2.23.0

Re: [PATCH] ipv6:fix crash when idev is NULL

[Eric Dumazet]

On Thu, Apr 7, 2022 at 4:26 AM kongweibin <kongweibin2@huawei.com> wrote:
>
> When the remote device uses tc command to construct exception packages,
> and send it to the local device, which acts as a forwarding device, it
> will crash.
>
> the tc cmd such as:
> tc qdisc del dev vxlan100 root
> tc qdisc add dev vxlan100 root netem corrupt 5%

Probably not related to your fix.

>
> When using dev_get_by_index_rcu to get net_device struct, once the
> package is abnormal, the corresponding net_device can't be found
> according with error device index, then return a null value, which
> value will be directly used in the policy check below, resulting in
> system crash.
>
> Anyway, we can't directly use the idev variable. We need to ensure
> that it is a valid value.
>
> kernel version is base on kernel-5.10.0, and the stack information
> of the crash is as follows:
>
> [ 4484.161259] IPVS: __ip_vs_del_service: enter
> [ 4484.162263] IPVS: __ip_vs_del_service: enter
> [ 4686.564468] Unable to handle kernel NULL pointer dereference at virtual address 0000000000000240
> [ 4686.565109] Mem abort info:
> [ 4686.565328]   ESR = 0x96000004
> [ 4686.565564]   EC = 0x25: DABT (current EL), IL = 32 bits
> [ 4686.565948]   SET = 0, FnV = 0
> [ 4686.566184]   EA = 0, S1PTW = 0
> [ 4686.566427] Data abort info:
> [ 4686.566651]   ISV = 0, ISS = 0x00000004
> [ 4686.567024]   CM = 0, WnR = 0
> [ 4686.567261] user pgtable: 4k pages, 48-bit VAs, pgdp=0000000102daa000
> [ 4686.567708] [0000000000000240] pgd=0000000000000000, p4d=0000000000000000
> [ 4686.568182] Internal error: Oops: 96000004 [#1] SMP
> [ 4686.568530] CPU: 1 PID: 0 Comm: swapper/1 Kdump: loaded Tainted: G        W  O      5.10.0-xxxxxx.aarch64 #1
> [ 4686.569316] Hardware name: QEMU KVM Virtual Machine, BIOS 0.0.0 02/06/2015
> [ 4686.569787] pstate: 40400005 (nZcv daif +PAN -UAO -TCO BTYPE=--)
> [ 4686.570214] pc : ip6_forward+0xb4/0x744
> [ 4686.570499] lr : ip6_forward+0x5c/0x744
> [ 4686.570782] sp : ffff80008800ba00
> [ 4686.571098] x29: ffff80008800ba00 x28: ffff0000c02e39c0
> [ 4686.571560] x27: ffff0000f6e97000 x26: ffff800089cfa500
> [ 4686.572021] x25: ffff80008800bc98 x24: ffff80008800bc08
> [ 4686.572487] x23: ffff800089cfa500 x22: ffff0000cbfd6c94
> [ 4686.572953] x21: 0000000000000000 x20: ffff80008800bb38
> [ 4686.573416] x19: ffff0000c995fc00 x18: 0000000000000000
> [ 4686.573882] x17: 0000000000000000 x16: ffff8000881b65c0
> [ 4686.574350] x15: 0000000000000000 x14: 0000000000000000
> [ 4686.574816] x13: 0000000065f01475 x12: 0000000002cc68fd
> [ 4686.575298] x11: 00000000d44127a3 x10: b181f30000000000
> [ 4686.575760] x9 : ffff800088d5d9cc x8 : ffff0000c02e39c0
> [ 4686.576224] x7 : 0000000000000000 x6 : 0000000000000000
> [ 4686.576686] x5 : ffff0000c995fc00 x4 : ffff80008800bb38
> [ 4686.577148] x3 : 0000000000000000 x2 : ffff0000cbfd6ec0
> [ 4686.577609] x1 : 0000000000000000 x0 : 0000000000000000
> [ 4686.578079] Call trace:
> [ 4686.578323]  ip6_forward+0xb4/0x744
> [ 4686.578646]  ip6_sublist_rcv_finish+0x6c/0x90
> [ 4686.579051]  ip6_list_rcv_finish.constprop.0+0x198/0x260
> [ 4686.579512]  ip6_sublist_rcv+0x40/0xb0
> [ 4686.579852]  ipv6_list_rcv+0x144/0x180
> [ 4686.580197]  __netif_receive_skb_list_core+0x154/0x28c
> [ 4686.580643]  __netif_receive_skb_list+0x120/0x1a0
> [ 4686.581057]  netif_receive_skb_list_internal+0xe4/0x1f0
> [ 4686.581508]  napi_complete_done+0x70/0x1f0
> [ 4686.581883]  virtnet_poll+0x214/0x2b0 [virtio_net]
> [ 4686.582309]  napi_poll+0xcc/0x264
> [ 4686.582617]  net_rx_action+0xd4/0x21c
> [ 4686.582969]  __do_softirq+0x130/0x358
> [ 4686.583308]  irq_exit+0x12c/0x150
> [ 4686.583621]  __handle_domain_irq+0x88/0xf0
> [ 4686.583991]  gic_handle_irq+0x78/0x2c0
> [ 4686.584332]  el1_irq+0xc8/0x180
> [ 4686.584628]  arch_cpu_idle+0x18/0x40
> [ 4686.584960]  default_idle_call+0x5c/0x1c0
> [ 4686.585323]  cpuidle_idle_call+0x174/0x1b0
> [ 4686.585690]  do_idle+0xc8/0x160
> [ 4686.585989]  cpu_startup_entry+0x30/0x10c
> [ 4686.586351]  secondary_start_kernel+0x158/0x1e4
> [ 4686.586754] Code: b9401842 34002ce2 b940d021 35000281 (b94242a1)
> [ 4686.587301] kernel fault(0x1) notification starting on CPU 1
> [ 4686.587787] kernel fault(0x1) notification finished on CPU 1
>
> Signed-off-by: kongweibin <kongweibin2@huawei.com>

Always provide a Fixes: tag for fixes.

And CC patch author for feedback.

In this case I suspect:

commit ccd27f05ae7b8ebc40af5b004e94517a919aa862
Author: Nicolas Dichtel <nicolas.dichtel@6wind.com>
Date:   Tue Jul 6 11:13:35 2021 +0200

    ipv6: fix 'disable_policy' for fwd packets



> ---
>  net/ipv6/ip6_output.c | 3 +++
>  1 file changed, 3 insertions(+)
>
> diff --git a/net/ipv6/ip6_output.c b/net/ipv6/ip6_output.c
> index 54cabf1c2..347b5600d 100644
> --- a/net/ipv6/ip6_output.c
> +++ b/net/ipv6/ip6_output.c
> @@ -495,6 +495,9 @@ int ip6_forward(struct sk_buff *skb)
>         u32 mtu;
>
>         idev = __in6_dev_get_safely(dev_get_by_index_rcu(net, IP6CB(skb)->iif));
> +       if (!idev)
> +               goto drop;
> +
>         if (net->ipv6.devconf_all->forwarding == 0)
>                 goto error;
>
> --
> 2.23.0
>

Re: [PATCH] ipv6:fix crash when idev is NULL

[Nicolas Dichtel]

Le 07/04/2022 à 16:08, Eric Dumazet a écrit :
[snip]
> 
> And CC patch author for feedback.
Thanks Eric.

> 
> In this case I suspect:
> 
> commit ccd27f05ae7b8ebc40af5b004e94517a919aa862
> Author: Nicolas Dichtel <nicolas.dichtel@6wind.com>
> Date:   Tue Jul 6 11:13:35 2021 +0200
> 
>     ipv6: fix 'disable_policy' for fwd packets
I agree.

> 
> 
> 
>> ---
>>  net/ipv6/ip6_output.c | 3 +++
>>  1 file changed, 3 insertions(+)
>>
>> diff --git a/net/ipv6/ip6_output.c b/net/ipv6/ip6_output.c
>> index 54cabf1c2..347b5600d 100644
>> --- a/net/ipv6/ip6_output.c
>> +++ b/net/ipv6/ip6_output.c
>> @@ -495,6 +495,9 @@ int ip6_forward(struct sk_buff *skb)
>>         u32 mtu;
>>
>>         idev = __in6_dev_get_safely(dev_get_by_index_rcu(net, IP6CB(skb)->iif));
>> +       if (!idev)
>> +               goto drop;
>> +
>>         if (net->ipv6.devconf_all->forwarding == 0)
>>                 goto error;

Dropping packet in this case may introduce another regression, because there was
no drop before commit ccd27f05ae7b ("ipv6: fix 'disable_policy' for fwd packets").

Maybe something like this:
--- a/net/ipv6/ip6_output.c
+++ b/net/ipv6/ip6_output.c
@@ -485,7 +485,7 @@ int ip6_forward(struct sk_buff *skb)
                goto drop;

        if (!net->ipv6.devconf_all->disable_policy &&
-           !idev->cnf.disable_policy &&
+           (!idev || !idev->cnf.disable_policy) &&
            !xfrm6_policy_check(NULL, XFRM_POLICY_FWD, skb)) {
                __IP6_INC_STATS(net, idev, IPSTATS_MIB_INDISCARDS);
                goto drop;

I could submit it formally tomorrow.


Regards,
Nicolas

[PATCH net] ipv6: fix panic when forwarding a pkt with no in6 dev

[Nicolas Dichtel]

kongweibin reported a kernel panic in ip6_forward() when input interface
has no in6 dev associated.

The following tc commands were used to reproduce this panic:
tc qdisc del dev vxlan100 root
tc qdisc add dev vxlan100 root netem corrupt 5%

CC: stable@vger.kernel.org
Fixes: ccd27f05ae7b ("ipv6: fix 'disable_policy' for fwd packets")
Reported-by: kongweibin <kongweibin2@huawei.com>
Signed-off-by: Nicolas Dichtel <nicolas.dichtel@6wind.com>
---

kongweibin, could you test this patch with your setup?

Thanks,
Nicolas

 net/ipv6/ip6_output.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/net/ipv6/ip6_output.c b/net/ipv6/ip6_output.c
index e23f058166af..fa63ef2bd99c 100644
--- a/net/ipv6/ip6_output.c
+++ b/net/ipv6/ip6_output.c
@@ -485,7 +485,7 @@ int ip6_forward(struct sk_buff *skb)
 		goto drop;
 
 	if (!net->ipv6.devconf_all->disable_policy &&
-	    !idev->cnf.disable_policy &&
+	    (!idev || !idev->cnf.disable_policy) &&
 	    !xfrm6_policy_check(NULL, XFRM_POLICY_FWD, skb)) {
 		__IP6_INC_STATS(net, idev, IPSTATS_MIB_INDISCARDS);
 		goto drop;
-- 
2.33.0

Re: [PATCH net] ipv6: fix panic when forwarding a pkt with no in6 dev

[Eric Dumazet]

On 4/8/22 07:03, Nicolas Dichtel wrote:
> kongweibin reported a kernel panic in ip6_forward() when input interface
> has no in6 dev associated.
>
> The following tc commands were used to reproduce this panic:
> tc qdisc del dev vxlan100 root
> tc qdisc add dev vxlan100 root netem corrupt 5%

Not sure I understand how these qdisc changes can trigger a NULL idev ?

Do we have another bug, like skb->cb[] content being mangled ?


>
> CC: stable@vger.kernel.org
> Fixes: ccd27f05ae7b ("ipv6: fix 'disable_policy' for fwd packets")
> Reported-by: kongweibin <kongweibin2@huawei.com>
> Signed-off-by: Nicolas Dichtel <nicolas.dichtel@6wind.com>
> ---
>
> kongweibin, could you test this patch with your setup?
>
> Thanks,
> Nicolas
>
>   net/ipv6/ip6_output.c | 2 +-
>   1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/net/ipv6/ip6_output.c b/net/ipv6/ip6_output.c
> index e23f058166af..fa63ef2bd99c 100644
> --- a/net/ipv6/ip6_output.c
> +++ b/net/ipv6/ip6_output.c
> @@ -485,7 +485,7 @@ int ip6_forward(struct sk_buff *skb)
>   		goto drop;
>   
>   	if (!net->ipv6.devconf_all->disable_policy &&
> -	    !idev->cnf.disable_policy &&
> +	    (!idev || !idev->cnf.disable_policy) &&
>   	    !xfrm6_policy_check(NULL, XFRM_POLICY_FWD, skb)) {
>   		__IP6_INC_STATS(net, idev, IPSTATS_MIB_INDISCARDS);
>   		goto drop;

答复: [PATCH net] ipv6: fix panic when forwarding a pkt with no in6 dev

[kongweibin (A)]

> kongweibin, could you test this patch with your setup?
>
> Thanks,
> Nicolas
>
>   net/ipv6/ip6_output.c | 2 +-
>   1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/net/ipv6/ip6_output.c b/net/ipv6/ip6_output.c index 
> e23f058166af..fa63ef2bd99c 100644
> --- a/net/ipv6/ip6_output.c
> +++ b/net/ipv6/ip6_output.c
> @@ -485,7 +485,7 @@ int ip6_forward(struct sk_buff *skb)
>   		goto drop;
>   
>   	if (!net->ipv6.devconf_all->disable_policy &&
> -	    !idev->cnf.disable_policy &&
> +	    (!idev || !idev->cnf.disable_policy) &&
>   	    !xfrm6_policy_check(NULL, XFRM_POLICY_FWD, skb)) {
>   		__IP6_INC_STATS(net, idev, IPSTATS_MIB_INDISCARDS);
>   		goto drop;

I have test the patch with my setup, it is OK.



>> kongweibin reported a kernel panic in ip6_forward() when input 
>> interface has no in6 dev associated.
>>
>> The following tc commands were used to reproduce this panic:
>> tc qdisc del dev vxlan100 root
>> tc qdisc add dev vxlan100 root netem corrupt 5%
>
>Not sure I understand how these qdisc changes can trigger a NULL idev ?
>
>Do we have another bug, like skb->cb[] content being mangled ?
>

As for why qdisc makes the idev null, I tracked where the iif assigned in ip6_rcv_core,
there is no problem there.

Not sure what has changed that makes the iif value wrong.

Re: [PATCH net] ipv6: fix panic when forwarding a pkt with no in6 dev

[David Ahern]

On 4/8/22 8:03 AM, Nicolas Dichtel wrote:
> kongweibin reported a kernel panic in ip6_forward() when input interface
> has no in6 dev associated.
> 
> The following tc commands were used to reproduce this panic:
> tc qdisc del dev vxlan100 root
> tc qdisc add dev vxlan100 root netem corrupt 5%
> 
> CC: stable@vger.kernel.org
> Fixes: ccd27f05ae7b ("ipv6: fix 'disable_policy' for fwd packets")
> Reported-by: kongweibin <kongweibin2@huawei.com>
> Signed-off-by: Nicolas Dichtel <nicolas.dichtel@6wind.com>
> ---
> 

Reviewed-by: David Ahern <dsahern@kernel.org>

Re: [PATCH net] ipv6: fix panic when forwarding a pkt with no in6 dev

[patchwork-bot+netdevbpf]

Hello:

This patch was applied to netdev/net.git (master)
by David S. Miller <davem@davemloft.net>:

On Fri,  8 Apr 2022 16:03:42 +0200 you wrote:
> kongweibin reported a kernel panic in ip6_forward() when input interface
> has no in6 dev associated.
> 
> The following tc commands were used to reproduce this panic:
> tc qdisc del dev vxlan100 root
> tc qdisc add dev vxlan100 root netem corrupt 5%
> 
> [...]

Here is the summary with links:
  - [net] ipv6: fix panic when forwarding a pkt with no in6 dev
    https://git.kernel.org/netdev/net/c/e3fa461d8b0e

You are awesome, thank you!
-- 
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html