Re: [PATCH bpf-next 1/4] bpf, sockmap: Fix memleak in sk_psock_queue_msg

wangyufen <wangyufen@xxxxxxxxxx> · Tue, 1 Mar 2022 09:49:12 +0800

在 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()

Thanks.
.