CVE-2023-53585

In the Linux kernel, the following vulnerability has been resolved:

bpf: reject unhashed sockets in bpfskassign

The semantics for bpfskassign are as follows:

sk = some_lookup_func()
bpf_sk_assign(skb, sk)
bpf_sk_release(sk)

That is, the sk is not consumed by bpfskassign. The function therefore needs to make sure that sk lives long enough to be consumed from __inetlookupskb. The path through the stack for a TCPv4 packet is roughly:

netifreceiveskb_core: takes RCU read lock __netifreceiveskbcore: schhandleingress: tcfclassify: bpfskassign() deliverptypelistskb: deliverskb: ippackettype->func == iprcv: iprcvcore: iprcvfinishcore: dstinput: iplocaldeliver: iplocaldeliverfinish: ipprotocoldeliverrcu: tcpv4_rcv: __inetlookupskb: skbstealsock

The existing helper takes advantage of the fact that everything happens in the same RCU critical section: for sockets with SOCKRCUFREE set bpfskassign never takes a reference. skbstealsock then checks SOCKRCUFREE again and does sock_put if necessary.

This approach assumes that SOCKRCUFREE is never set on a sk between bpfskassign and skbstealsock, but this invariant is violated by unhashed UDP sockets. A new UDP socket is created in TCPCLOSE state but without SOCKRCUFREE set. That flag is only added in udplibgetport() which happens when a socket is bound.

When bpfskassign was added it wasn't possible to access unhashed UDP sockets from BPF, so this wasn't a problem. This changed in commit 0c48eefae712 ("sock_map: Lift socket state restriction for datagram sockets"), but the helper wasn't adjusted accordingly. The following sequence of events will therefore lead to a refcount leak:

1. Add socket(AFINET, SOCKDGRAM) to a sockmap.
2. Pull socket out of sockmap and bpfskassign it. Since SOCKRCUFREE is not set we increment the refcount.
3. bind() or connect() the socket, setting SOCKRCUFREE.
4. skbstealsock will now set refcounted = false due to SOCKRCUFREE.
5. tcpv4rcv() skips sock_put().

Fix the problem by rejecting unhashed sockets in bpfskassign(). This matches the behaviour of __inetlookupskb which is ultimately the goal of bpfskassign().