* [PATCH v3] net: use sock_gen_put() when sk_state is TCP_TIME_WAIT
@ 2025-04-29 1:59 Shiming Cheng
2025-04-30 20:56 ` Jacob Keller
2025-05-01 14:10 ` patchwork-bot+netdevbpf
0 siblings, 2 replies; 3+ messages in thread
From: Shiming Cheng @ 2025-04-29 1:59 UTC (permalink / raw)
To: edumazet, davem, dsahern, kuba, pabeni, horms, matthias.bgg,
angelogioacchino.delregno, netdev, linux-kernel, linux-arm-kernel,
linux-mediatek
Cc: shiming.cheng, lena.wang, Jibin Zhang
From: Jibin Zhang <jibin.zhang@mediatek.com>
It is possible for a pointer of type struct inet_timewait_sock to be
returned from the functions __inet_lookup_established() and
__inet6_lookup_established(). This can cause a crash when the
returned pointer is of type struct inet_timewait_sock and
sock_put() is called on it. The following is a crash call stack that
shows sk->sk_wmem_alloc being accessed in sk_free() during the call to
sock_put() on a struct inet_timewait_sock pointer. To avoid this issue,
use sock_gen_put() instead of sock_put() when sk->sk_state
is TCP_TIME_WAIT.
mrdump.ko ipanic() + 120
vmlinux notifier_call_chain(nr_to_call=-1, nr_calls=0) + 132
vmlinux atomic_notifier_call_chain(val=0) + 56
vmlinux panic() + 344
vmlinux add_taint() + 164
vmlinux end_report() + 136
vmlinux kasan_report(size=0) + 236
vmlinux report_tag_fault() + 16
vmlinux do_tag_recovery() + 16
vmlinux __do_kernel_fault() + 88
vmlinux do_bad_area() + 28
vmlinux do_tag_check_fault() + 60
vmlinux do_mem_abort() + 80
vmlinux el1_abort() + 56
vmlinux el1h_64_sync_handler() + 124
vmlinux > 0xFFFFFFC080011294()
vmlinux __lse_atomic_fetch_add_release(v=0xF2FFFF82A896087C)
vmlinux __lse_atomic_fetch_sub_release(v=0xF2FFFF82A896087C)
vmlinux arch_atomic_fetch_sub_release(i=1, v=0xF2FFFF82A896087C)
+ 8
vmlinux raw_atomic_fetch_sub_release(i=1, v=0xF2FFFF82A896087C)
+ 8
vmlinux atomic_fetch_sub_release(i=1, v=0xF2FFFF82A896087C) + 8
vmlinux __refcount_sub_and_test(i=1, r=0xF2FFFF82A896087C,
oldp=0) + 8
vmlinux __refcount_dec_and_test(r=0xF2FFFF82A896087C, oldp=0) + 8
vmlinux refcount_dec_and_test(r=0xF2FFFF82A896087C) + 8
vmlinux sk_free(sk=0xF2FFFF82A8960700) + 28
vmlinux sock_put() + 48
vmlinux tcp6_check_fraglist_gro() + 236
vmlinux tcp6_gro_receive() + 624
vmlinux ipv6_gro_receive() + 912
vmlinux dev_gro_receive() + 1116
vmlinux napi_gro_receive() + 196
ccmni.ko ccmni_rx_callback() + 208
ccmni.ko ccmni_queue_recv_skb() + 388
ccci_dpmaif.ko dpmaif_rxq_push_thread() + 1088
vmlinux kthread() + 268
vmlinux 0xFFFFFFC08001F30C()
Fixes: c9d1d23e5239 ("net: add heuristic for enabling TCP fraglist GRO")
Signed-off-by: Jibin Zhang <jibin.zhang@mediatek.com>
Signed-off-by: Shiming Cheng <shiming.cheng@mediatek.com>
---
net/ipv4/tcp_offload.c | 2 +-
net/ipv6/tcpv6_offload.c | 2 +-
2 files changed, 2 insertions(+), 2 deletions(-)
diff --git a/net/ipv4/tcp_offload.c b/net/ipv4/tcp_offload.c
index 2308665b51c5..f55026b597ff 100644
--- a/net/ipv4/tcp_offload.c
+++ b/net/ipv4/tcp_offload.c
@@ -432,7 +432,7 @@ static void tcp4_check_fraglist_gro(struct list_head *head, struct sk_buff *skb,
iif, sdif);
NAPI_GRO_CB(skb)->is_flist = !sk;
if (sk)
- sock_put(sk);
+ sock_gen_put(sk);
}
INDIRECT_CALLABLE_SCOPE
diff --git a/net/ipv6/tcpv6_offload.c b/net/ipv6/tcpv6_offload.c
index a45bf17cb2a1..b1f284e0c15a 100644
--- a/net/ipv6/tcpv6_offload.c
+++ b/net/ipv6/tcpv6_offload.c
@@ -42,7 +42,7 @@ static void tcp6_check_fraglist_gro(struct list_head *head, struct sk_buff *skb,
iif, sdif);
NAPI_GRO_CB(skb)->is_flist = !sk;
if (sk)
- sock_put(sk);
+ sock_gen_put(sk);
#endif /* IS_ENABLED(CONFIG_IPV6) */
}
--
2.45.2
^ permalink raw reply related [flat|nested] 3+ messages in thread* Re: [PATCH v3] net: use sock_gen_put() when sk_state is TCP_TIME_WAIT
2025-04-29 1:59 [PATCH v3] net: use sock_gen_put() when sk_state is TCP_TIME_WAIT Shiming Cheng
@ 2025-04-30 20:56 ` Jacob Keller
2025-05-01 14:10 ` patchwork-bot+netdevbpf
1 sibling, 0 replies; 3+ messages in thread
From: Jacob Keller @ 2025-04-30 20:56 UTC (permalink / raw)
To: Shiming Cheng, edumazet, davem, dsahern, kuba, pabeni, horms,
matthias.bgg, angelogioacchino.delregno, netdev, linux-kernel,
linux-arm-kernel, linux-mediatek
Cc: lena.wang, Jibin Zhang
On 4/28/2025 6:59 PM, Shiming Cheng wrote:
> From: Jibin Zhang <jibin.zhang@mediatek.com>
>
> It is possible for a pointer of type struct inet_timewait_sock to be
> returned from the functions __inet_lookup_established() and
> __inet6_lookup_established(). This can cause a crash when the
> returned pointer is of type struct inet_timewait_sock and
> sock_put() is called on it. The following is a crash call stack that
> shows sk->sk_wmem_alloc being accessed in sk_free() during the call to
> sock_put() on a struct inet_timewait_sock pointer. To avoid this issue,
> use sock_gen_put() instead of sock_put() when sk->sk_state
> is TCP_TIME_WAIT.
>
> mrdump.ko ipanic() + 120
> vmlinux notifier_call_chain(nr_to_call=-1, nr_calls=0) + 132
> vmlinux atomic_notifier_call_chain(val=0) + 56
> vmlinux panic() + 344
> vmlinux add_taint() + 164
> vmlinux end_report() + 136
> vmlinux kasan_report(size=0) + 236
> vmlinux report_tag_fault() + 16
> vmlinux do_tag_recovery() + 16
> vmlinux __do_kernel_fault() + 88
> vmlinux do_bad_area() + 28
> vmlinux do_tag_check_fault() + 60
> vmlinux do_mem_abort() + 80
> vmlinux el1_abort() + 56
> vmlinux el1h_64_sync_handler() + 124
> vmlinux > 0xFFFFFFC080011294()
> vmlinux __lse_atomic_fetch_add_release(v=0xF2FFFF82A896087C)
> vmlinux __lse_atomic_fetch_sub_release(v=0xF2FFFF82A896087C)
> vmlinux arch_atomic_fetch_sub_release(i=1, v=0xF2FFFF82A896087C)
> + 8
> vmlinux raw_atomic_fetch_sub_release(i=1, v=0xF2FFFF82A896087C)
> + 8
> vmlinux atomic_fetch_sub_release(i=1, v=0xF2FFFF82A896087C) + 8
> vmlinux __refcount_sub_and_test(i=1, r=0xF2FFFF82A896087C,
> oldp=0) + 8
> vmlinux __refcount_dec_and_test(r=0xF2FFFF82A896087C, oldp=0) + 8
> vmlinux refcount_dec_and_test(r=0xF2FFFF82A896087C) + 8
> vmlinux sk_free(sk=0xF2FFFF82A8960700) + 28
> vmlinux sock_put() + 48
> vmlinux tcp6_check_fraglist_gro() + 236
> vmlinux tcp6_gro_receive() + 624
> vmlinux ipv6_gro_receive() + 912
> vmlinux dev_gro_receive() + 1116
> vmlinux napi_gro_receive() + 196
> ccmni.ko ccmni_rx_callback() + 208
> ccmni.ko ccmni_queue_recv_skb() + 388
> ccci_dpmaif.ko dpmaif_rxq_push_thread() + 1088
> vmlinux kthread() + 268
> vmlinux 0xFFFFFFC08001F30C()
>
> Fixes: c9d1d23e5239 ("net: add heuristic for enabling TCP fraglist GRO")
> Signed-off-by: Jibin Zhang <jibin.zhang@mediatek.com>
> Signed-off-by: Shiming Cheng <shiming.cheng@mediatek.com>
> ---
The code change looks good to me.
Reviewed-by: Jacob Keller <jacob.e.keller@intel.com>
One note is that it helps reviewers if you could include a terse summary
of the changes between versions. This is marked as v3, but there's no
mention of what changed.
> net/ipv4/tcp_offload.c | 2 +-
> net/ipv6/tcpv6_offload.c | 2 +-
> 2 files changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/net/ipv4/tcp_offload.c b/net/ipv4/tcp_offload.c
> index 2308665b51c5..f55026b597ff 100644
> --- a/net/ipv4/tcp_offload.c
> +++ b/net/ipv4/tcp_offload.c
> @@ -432,7 +432,7 @@ static void tcp4_check_fraglist_gro(struct list_head *head, struct sk_buff *skb,
> iif, sdif);
> NAPI_GRO_CB(skb)->is_flist = !sk;
> if (sk)
> - sock_put(sk);
> + sock_gen_put(sk);
> }
>
> INDIRECT_CALLABLE_SCOPE
> diff --git a/net/ipv6/tcpv6_offload.c b/net/ipv6/tcpv6_offload.c
> index a45bf17cb2a1..b1f284e0c15a 100644
> --- a/net/ipv6/tcpv6_offload.c
> +++ b/net/ipv6/tcpv6_offload.c
> @@ -42,7 +42,7 @@ static void tcp6_check_fraglist_gro(struct list_head *head, struct sk_buff *skb,
> iif, sdif);
> NAPI_GRO_CB(skb)->is_flist = !sk;
> if (sk)
> - sock_put(sk);
> + sock_gen_put(sk);
> #endif /* IS_ENABLED(CONFIG_IPV6) */
> }
>
^ permalink raw reply [flat|nested] 3+ messages in thread* Re: [PATCH v3] net: use sock_gen_put() when sk_state is TCP_TIME_WAIT
2025-04-29 1:59 [PATCH v3] net: use sock_gen_put() when sk_state is TCP_TIME_WAIT Shiming Cheng
2025-04-30 20:56 ` Jacob Keller
@ 2025-05-01 14:10 ` patchwork-bot+netdevbpf
1 sibling, 0 replies; 3+ messages in thread
From: patchwork-bot+netdevbpf @ 2025-05-01 14:10 UTC (permalink / raw)
To: Shiming Cheng
Cc: edumazet, davem, dsahern, kuba, pabeni, horms, matthias.bgg,
angelogioacchino.delregno, netdev, linux-kernel, linux-arm-kernel,
linux-mediatek, lena.wang, jibin.zhang
Hello:
This patch was applied to netdev/net.git (main)
by Jakub Kicinski <kuba@kernel.org>:
On Tue, 29 Apr 2025 09:59:48 +0800 you wrote:
> From: Jibin Zhang <jibin.zhang@mediatek.com>
>
> It is possible for a pointer of type struct inet_timewait_sock to be
> returned from the functions __inet_lookup_established() and
> __inet6_lookup_established(). This can cause a crash when the
> returned pointer is of type struct inet_timewait_sock and
> sock_put() is called on it. The following is a crash call stack that
> shows sk->sk_wmem_alloc being accessed in sk_free() during the call to
> sock_put() on a struct inet_timewait_sock pointer. To avoid this issue,
> use sock_gen_put() instead of sock_put() when sk->sk_state
> is TCP_TIME_WAIT.
>
> [...]
Here is the summary with links:
- [v3] net: use sock_gen_put() when sk_state is TCP_TIME_WAIT
https://git.kernel.org/netdev/net/c/f920436a4429
You are awesome, thank you!
--
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2025-05-01 14:11 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-04-29 1:59 [PATCH v3] net: use sock_gen_put() when sk_state is TCP_TIME_WAIT Shiming Cheng
2025-04-30 20:56 ` Jacob Keller
2025-05-01 14:10 ` patchwork-bot+netdevbpf
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox