[PATCH net] nfc: pn533: fix use-after-free in pn533_recv_frame
From: Yinhao Hu <hidden>
Date: 2026-06-27 13:14:56
Subsystem:
nfc subsystem, the rest · Maintainers:
David Heidelberg, Linus Torvalds
pn533_recv_frame() checks dev->cmd and then dereferences it several
times to store the receive status and the response skb. In parallel the
command completion worker runs pn533_send_async_complete(), which frees
the same struct pn533_cmd and only clears dev->cmd after the free. With
fast receive/completion timing the receive path can therefore write to
an already freed command object.
Serialize all accesses to dev->cmd with a new cmd_state_lock spinlock:
pn533_recv_frame() now inspects and updates the current command while
holding the lock, the send paths publish and clear the pointer through
pn533_set_current_cmd(), and the completion worker detaches the command
under the lock before freeing it. Once the worker has detached the command,
a racing receive observes a NULL dev->cmd and no longer touches the freed
object.
BUG: KASAN: slab-use-after-free in pn533_recv_frame+0x54f/0x670
Write of size 4 at addr ffff888105566a14 by task kworker/1:3/126
CPU: 1 UID: 0 PID: 126 Comm: kworker/1:3 Not tainted 7.1.0+ #8 PREEMPT
Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS rel-1.16.3-0-ga6ed6b701f0a-prebuilt.qemu.org 04/01/2014
Workqueue: events nfc_urelease_event_work
Call Trace:
<TASK>
dump_stack_lvl+0x8c/0xb0
print_report+0xd0/0x630
kasan_report+0xce/0x100
pn533_recv_frame+0x54f/0x670
pn533_stop_poll+0xdc/0x150
nfc_stop_poll+0xf2/0x1c0
nfc_urelease_event_work+0x194/0x270
process_one_work+0x701/0x1160
worker_thread+0x5e6/0xd50
kthread+0x373/0x4c0
ret_from_fork+0x2e9/0x750
ret_from_fork_asm+0x1a/0x30
</TASK>
Allocated by task 1744:
kasan_save_stack+0x33/0x60
kasan_save_track+0x14/0x30
__kasan_kmalloc+0x8f/0xa0
__kmalloc_cache_noprof+0x194/0x430
__pn533_send_async+0x5c/0x410
pn533_wq_rf+0xc6/0x160
process_one_work+0x701/0x1160
worker_thread+0x5e6/0xd50
kthread+0x373/0x4c0
ret_from_fork+0x2e9/0x750
ret_from_fork_asm+0x1a/0x30
Freed by task 1744:
kasan_save_stack+0x33/0x60
kasan_save_track+0x14/0x30
kasan_save_free_info+0x3b/0x60
__kasan_slab_free+0x43/0x70
kfree+0x17b/0x440
pn533_wq_cmd_complete+0x24d/0x450
process_one_work+0x701/0x1160
worker_thread+0x5e6/0xd50
kthread+0x373/0x4c0
ret_from_fork+0x2e9/0x750
ret_from_fork_asm+0x1a/0x30
Fixes: 9815c7cf22da ("NFC: pn533: Separate physical layer from the core implementation")
Signed-off-by: Yinhao Hu <redacted>
---
drivers/nfc/pn533/pn533.c | 47 ++++++++++++++++++++++++++++++++-------
drivers/nfc/pn533/pn533.h | 1 +
2 files changed, 40 insertions(+), 8 deletions(-)
diff --git a/drivers/nfc/pn533/pn533.c b/drivers/nfc/pn533/pn533.c
index d7bdbc82e2ba..921e93a5f16f 100644
--- a/drivers/nfc/pn533/pn533.c
+++ b/drivers/nfc/pn533/pn533.c@@ -394,12 +394,32 @@ static void pn533_build_cmd_frame(struct pn533 *dev, u8 cmd_code, ops->tx_frame_finish(skb->data); } +static void pn533_set_current_cmd(struct pn533 *dev, struct pn533_cmd *cmd) +{ + unsigned long flags; + + spin_lock_irqsave(&dev->cmd_state_lock, flags); + dev->cmd = cmd; + spin_unlock_irqrestore(&dev->cmd_state_lock, flags); +} + static int pn533_send_async_complete(struct pn533 *dev) { - struct pn533_cmd *cmd = dev->cmd; + struct pn533_cmd *cmd; struct sk_buff *resp; + unsigned long flags; int status, rc = 0; + /* + * Detach the current command before freeing it, so a concurrent + * pn533_recv_frame() either observes a valid command under the lock + * or a NULL dev->cmd and stops touching the freed object. + */ + spin_lock_irqsave(&dev->cmd_state_lock, flags); + cmd = dev->cmd; + dev->cmd = NULL; + spin_unlock_irqrestore(&dev->cmd_state_lock, flags); + if (!cmd) { dev_dbg(dev->dev, "%s: cmd not set\n", __func__); goto done;
@@ -430,7 +450,6 @@ static int pn533_send_async_complete(struct pn533 *dev) done: kfree(cmd); - dev->cmd = NULL; return rc; }
@@ -458,10 +477,10 @@ static int __pn533_send_async(struct pn533 *dev, u8 cmd_code, mutex_lock(&dev->cmd_lock); if (!dev->cmd_pending) { - dev->cmd = cmd; + pn533_set_current_cmd(dev, cmd); rc = dev->phy_ops->send_frame(dev, req); if (rc) { - dev->cmd = NULL; + pn533_set_current_cmd(dev, NULL); goto error; }
@@ -529,10 +548,10 @@ static int pn533_send_cmd_direct_async(struct pn533 *dev, u8 cmd_code, pn533_build_cmd_frame(dev, cmd_code, req); - dev->cmd = cmd; + pn533_set_current_cmd(dev, cmd); rc = dev->phy_ops->send_frame(dev, req); if (rc < 0) { - dev->cmd = NULL; + pn533_set_current_cmd(dev, NULL); kfree(cmd); }
@@ -569,10 +588,10 @@ static void pn533_wq_cmd(struct work_struct *work) mutex_unlock(&dev->cmd_lock); - dev->cmd = cmd; + pn533_set_current_cmd(dev, cmd); rc = dev->phy_ops->send_frame(dev, cmd->req); if (rc < 0) { - dev->cmd = NULL; + pn533_set_current_cmd(dev, NULL); dev_kfree_skb(cmd->req); kfree(cmd); return;
@@ -2165,6 +2184,15 @@ static int pn533_data_exchange_complete(struct pn533 *dev, void *_arg, */ void pn533_recv_frame(struct pn533 *dev, struct sk_buff *skb, int status) { + unsigned long flags; + + /* + * Hold cmd_state_lock across the whole receive path so the current + * command cannot be freed by pn533_send_async_complete() between the + * dev->cmd check and the stores into it. + */ + spin_lock_irqsave(&dev->cmd_state_lock, flags); + if (!dev->cmd) goto sched_wq;
@@ -2182,6 +2210,7 @@ void pn533_recv_frame(struct pn533 *dev, struct sk_buff *skb, int status) if (pn533_rx_frame_is_ack(skb->data)) { dev_dbg(dev->dev, "%s: Received ACK frame\n", __func__); + spin_unlock_irqrestore(&dev->cmd_state_lock, flags); dev_kfree_skb(skb); return; }
@@ -2200,6 +2229,7 @@ void pn533_recv_frame(struct pn533 *dev, struct sk_buff *skb, int status) dev->cmd->resp = skb; sched_wq: + spin_unlock_irqrestore(&dev->cmd_state_lock, flags); queue_work(dev->wq, &dev->cmd_complete_work); } EXPORT_SYMBOL(pn533_recv_frame);
@@ -2760,6 +2790,7 @@ struct pn533 *pn53x_common_init(u32 device_type, priv->device_type = device_type; mutex_init(&priv->cmd_lock); + spin_lock_init(&priv->cmd_state_lock); INIT_WORK(&priv->cmd_work, pn533_wq_cmd); INIT_WORK(&priv->cmd_complete_work, pn533_wq_cmd_complete);
diff --git a/drivers/nfc/pn533/pn533.h b/drivers/nfc/pn533/pn533.h
index 09e35b8693f5..8b009b2318d0 100644
--- a/drivers/nfc/pn533/pn533.h
+++ b/drivers/nfc/pn533/pn533.h@@ -153,6 +153,7 @@ struct pn533 { struct pn533_cmd *cmd; u8 cmd_pending; struct mutex cmd_lock; /* protects cmd queue */ + spinlock_t cmd_state_lock; /* protects dev->cmd lifetime */ void *cmd_complete_mi_arg; void *cmd_complete_dep_arg;
--
2.43.0