On Tue, Mar 01, 2022 at 09:49:12AM +0800, wangyufen wrote: > > 在 2022/2/28 3:21, Cong Wang 写道: > > On Fri, Feb 25, 2022 at 09:49:26AM +0800, Wang Yufen wrote: > > > If tcp_bpf_sendmsg is running during a tear down operation we may enqueue > > > data on the ingress msg queue while tear down is trying to free it. > > > > > > sk1 (redirect sk2) sk2 > > > ------------------- --------------- > > > tcp_bpf_sendmsg() > > > tcp_bpf_send_verdict() > > > tcp_bpf_sendmsg_redir() > > > bpf_tcp_ingress() > > > sock_map_close() > > > lock_sock() > > > lock_sock() ... blocking > > > sk_psock_stop > > > sk_psock_clear_state(psock, SK_PSOCK_TX_ENABLED); > > > release_sock(sk); > > > lock_sock() > > > sk_mem_charge() > > > get_page() > > > sk_psock_queue_msg() > > > sk_psock_test_state(psock, SK_PSOCK_TX_ENABLED); > > > drop_sk_msg() > > > release_sock() > > > > > > While drop_sk_msg(), the msg has charged memory form sk by sk_mem_charge > > > and has sg pages need to put. To fix we use sk_msg_free() and then kfee() > > > msg. > > > > > What about the other code path? That is, sk_psock_skb_ingress_enqueue(). > > I don't see skmsg is charged there. > > sk_psock_skb_ingress_self() | sk_psock_skb_ingress() > skb_set_owner_r() > sk_mem_charge() > sk_psock_skb_ingress_enqueue() > > The other code path skmsg is charged by skb_set_owner_r()->sk_mem_charge() > skb_set_owner_r() charges skb, I was asking skmsg. ;) In sk_psock_skb_ingress_enqueue(), the skmsg was initialized but not actually charged, hence I was asking... From a second look, it seems sk_mem_uncharge() is not called for sk_psock_skb_ingress_enqueue() where msg->skb is clearly not NULL. Also, you introduce an unnecessary sk_msg_init() from __sk_msg_free(), because you call kfree(msg) after it. Thanks.
