rxrpc_recvmsg_oob() takes a received oob message off recvmsg_oobq and, if a response is needed, moves it onto the pending_oobq tree. However, only the unlink from recvmsg_oobq is guarded by MSG_PEEK; the move onto pending_oobq always runs. As a result, reading a challenge with MSG_PEEK leaves the skb on recvmsg_oobq while also adding it to pending_oobq. Since struct sk_buff's rbnode shares storage with its next and prev pointers, rb_insert_color() overwrites the list linkage, and the skb, which holds a single reference, becomes reachable from both queues at once. When the socket is closed both queues are drained in turn. While draining recvmsg_oobq, __skb_unlink() follows the next and prev pointers that rbnode has overwritten and writes to a bad address. Also, as the skb holds a single reference but is freed from each queue, both the skb and the connection reference it holds are released twice. This leads to memory corruption and to a use-after-free caused by the connection refcount underflow. MSG_PEEK does not consume the message from the queue, so only unlink it from recvmsg_oobq and then move it onto pending_oobq or free it when the message is actually consumed. Fixes: 5800b1cf3fd8 ("rxrpc: Allow CHALLENGEs to the passed to the app for a RESPONSE") Signed-off-by: Hyunwoo Kim --- net/rxrpc/recvmsg.c | 11 ++++++----- 1 file changed, 6 insertions(+), 5 deletions(-) diff --git a/net/rxrpc/recvmsg.c b/net/rxrpc/recvmsg.c index c940600117a4..bc7f8a505595 100644 --- a/net/rxrpc/recvmsg.c +++ b/net/rxrpc/recvmsg.c @@ -262,12 +262,13 @@ static int rxrpc_recvmsg_oob(struct socket *sock, struct msghdr *msg, break; } - if (!(flags & MSG_PEEK)) + if (!(flags & MSG_PEEK)) { skb_unlink(skb, &rx->recvmsg_oobq); - if (need_response) - rxrpc_add_pending_oob(rx, skb); - else - rxrpc_free_skb(skb, rxrpc_skb_put_oob); + if (need_response) + rxrpc_add_pending_oob(rx, skb); + else + rxrpc_free_skb(skb, rxrpc_skb_put_oob); + } return ret; } -- 2.43.0