On Thu, Nov 23, 2023 at 12:24 PM -08, Cong Wang wrote: > On Wed, Nov 22, 2023 at 11:24:51AM -0800, John Fastabend wrote: >> AF_UNIX stream sockets are a paired socket. So sending on one of the pairs >> will lookup the paired socket as part of the send operation. It is possible >> however to put just one of the pairs in a BPF map. This currently >> increments the refcnt on the sock in the sockmap to ensure it is not >> free'd by the stack before sockmap cleans up its state and stops any >> skbs being sent/recv'd to that socket. >> >> But we missed a case. If the peer socket is closed it will be >> free'd by the stack. However, the paired socket can still be >> referenced from BPF sockmap side because we hold a reference >> there. Then if we are sending traffic through BPF sockmap to >> that socket it will try to dereference the free'd pair in its >> send logic creating a use after free. And following splat, > > Hmm, how could it pass the SOCK_DEAD test in unix_stream_sendmsg()? > > 2285 unix_state_lock(other); > 2286 > 2287 if (sock_flag(other, SOCK_DEAD) || > 2288 (other->sk_shutdown & RCV_SHUTDOWN)) > 2289 goto pipe_err_free; The quoted UAF happens after unix_state_unlock(other): 2285 unix_state_lock(other); 2286 2287 if (sock_flag(other, SOCK_DEAD) || 2288 (other->sk_shutdown & RCV_SHUTDOWN)) 2289 goto pipe_err_free; 2290 2291 maybe_add_creds(skb, sock, other); 2292 scm_stat_add(other, skb); 2293 skb_queue_tail(&other->sk_receive_queue, skb); 2294 unix_state_unlock(other); 2295 other->sk_data_ready(other); <-- UAF Although, I think I saw it happen at unix_state_lock(other) as well. We don't hold a ref on other, so we're racing with __sock_release / unix_release_sock.
