[PATCH 1/2] nfc: pn533: bound device-supplied lengths in the receive path
From: Michael Bommarito
Date: Wed Jun 17 2026 - 23:00:32 EST
The PN533 receive path trusts device-supplied length fields and reads
past the received buffer when they exceed it. The rx skb is sized to the
device's USB transfer length, so a malicious PN53x reader triggers
out-of-bounds reads in the standard/ACR122 frame validators, in Type A
target parsing (nfcid_len), and in the autopoll record walk.
Thread the received length into the rx_is_frame_valid callbacks, reject
ACR122 frames with datalen < 2, and add the missing nfcid and per-record
bounds.
This was found with static analysis and confirmed with the KUnit tests
added in the following patch.
Fixes: c46ee38620a2 ("NFC: pn533: add NXP pn533 nfc device driver")
Fixes: 9815c7cf22da ("NFC: pn533: Separate physical layer from the core implementation")
Assisted-by: Claude:claude-opus-4-8
Signed-off-by: Michael Bommarito <michael.bommarito@xxxxxxxxx>
---
drivers/nfc/pn533/pn533.c | 32 ++++++++++++++++++++++++++++++--
drivers/nfc/pn533/pn533.h | 3 ++-
drivers/nfc/pn533/usb.c | 12 ++++++++++--
3 files changed, 42 insertions(+), 5 deletions(-)
diff --git a/drivers/nfc/pn533/pn533.c b/drivers/nfc/pn533/pn533.c
index d7bdbc82e2ba2..984a61a83feaf 100644
--- a/drivers/nfc/pn533/pn533.c
+++ b/drivers/nfc/pn533/pn533.c
@@ -274,11 +274,15 @@ static void pn533_std_tx_update_payload_len(void *_frame, int len)
frame->datalen += len;
}
-static bool pn533_std_rx_frame_is_valid(void *_frame, struct pn533 *dev)
+static bool pn533_std_rx_frame_is_valid(void *_frame, size_t frame_len,
+ struct pn533 *dev)
{
u8 checksum;
struct pn533_std_frame *stdf = _frame;
+ if (frame_len < sizeof(struct pn533_std_frame))
+ return false;
+
if (stdf->start_frame != cpu_to_be16(PN533_STD_FRAME_SOF))
return false;
@@ -286,6 +290,10 @@ static bool pn533_std_rx_frame_is_valid(void *_frame, struct pn533 *dev)
/* Standard frame code */
dev->ops->rx_header_len = PN533_STD_FRAME_HEADER_LEN;
+ if (frame_len < sizeof(struct pn533_std_frame) +
+ stdf->datalen + PN533_STD_FRAME_TAIL_LEN)
+ return false;
+
checksum = pn533_std_checksum(stdf->datalen);
if (checksum != stdf->datalen_checksum)
return false;
@@ -299,6 +307,11 @@ static bool pn533_std_rx_frame_is_valid(void *_frame, struct pn533 *dev)
dev->ops->rx_header_len = PN533_EXT_FRAME_HEADER_LEN;
+ if (frame_len < sizeof(struct pn533_ext_frame) ||
+ frame_len < sizeof(struct pn533_ext_frame) +
+ be16_to_cpu(eif->datalen) + PN533_STD_FRAME_TAIL_LEN)
+ return false;
+
checksum = pn533_ext_checksum(be16_to_cpu(eif->datalen));
if (checksum != eif->datalen_checksum)
return false;
@@ -701,6 +714,10 @@ static bool pn533_target_type_a_is_valid(struct pn533_target_type_a *type_a,
if (type_a->nfcid_len > NFC_NFCID1_MAXSIZE)
return false;
+ if (sizeof(struct pn533_target_type_a) + type_a->nfcid_len >
+ (size_t)target_data_len)
+ return false;
+
return true;
}
@@ -1399,6 +1416,7 @@ static int pn533_autopoll_complete(struct pn533 *dev, void *arg,
{
struct pn532_autopoll_resp *apr;
struct nfc_target nfc_tgt;
+ size_t remaining;
u8 nbtg;
int rc;
@@ -1417,12 +1435,21 @@ static int pn533_autopoll_complete(struct pn533 *dev, void *arg,
goto stop_poll;
}
+ if (resp->len < 1)
+ return -EAGAIN;
+
nbtg = resp->data[0];
if ((nbtg > 2) || (nbtg <= 0))
return -EAGAIN;
+ remaining = resp->len - 1;
apr = (struct pn532_autopoll_resp *)&resp->data[1];
while (nbtg--) {
+ if (remaining < sizeof(struct pn532_autopoll_resp) ||
+ apr->ln < 1 ||
+ remaining < (size_t)apr->ln + 2)
+ return -EAGAIN;
+
memset(&nfc_tgt, 0, sizeof(struct nfc_target));
switch (apr->type) {
case PN532_AUTOPOLL_TYPE_ISOA:
@@ -1468,6 +1495,7 @@ static int pn533_autopoll_complete(struct pn533 *dev, void *arg,
}
dev->tgt_available_prots = nfc_tgt.supported_protocols;
+ remaining -= (size_t)apr->ln + 2;
apr = (struct pn532_autopoll_resp *)
(apr->tgdata + (apr->ln - 1));
}
@@ -2189,7 +2217,7 @@ void pn533_recv_frame(struct pn533 *dev, struct sk_buff *skb, int status)
print_hex_dump_debug("PN533 RX: ", DUMP_PREFIX_NONE, 16, 1, skb->data,
dev->ops->rx_frame_size(skb->data), false);
- if (!dev->ops->rx_is_frame_valid(skb->data, dev)) {
+ if (!dev->ops->rx_is_frame_valid(skb->data, skb->len, dev)) {
nfc_err(dev->dev, "Received an invalid frame\n");
dev->cmd->status = -EIO;
} else if (!pn533_rx_frame_is_cmd_response(dev, skb->data)) {
diff --git a/drivers/nfc/pn533/pn533.h b/drivers/nfc/pn533/pn533.h
index 09e35b8693f5a..08804f88b38a3 100644
--- a/drivers/nfc/pn533/pn533.h
+++ b/drivers/nfc/pn533/pn533.h
@@ -201,7 +201,8 @@ struct pn533_frame_ops {
int tx_header_len;
int tx_tail_len;
- bool (*rx_is_frame_valid)(void *frame, struct pn533 *dev);
+ bool (*rx_is_frame_valid)(void *frame, size_t frame_len,
+ struct pn533 *dev);
bool (*rx_frame_is_ack)(void *frame);
int (*rx_frame_size)(void *frame);
int rx_header_len;
diff --git a/drivers/nfc/pn533/usb.c b/drivers/nfc/pn533/usb.c
index efb07f944fce2..1d50600609dd4 100644
--- a/drivers/nfc/pn533/usb.c
+++ b/drivers/nfc/pn533/usb.c
@@ -312,14 +312,22 @@ static void pn533_acr122_tx_update_payload_len(void *_frame, int len)
frame->datalen += len;
}
-static bool pn533_acr122_is_rx_frame_valid(void *_frame, struct pn533 *dev)
+static bool pn533_acr122_is_rx_frame_valid(void *_frame, size_t frame_len,
+ struct pn533 *dev)
{
struct pn533_acr122_rx_frame *frame = _frame;
+ if (frame_len < sizeof(struct pn533_acr122_rx_frame))
+ return false;
+
if (frame->ccid.type != 0x83)
return false;
- if (!frame->ccid.datalen)
+ if (frame->ccid.datalen < 2)
+ return false;
+
+ if (frame_len < sizeof(struct pn533_acr122_rx_frame) +
+ frame->ccid.datalen)
return false;
if (frame->data[frame->ccid.datalen - 2] == 0x63)
--
2.53.0