On Thu, Apr 7, 2022 at 4:26 AM kongweibin <[email protected]> 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 <[email protected]>
Always provide a Fixes: tag for fixes.
And CC patch author for feedback.
In this case I suspect:
commit ccd27f05ae7b8ebc40af5b004e94517a919aa862
Author: Nicolas Dichtel <[email protected]>
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
>
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 <[email protected]>
> 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
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: [email protected]
Fixes: ccd27f05ae7b ("ipv6: fix 'disable_policy' for fwd packets")
Reported-by: kongweibin <[email protected]>
Signed-off-by: Nicolas Dichtel <[email protected]>
---
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
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: [email protected]
> Fixes: ccd27f05ae7b ("ipv6: fix 'disable_policy' for fwd packets")
> Reported-by: kongweibin <[email protected]>
> Signed-off-by: Nicolas Dichtel <[email protected]>
> ---
>
> 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;
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: [email protected]
> Fixes: ccd27f05ae7b ("ipv6: fix 'disable_policy' for fwd packets")
> Reported-by: kongweibin <[email protected]>
> Signed-off-by: Nicolas Dichtel <[email protected]>
> ---
>
Reviewed-by: David Ahern <[email protected]>
Hello:
This patch was applied to netdev/net.git (master)
by David S. Miller <[email protected]>:
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