On Tue, Mar 01, 2022 at 09:49:12AM +0800, wangyufen wrote:

在 2022/2/28 3:21, Cong Wang 写道:

quoted

On Fri, Feb 25, 2022 at 09:49:26AM +0800, Wang Yufen wrote:

quoted

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.