nfc_llcp_socket_release() walks the local sockets list and the listener socket's accept_queue under bh_lock_sock(). However, bh_lock_sock() does not provide mutual exclusion against process-context lock_sock() held by nfc_llcp_accept_dequeue() during accept(). Since socket_release() runs in process context (workqueue or kref cleanup), it never checks sock_owned_by_user(), allowing both paths to manipulate the same child socket concurrently. This leads to a double sock_put() (Use-After-Free) or a NULL pointer dereference of child->parent in nfc_llcp_accept_unlink(). Fix this by converting nfc_llcp_socket_release() to use lock_sock(). Since it is called with the local sockets write-lock held (which disables preemption, preventing sleep), repeatedly pop the head socket from the list under the write-lock using a new helper nfc_llcp_sock_list_pop(), then safely acquire lock_sock() on it after dropping the list lock. A sock_hold() is taken under the list lock to keep the socket alive across the sleep and balanced with a sock_put() after release. Additionally, add a check for a non-NULL parent in nfc_llcp_accept_unlink() to ensure idempotency and prevent NULL pointer dereference if the unlink races with dequeue. Fixes: 50b78b2a6500 ("NFC: Fix sleeping in atomic when releasing socket") Signed-off-by: Lee Jones --- net/nfc/llcp_core.c | 47 +++++++++++++++++++++++++-------------------- net/nfc/llcp_sock.c | 13 +++++++------ 2 files changed, 33 insertions(+), 27 deletions(-) diff --git a/net/nfc/llcp_core.c b/net/nfc/llcp_core.c index dc65c719f35f2..e51bd0d081b62 100644 --- a/net/nfc/llcp_core.c +++ b/net/nfc/llcp_core.c @@ -63,21 +63,33 @@ static void nfc_llcp_socket_purge(struct nfc_llcp_sock *sock) } } +static struct sock *nfc_llcp_sock_list_pop(struct llcp_sock_list *l) +{ + struct sock *sk; + + write_lock(&l->lock); + sk = sk_head(&l->head); + if (sk) { + sock_hold(sk); + sk_del_node_init(sk); + } + write_unlock(&l->lock); + + return sk; +} + static void nfc_llcp_socket_release(struct nfc_llcp_local *local, bool device, int err) { struct sock *sk; - struct hlist_node *tmp; struct nfc_llcp_sock *llcp_sock; skb_queue_purge(&local->tx_queue); - write_lock(&local->sockets.lock); - - sk_for_each_safe(sk, tmp, &local->sockets.head) { + while ((sk = nfc_llcp_sock_list_pop(&local->sockets))) { llcp_sock = nfc_llcp_sock(sk); - bh_lock_sock(sk); + lock_sock(sk); nfc_llcp_socket_purge(llcp_sock); @@ -92,7 +104,8 @@ static void nfc_llcp_socket_release(struct nfc_llcp_local *local, bool device, &llcp_sock->accept_queue, accept_queue) { accept_sk = &lsk->sk; - bh_lock_sock(accept_sk); + lock_sock_nested(accept_sk, + SINGLE_DEPTH_NESTING); nfc_llcp_accept_unlink(accept_sk); @@ -101,7 +114,7 @@ static void nfc_llcp_socket_release(struct nfc_llcp_local *local, bool device, accept_sk->sk_state = LLCP_CLOSED; accept_sk->sk_state_change(sk); - bh_unlock_sock(accept_sk); + release_sock(accept_sk); } } @@ -110,23 +123,18 @@ static void nfc_llcp_socket_release(struct nfc_llcp_local *local, bool device, sk->sk_state = LLCP_CLOSED; sk->sk_state_change(sk); - bh_unlock_sock(sk); - - sk_del_node_init(sk); + release_sock(sk); + sock_put(sk); } - write_unlock(&local->sockets.lock); - /* If we still have a device, we keep the RAW sockets alive */ if (device == true) return; - write_lock(&local->raw_sockets.lock); - - sk_for_each_safe(sk, tmp, &local->raw_sockets.head) { + while ((sk = nfc_llcp_sock_list_pop(&local->raw_sockets))) { llcp_sock = nfc_llcp_sock(sk); - bh_lock_sock(sk); + lock_sock(sk); nfc_llcp_socket_purge(llcp_sock); @@ -135,12 +143,9 @@ static void nfc_llcp_socket_release(struct nfc_llcp_local *local, bool device, sk->sk_state = LLCP_CLOSED; sk->sk_state_change(sk); - bh_unlock_sock(sk); - - sk_del_node_init(sk); + release_sock(sk); + sock_put(sk); } - - write_unlock(&local->raw_sockets.lock); } static struct nfc_llcp_local *nfc_llcp_local_get(struct nfc_llcp_local *local) diff --git a/net/nfc/llcp_sock.c b/net/nfc/llcp_sock.c index feab29fc62f44..3d30f0233b986 100644 --- a/net/nfc/llcp_sock.c +++ b/net/nfc/llcp_sock.c @@ -384,11 +384,12 @@ void nfc_llcp_accept_unlink(struct sock *sk) pr_debug("state %d\n", sk->sk_state); - list_del_init(&llcp_sock->accept_queue); - sk_acceptq_removed(llcp_sock->parent); - llcp_sock->parent = NULL; - - sock_put(sk); + if (llcp_sock->parent) { + list_del_init(&llcp_sock->accept_queue); + sk_acceptq_removed(llcp_sock->parent); + llcp_sock->parent = NULL; + sock_put(sk); + } } void nfc_llcp_accept_enqueue(struct sock *parent, struct sock *sk) @@ -622,7 +623,7 @@ static int llcp_sock_release(struct socket *sock) list_for_each_entry_safe(lsk, n, &llcp_sock->accept_queue, accept_queue) { accept_sk = &lsk->sk; - lock_sock(accept_sk); + lock_sock_nested(accept_sk, SINGLE_DEPTH_NESTING); nfc_llcp_send_disconnect(lsk); nfc_llcp_accept_unlink(accept_sk); -- 2.54.0.823.g6e5bcc1fc9-goog