| Message ID | 20220302022755.3876705-4-wangyufen@huawei.com (mailing list archive) |
|---|---|
| State | Superseded |
| Delegated to: | BPF |
| Headers | show |
| Series | bpf, sockmap: Fix memleaks and issues of mem charge/uncharge | expand |
| Context | Check | Description |
|---|---|---|
| bpf/vmtest-bpf-next-PR | success | PR summary |
| netdev/tree_selection | success | Clearly marked for bpf-next |
| netdev/fixes_present | success | Fixes tag not required for -next series |
| netdev/subject_prefix | success | Link |
| netdev/cover_letter | success | Series has a cover letter |
| netdev/patch_count | success | Link |
| netdev/header_inline | success | No static functions without inline keyword in header files |
| netdev/build_32bit | success | Errors and warnings before: 2 this patch: 2 |
| netdev/cc_maintainers | success | CCed 17 of 17 maintainers |
| netdev/build_clang | success | Errors and warnings before: 18 this patch: 18 |
| netdev/module_param | success | Was 0 now: 0 |
| netdev/verify_signedoff | success | Signed-off-by tag matches author and committer |
| netdev/verify_fixes | success | Fixes tag looks correct |
| netdev/build_allmodconfig_warn | success | Errors and warnings before: 7 this patch: 7 |
| netdev/checkpatch | success | total: 0 errors, 0 warnings, 0 checks, 20 lines checked |
| netdev/kdoc | success | Errors and warnings before: 0 this patch: 0 |
| netdev/source_inline | success | Was 0 now: 0 |
| bpf/vmtest-bpf-next | success | VM_Test |
On Wed, Mar 02, 2022 at 10:27:54AM +0800, Wang Yufen wrote: > In tcp_bpf_send_verdict(), if msg has more data after > tcp_bpf_sendmsg_redir(): > > tcp_bpf_send_verdict() > tosend = msg->sg.size //msg->sg.size = 22220 > case __SK_REDIRECT: > sk_msg_return() //uncharged msg->sg.size(22220) sk->sk_forward_alloc > tcp_bpf_sendmsg_redir() //after tcp_bpf_sendmsg_redir, msg->sg.size=11000 > goto more_data; > tosend = msg->sg.size //msg->sg.size = 11000 > case __SK_REDIRECT: > sk_msg_return() //uncharged msg->sg.size(11000) to sk->sk_forward_alloc > > The msg->sg.size(11000) has been uncharged twice, to fix we can charge the > remaining msg->sg.size before goto more data. It looks like bpf_exec_tx_verdict() has the same issue.
在 2022/3/3 14:14, Cong Wang 写道: > On Wed, Mar 02, 2022 at 10:27:54AM +0800, Wang Yufen wrote: >> In tcp_bpf_send_verdict(), if msg has more data after >> tcp_bpf_sendmsg_redir(): >> >> tcp_bpf_send_verdict() >> tosend = msg->sg.size //msg->sg.size = 22220 >> case __SK_REDIRECT: >> sk_msg_return() //uncharged msg->sg.size(22220) sk->sk_forward_alloc >> tcp_bpf_sendmsg_redir() //after tcp_bpf_sendmsg_redir, msg->sg.size=11000 >> goto more_data; >> tosend = msg->sg.size //msg->sg.size = 11000 >> case __SK_REDIRECT: >> sk_msg_return() //uncharged msg->sg.size(11000) to sk->sk_forward_alloc >> >> The msg->sg.size(11000) has been uncharged twice, to fix we can charge the >> remaining msg->sg.size before goto more data. > It looks like bpf_exec_tx_verdict() has the same issue. > > . In bpf_exec_tx_verdict(), case __SK_REDIRECT, msg_redir is used and msg->sg.size is deducted in advance. Therefore, this issue (more uncharged) does not exist. However, I think that if msg_redir processing cannot be completed , that is msg_redir has more data, and there is no subsequent processing, maybe that is another problem. Thanks.
diff --git a/net/ipv4/tcp_bpf.c b/net/ipv4/tcp_bpf.c index ac9f491cc139..1f0364e06619 100644 --- a/net/ipv4/tcp_bpf.c +++ b/net/ipv4/tcp_bpf.c @@ -335,7 +335,7 @@ static int tcp_bpf_send_verdict(struct sock *sk, struct sk_psock *psock, cork = true; psock->cork = NULL; } - sk_msg_return(sk, msg, tosend); + sk_msg_return(sk, msg, msg->sg.size); release_sock(sk); ret = tcp_bpf_sendmsg_redir(sk_redir, msg, tosend, flags); @@ -375,8 +375,11 @@ static int tcp_bpf_send_verdict(struct sock *sk, struct sk_psock *psock, } if (msg && msg->sg.data[msg->sg.start].page_link && - msg->sg.data[msg->sg.start].length) + msg->sg.data[msg->sg.start].length) { + if (eval == __SK_REDIRECT) + sk_mem_charge(sk, msg->sg.size); goto more_data; + } } return ret; }
In tcp_bpf_send_verdict(), if msg has more data after tcp_bpf_sendmsg_redir(): tcp_bpf_send_verdict() tosend = msg->sg.size //msg->sg.size = 22220 case __SK_REDIRECT: sk_msg_return() //uncharged msg->sg.size(22220) sk->sk_forward_alloc tcp_bpf_sendmsg_redir() //after tcp_bpf_sendmsg_redir, msg->sg.size=11000 goto more_data; tosend = msg->sg.size //msg->sg.size = 11000 case __SK_REDIRECT: sk_msg_return() //uncharged msg->sg.size(11000) to sk->sk_forward_alloc The msg->sg.size(11000) has been uncharged twice, to fix we can charge the remaining msg->sg.size before goto more data. This issue can cause the following info: WARNING: CPU: 0 PID: 9860 at net/core/stream.c:208 sk_stream_kill_queues+0xd4/0x1a0 Call Trace: <TASK> inet_csk_destroy_sock+0x55/0x110 __tcp_close+0x279/0x470 tcp_close+0x1f/0x60 inet_release+0x3f/0x80 __sock_release+0x3d/0xb0 sock_close+0x11/0x20 __fput+0x92/0x250 task_work_run+0x6a/0xa0 do_exit+0x33b/0xb60 do_group_exit+0x2f/0xa0 get_signal+0xb6/0x950 arch_do_signal_or_restart+0xac/0x2a0 ? vfs_write+0x237/0x290 exit_to_user_mode_prepare+0xa9/0x200 syscall_exit_to_user_mode+0x12/0x30 do_syscall_64+0x46/0x80 entry_SYSCALL_64_after_hwframe+0x44/0xae </TASK> 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@huawei.com> Acked-by: John Fastabend <john.fastabend@gmail.com> --- net/ipv4/tcp_bpf.c | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-)