mirror of
https://github.com/torvalds/linux.git
synced 2026-09-26 01:52:03 +02:00
nfc: llcp: Fix race condition in accept_queue lifecycle
In nfc_llcp_socket_release(), sockets and listener accept queues are
walked under the local sockets rwlock and bh_lock_sock(). However,
bh_lock_sock() does not synchronise against process-context lock_sock()
held by nfc_llcp_accept_dequeue() during accept(). Because
socket_release() does not check sock_owned_by_user(), both paths can
concurrently unlink and release the same child socket, resulting in
use-after-free or a NULL pointer dereference of child->parent in
nfc_llcp_accept_unlink().
Fix this synchronisation race by having nfc_llcp_socket_release() use
process-context lock_sock() instead of bh_lock_sock():
1. Pop sockets from the local sockets list under the write lock using
nfc_llcp_sock_list_pop() so lock_sock() can be acquired without
holding the rwlock.
2. Because lock_sock() can sleep, defer the final release of the
nfc_llcp_local structure to a workqueue (release_work). This avoids
a sleeping-in-atomic bug when the last local reference is dropped
from softirq context. Additionally, hold a single device reference
on local from registration until final destruction.
3. In nfc_llcp_local_get(), use kref_get_unless_zero() to prevent
resurrecting a local object whose teardown has been scheduled.
4. In llcp_sock_accept(), verify that the listener socket state is still
LLCP_LISTEN after waking from schedule_timeout() to prevent hangs if
the listener is closed concurrently.
5. When unlinking unaccepted child sockets during listener release,
unlink them from local->sockets, call sock_orphan(), and drop their
initial sk_alloc creation reference via sock_put().
6. Make nfc_llcp_accept_unlink() idempotent by guarding parent access with
a NULL check.
Fixes: 50b78b2a65 ("NFC: Fix sleeping in atomic when releasing socket")
Signed-off-by: Lee Jones <lee@kernel.org>
Link: https://patch.msgid.link/20260902123033.1169067-1-lee@kernel.org
Signed-off-by: David Heidelberg <david@ixit.cz>
This commit is contained in:
parent
3d8afc5243
commit
c3eef2f988
|
|
@ -91,6 +91,7 @@ struct nfc_llcp_local {
|
|||
struct hlist_head pending_sdreqs;
|
||||
struct timer_list sdreq_timer;
|
||||
struct work_struct sdreq_timeout_work;
|
||||
struct work_struct release_work;
|
||||
u8 sdreq_next_tid;
|
||||
|
||||
/* sockets array */
|
||||
|
|
|
|||
|
|
@ -20,6 +20,8 @@ static LIST_HEAD(llcp_devices);
|
|||
/* Protects llcp_devices list */
|
||||
static DEFINE_SPINLOCK(llcp_devices_lock);
|
||||
|
||||
static struct workqueue_struct *llcp_wq;
|
||||
|
||||
static void nfc_llcp_rx_skb(struct nfc_llcp_local *local, struct sk_buff *skb);
|
||||
|
||||
void nfc_llcp_sock_link(struct llcp_sock_list *l, struct sock *sk)
|
||||
|
|
@ -63,21 +65,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);
|
||||
|
||||
|
|
@ -91,17 +105,27 @@ static void nfc_llcp_socket_release(struct nfc_llcp_local *local, bool device,
|
|||
list_for_each_entry_safe(lsk, n,
|
||||
&llcp_sock->accept_queue,
|
||||
accept_queue) {
|
||||
bool put_creation = false;
|
||||
|
||||
accept_sk = &lsk->sk;
|
||||
bh_lock_sock(accept_sk);
|
||||
lock_sock_nested(accept_sk,
|
||||
SINGLE_DEPTH_NESTING);
|
||||
|
||||
nfc_llcp_accept_unlink(accept_sk);
|
||||
if (nfc_llcp_sock(accept_sk)->parent == sk) {
|
||||
nfc_llcp_accept_unlink(accept_sk);
|
||||
nfc_llcp_sock_unlink(&local->sockets, accept_sk);
|
||||
|
||||
if (err)
|
||||
accept_sk->sk_err = err;
|
||||
accept_sk->sk_state = LLCP_CLOSED;
|
||||
accept_sk->sk_state_change(sk);
|
||||
if (err)
|
||||
accept_sk->sk_err = err;
|
||||
accept_sk->sk_state = LLCP_CLOSED;
|
||||
accept_sk->sk_state_change(accept_sk);
|
||||
sock_orphan(accept_sk);
|
||||
put_creation = true;
|
||||
}
|
||||
|
||||
bh_unlock_sock(accept_sk);
|
||||
release_sock(accept_sk);
|
||||
if (put_creation)
|
||||
sock_put(accept_sk); /* creation ref */
|
||||
}
|
||||
}
|
||||
|
||||
|
|
@ -110,23 +134,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,26 +154,20 @@ 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)
|
||||
{
|
||||
/* Since using nfc_llcp_local may result in usage of nfc_dev, whenever
|
||||
* we hold a reference to local, we also need to hold a reference to
|
||||
* the device to avoid UAF.
|
||||
*/
|
||||
if (!nfc_get_device(local->dev->idx))
|
||||
if (!local)
|
||||
return NULL;
|
||||
|
||||
kref_get(&local->ref);
|
||||
if (kref_get_unless_zero(&local->ref))
|
||||
return local;
|
||||
|
||||
return local;
|
||||
return NULL;
|
||||
}
|
||||
|
||||
static void local_cleanup(struct nfc_llcp_local *local)
|
||||
|
|
@ -172,30 +185,34 @@ static void local_cleanup(struct nfc_llcp_local *local)
|
|||
nfc_llcp_free_sdp_tlv_list(&local->pending_sdreqs);
|
||||
}
|
||||
|
||||
static void local_release_work(struct work_struct *work)
|
||||
{
|
||||
struct nfc_llcp_local *local;
|
||||
struct nfc_dev *dev;
|
||||
|
||||
local = container_of(work, struct nfc_llcp_local, release_work);
|
||||
dev = local->dev;
|
||||
|
||||
local_cleanup(local);
|
||||
kfree(local);
|
||||
nfc_put_device(dev);
|
||||
}
|
||||
|
||||
static void local_release(struct kref *ref)
|
||||
{
|
||||
struct nfc_llcp_local *local;
|
||||
|
||||
local = container_of(ref, struct nfc_llcp_local, ref);
|
||||
|
||||
local_cleanup(local);
|
||||
kfree(local);
|
||||
queue_work(llcp_wq, &local->release_work);
|
||||
}
|
||||
|
||||
int nfc_llcp_local_put(struct nfc_llcp_local *local)
|
||||
{
|
||||
struct nfc_dev *dev;
|
||||
int ret;
|
||||
|
||||
if (local == NULL)
|
||||
if (!local)
|
||||
return 0;
|
||||
|
||||
dev = local->dev;
|
||||
|
||||
ret = kref_put(&local->ref, local_release);
|
||||
nfc_put_device(dev);
|
||||
|
||||
return ret;
|
||||
return kref_put(&local->ref, local_release);
|
||||
}
|
||||
|
||||
static struct nfc_llcp_sock *nfc_llcp_sock_get(struct nfc_llcp_local *local,
|
||||
|
|
@ -1705,6 +1722,7 @@ int nfc_llcp_register_device(struct nfc_dev *ndev)
|
|||
INIT_WORK(&local->rx_work, nfc_llcp_rx_work);
|
||||
|
||||
INIT_WORK(&local->timeout_work, nfc_llcp_timeout_work);
|
||||
INIT_WORK(&local->release_work, local_release_work);
|
||||
|
||||
rwlock_init(&local->sockets.lock);
|
||||
rwlock_init(&local->connecting_sockets.lock);
|
||||
|
|
@ -1748,10 +1766,23 @@ void nfc_llcp_unregister_device(struct nfc_dev *dev)
|
|||
|
||||
int __init nfc_llcp_init(void)
|
||||
{
|
||||
return nfc_llcp_sock_init();
|
||||
int ret;
|
||||
|
||||
llcp_wq = alloc_workqueue("nfc_llcp_wq", WQ_UNBOUND, 0);
|
||||
if (!llcp_wq)
|
||||
return -ENOMEM;
|
||||
|
||||
ret = nfc_llcp_sock_init();
|
||||
if (ret) {
|
||||
destroy_workqueue(llcp_wq);
|
||||
return ret;
|
||||
}
|
||||
|
||||
return 0;
|
||||
}
|
||||
|
||||
void nfc_llcp_exit(void)
|
||||
{
|
||||
nfc_llcp_sock_exit();
|
||||
destroy_workqueue(llcp_wq);
|
||||
}
|
||||
|
|
|
|||
|
|
@ -392,11 +392,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)
|
||||
|
|
@ -423,12 +424,20 @@ struct sock *nfc_llcp_accept_dequeue(struct sock *parent,
|
|||
|
||||
list_for_each_entry_safe(lsk, n, &llcp_parent->accept_queue,
|
||||
accept_queue) {
|
||||
struct nfc_llcp_local *local;
|
||||
|
||||
sk = &lsk->sk;
|
||||
lock_sock(sk);
|
||||
lock_sock_nested(sk, SINGLE_DEPTH_NESTING);
|
||||
|
||||
if (sk->sk_state == LLCP_CLOSED) {
|
||||
release_sock(sk);
|
||||
local = nfc_llcp_sock(sk)->local;
|
||||
|
||||
nfc_llcp_accept_unlink(sk);
|
||||
if (local)
|
||||
nfc_llcp_sock_unlink(&local->sockets, sk);
|
||||
sock_orphan(sk);
|
||||
release_sock(sk);
|
||||
sock_put(sk);
|
||||
continue;
|
||||
}
|
||||
|
||||
|
|
@ -464,7 +473,7 @@ static int llcp_sock_accept(struct socket *sock, struct socket *newsock,
|
|||
|
||||
pr_debug("parent %p\n", sk);
|
||||
|
||||
lock_sock_nested(sk, SINGLE_DEPTH_NESTING);
|
||||
lock_sock(sk);
|
||||
|
||||
if (sk->sk_state != LLCP_LISTEN) {
|
||||
ret = -EBADFD;
|
||||
|
|
@ -490,7 +499,12 @@ static int llcp_sock_accept(struct socket *sock, struct socket *newsock,
|
|||
|
||||
release_sock(sk);
|
||||
timeo = schedule_timeout(timeo);
|
||||
lock_sock_nested(sk, SINGLE_DEPTH_NESTING);
|
||||
lock_sock(sk);
|
||||
|
||||
if (sk->sk_state != LLCP_LISTEN) {
|
||||
ret = -EBADFD;
|
||||
break;
|
||||
}
|
||||
}
|
||||
__set_current_state(TASK_RUNNING);
|
||||
remove_wait_queue(sk_sleep(sk), &wait);
|
||||
|
|
@ -629,13 +643,24 @@ 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);
|
||||
bool put_creation = false;
|
||||
|
||||
nfc_llcp_send_disconnect(lsk);
|
||||
nfc_llcp_accept_unlink(accept_sk);
|
||||
accept_sk = &lsk->sk;
|
||||
lock_sock_nested(accept_sk, SINGLE_DEPTH_NESTING);
|
||||
|
||||
if (nfc_llcp_sock(accept_sk)->parent == sk) {
|
||||
nfc_llcp_send_disconnect(lsk);
|
||||
nfc_llcp_accept_unlink(accept_sk);
|
||||
nfc_llcp_sock_unlink(&local->sockets, accept_sk);
|
||||
|
||||
accept_sk->sk_state = LLCP_CLOSED;
|
||||
sock_orphan(accept_sk);
|
||||
put_creation = true;
|
||||
}
|
||||
|
||||
release_sock(accept_sk);
|
||||
if (put_creation)
|
||||
sock_put(accept_sk); /* creation ref */
|
||||
}
|
||||
}
|
||||
|
||||
|
|
|
|||
Loading…
Reference in New Issue
Block a user