在 2022/3/1 12:11, John Fastabend 写道:
Wang Yufen wrote:
If tcp_bpf_sendmsg is running during a tear down operation, psock may be
freed.
tcp_bpf_sendmsg()
tcp_bpf_send_verdict()
sk_msg_return()
tcp_bpf_sendmsg_redir()
unlikely(!psock))
sk_msg_free()
The mem of msg has been uncharged in tcp_bpf_send_verdict() by
sk_msg_return(), so we need to use sk_msg_free_nocharge while psock
is null.
This issue can cause the following info:
WARNING: CPU: 0 PID: 2136 at net/ipv4/af_inet.c:155 inet_sock_destruct+0x13c/0x260
Call Trace:
<TASK>
__sk_destruct+0x24/0x1f0
sk_psock_destroy+0x19b/0x1c0
process_one_work+0x1b3/0x3c0
worker_thread+0x30/0x350
? process_one_work+0x3c0/0x3c0
kthread+0xe6/0x110
? kthread_complete_and_exit+0x20/0x20
ret_from_fork+0x22/0x30
</TASK>
Fixes: 604326b41a6f ("bpf, sockmap: convert to generic sk_msg interface")
Signed-off-by: Wang Yufen <wangyufen@xxxxxxxxxx>
---
net/ipv4/tcp_bpf.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/net/ipv4/tcp_bpf.c b/net/ipv4/tcp_bpf.c
index 1f0364e06619..03c037d2a055 100644
--- a/net/ipv4/tcp_bpf.c
+++ b/net/ipv4/tcp_bpf.c
@@ -139,7 +139,7 @@ int tcp_bpf_sendmsg_redir(struct sock *sk, struct sk_msg *msg,
int ret;
if (unlikely(!psock)) {
- sk_msg_free(sk, msg);
+ sk_msg_free_nocharge(sk, msg);
return 0;
}
ret = ingress ? bpf_tcp_ingress(sk, psock, msg, bytes, flags) :
Did you consider simply returning an error code here? This would then
trigger the sk_msg_free_nocharge in the error path of __SK_REDIRECT
and would have the side effect of throwing an error up to user space.
This would be a slight change in behavior from user side but would
look the same as an error if the redirect on the socket threw an
error so I think it would be OK.
Yes, I think it would be better to return -EPIPE, will do in v2.
Thanks.
Thanks,
John
.
