* [PATCH] nfc: st95hf: validate device-reported response length before reading
@ 2026-08-19 14:55 Tianchu Chen
2026-09-20 23:02 ` David Heidelberg
2026-09-23 8:18 ` [PATCH v2] " Tianchu Chen
0 siblings, 2 replies; 5+ messages in thread
From: Tianchu Chen @ 2026-08-19 14:55 UTC (permalink / raw)
To: david, sameo, shikha.singh; +Cc: oe-linux-nfc, linux-kernel
From: Tianchu Chen <flynnnchen@tencent.com>
st95hf_spi_recv_response() reads a 2-byte header from the device,
derives the total response length from it (up to 1025 bytes via the
long-frame encoding) and then blindly reads len - 2 more bytes into
the caller's buffer, whose size it knows nothing about.
Call sites can be overflowed by a device reporting a large length,
for example, st95hf_irq_thread_handler() passes the data area of a
280-byte skb.
ST95HF datasheet states "In Reader mode it is possible to receive up to
528 bytes of frame data from VICC and TypeB cards", so technically a bogus
NFC tag may trigger the OOB-write if the card-reader's firmware allows
large packets.
Add a buff_len parameter and reject a device-reported length that does
not fit into the caller's buffer before issuing the second SPI
transfer. Also modify callers to pass their real buffer sizes.
Discovered by Atuin - Automated Vulnerability Discovery Engine.
Fixes: cab47333f0f75 ("NFC: Add STMicroelectronics ST95HF driver")
Cc: stable@vger.kernel.org
Signed-off-by: Tianchu Chen <flynnnchen@tencent.com>
---
drivers/nfc/st95hf/core.c | 6 ++++--
drivers/nfc/st95hf/spi.c | 7 ++++++-
drivers/nfc/st95hf/spi.h | 2 +-
3 files changed, 11 insertions(+), 4 deletions(-)
diff --git a/drivers/nfc/st95hf/core.c b/drivers/nfc/st95hf/core.c
index 4d772a308bfff..0fbb60deada23 100644
--- a/drivers/nfc/st95hf/core.c
+++ b/drivers/nfc/st95hf/core.c
@@ -283,7 +283,8 @@ static int st95hf_send_recv_cmd(struct st95hf_context *st95context,
unsigned char st95hf_response_arr[2];
ret = st95hf_spi_recv_response(&st95context->spicontext,
- st95hf_response_arr);
+ st95hf_response_arr,
+ sizeof(st95hf_response_arr));
if (ret < 0) {
dev_err(dev, "spi error from st95hf_spi_recv_response(), err = 0x%x\n",
ret);
@@ -800,7 +801,8 @@ static irqreturn_t st95hf_irq_thread_handler(int irq, void *st95hfcontext)
mutex_lock(&stcontext->rm_lock);
res_len = st95hf_spi_recv_response(&stcontext->spicontext,
- skb_resp->data);
+ skb_resp->data,
+ skb_tailroom(skb_resp));
if (res_len < 0) {
dev_err(spidevice, "TISR spi response err = 0x%x\n", res_len);
result = res_len;
diff --git a/drivers/nfc/st95hf/spi.c b/drivers/nfc/st95hf/spi.c
index ffaf2789c4069..f48abd09a08fc 100644
--- a/drivers/nfc/st95hf/spi.c
+++ b/drivers/nfc/st95hf/spi.c
@@ -66,7 +66,7 @@ EXPORT_SYMBOL_GPL(st95hf_spi_send);
/* Function to Receive command Response */
int st95hf_spi_recv_response(struct st95hf_spi_context *spicontext,
- unsigned char *receivebuff)
+ unsigned char *receivebuff, int buff_len)
{
int len = 0;
struct spi_transfer tx_takedata;
@@ -106,6 +106,11 @@ int st95hf_spi_recv_response(struct st95hf_spi_context *spicontext,
else
len += receivebuff[1];
+ if (len > buff_len) {
+ mutex_unlock(&spicontext->spi_lock);
+ return -E2BIG;
+ }
+
/* Now make a transfer to read only relevant bytes */
tx_takedata.rx_buf = &receivebuff[2];
tx_takedata.len = len - 2;
diff --git a/drivers/nfc/st95hf/spi.h b/drivers/nfc/st95hf/spi.h
index 3ab678734c174..443a5053c0128 100644
--- a/drivers/nfc/st95hf/spi.h
+++ b/drivers/nfc/st95hf/spi.h
@@ -45,7 +45,7 @@ int st95hf_spi_send(struct st95hf_spi_context *spicontext,
enum req_type reqtype);
int st95hf_spi_recv_response(struct st95hf_spi_context *spicontext,
- unsigned char *receivebuff);
+ unsigned char *receivebuff, int buff_len);
int st95hf_spi_recv_echo_res(struct st95hf_spi_context *spicontext,
unsigned char *receivebuff);
--
2.51.0
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] nfc: st95hf: validate device-reported response length before reading
2026-08-19 14:55 [PATCH] nfc: st95hf: validate device-reported response length before reading Tianchu Chen
@ 2026-09-20 23:02 ` David Heidelberg
2026-09-23 8:20 ` Tianchu Chen
2026-09-23 8:18 ` [PATCH v2] " Tianchu Chen
1 sibling, 1 reply; 5+ messages in thread
From: David Heidelberg @ 2026-09-20 23:02 UTC (permalink / raw)
To: Tianchu Chen, sameo, shikha.singh; +Cc: oe-linux-nfc, linux-kernel
On 19/08/2026 16:55, Tianchu Chen wrote:
> From: Tianchu Chen <flynnnchen@tencent.com>
>
> st95hf_spi_recv_response() reads a 2-byte header from the device,
> derives the total response length from it (up to 1025 bytes via the
> long-frame encoding) and then blindly reads len - 2 more bytes into
> the caller's buffer, whose size it knows nothing about.
>
> Call sites can be overflowed by a device reporting a large length,
> for example, st95hf_irq_thread_handler() passes the data area of a
> 280-byte skb.
>
> ST95HF datasheet states "In Reader mode it is possible to receive up to
> 528 bytes of frame data from VICC and TypeB cards", so technically a bogus
> NFC tag may trigger the OOB-write if the card-reader's firmware allows
> large packets.
>
> Add a buff_len parameter and reject a device-reported length that does
> not fit into the caller's buffer before issuing the second SPI
> transfer. Also modify callers to pass their real buffer sizes.
>
> Discovered by Atuin - Automated Vulnerability Discovery Engine.
>
> Fixes: cab47333f0f75 ("NFC: Add STMicroelectronics ST95HF driver")
> Cc: stable@vger.kernel.org
> Signed-off-by: Tianchu Chen <flynnnchen@tencent.com>
> ---
> drivers/nfc/st95hf/core.c | 6 ++++--
> drivers/nfc/st95hf/spi.c | 7 ++++++-
> drivers/nfc/st95hf/spi.h | 2 +-
> 3 files changed, 11 insertions(+), 4 deletions(-)
>
Hello Tianchu,
please check and address the Sashiko output.
Thank you
David
[ ... ]
> @@ -106,6 +106,11 @@ int st95hf_spi_recv_response(struct st95hf_spi_context
*spicontext,
> else
> len += receivebuff[1];
>
> + if (len > buff_len) {
> + mutex_unlock(&spicontext->spi_lock);
> + return -E2BIG;
> + }
> +
Does this early return path leave the spi chip select (CS) permanently
asserted?
The first spi transfer uses .cs_change = 1 to keep the chip select active.
Skipping the second transfer would prevent the chip select from being
deasserted, potentially deadlocking the device.
> /* Now make a transfer to read only relevant bytes */
> tx_takedata.rx_buf = &receivebuff[2];
> tx_takedata.len = len - 2;
^ permalink raw reply [flat|nested] 5+ messages in thread
* [PATCH v2] nfc: st95hf: validate device-reported response length before reading
2026-08-19 14:55 [PATCH] nfc: st95hf: validate device-reported response length before reading Tianchu Chen
2026-09-20 23:02 ` David Heidelberg
@ 2026-09-23 8:18 ` Tianchu Chen
2026-09-23 9:56 ` David Heidelberg
1 sibling, 1 reply; 5+ messages in thread
From: Tianchu Chen @ 2026-09-23 8:18 UTC (permalink / raw)
To: david, sameo, shikha.singh; +Cc: oe-linux-nfc, linux-kernel
From: Tianchu Chen <flynnnchen@tencent.com>
st95hf_spi_recv_response() reads a 2-byte header from the device,
derives the total response length from it (up to 1025 bytes via the
long-frame encoding) and then blindly reads len - 2 more bytes into
the caller's buffer, whose size it knows nothing about.
Call sites can be overflowed by a device reporting a large length,
for example, st95hf_irq_thread_handler() passes the data area of a
280-byte skb.
ST95HF datasheet states "In Reader mode it is possible to receive up to
528 bytes of frame data from VICC and TypeB cards", so technically a bogus
NFC tag may trigger the OOB-write if the card-reader's firmware allows
large packets.
Add a buff_len parameter and clamp the device-reported length that does
not fit into the caller's buffer, Also modify callers to pass their
real buffer sizes.
Note that the clamped transfer must still run rather than bailing
out early: the header transfer keeps chip select asserted via
cs_change, and only completing a follow-up message lets the SPI
core deassert it. The device simply discards the unread bytes
at the chip select rising edge, then report the overflow with -E2BIG
after the transfer completes.
Discovered by Atuin - Automated Vulnerability Discovery Engine.
Fixes: cab47333f0f75 ("NFC: Add STMicroelectronics ST95HF driver")
Cc: stable@vger.kernel.org
Assisted-by: LLM
Signed-off-by: Tianchu Chen <flynnnchen@tencent.com>
---
v2: clamp the payload read to the caller's buffer and let the second
SPI transfer complete instead of returning early, so chip select (kept
asserted by the header transfer's cs_change) is deasserted by the SPI
core; report the overflow with -E2BIG after the transfer completes.
drivers/nfc/st95hf/core.c | 6 ++++--
drivers/nfc/st95hf/spi.c | 16 +++++++++++++---
drivers/nfc/st95hf/spi.h | 2 +-
3 files changed, 18 insertions(+), 6 deletions(-)
diff --git a/drivers/nfc/st95hf/core.c b/drivers/nfc/st95hf/core.c
index 4d772a308bfff..0fbb60deada23 100644
--- a/drivers/nfc/st95hf/core.c
+++ b/drivers/nfc/st95hf/core.c
@@ -283,7 +283,8 @@ static int st95hf_send_recv_cmd(struct st95hf_context *st95context,
unsigned char st95hf_response_arr[2];
ret = st95hf_spi_recv_response(&st95context->spicontext,
- st95hf_response_arr);
+ st95hf_response_arr,
+ sizeof(st95hf_response_arr));
if (ret < 0) {
dev_err(dev, "spi error from st95hf_spi_recv_response(), err = 0x%x\n",
ret);
@@ -800,7 +801,8 @@ static irqreturn_t st95hf_irq_thread_handler(int irq, void *st95hfcontext)
mutex_lock(&stcontext->rm_lock);
res_len = st95hf_spi_recv_response(&stcontext->spicontext,
- skb_resp->data);
+ skb_resp->data,
+ skb_tailroom(skb_resp));
if (res_len < 0) {
dev_err(spidevice, "TISR spi response err = 0x%x\n", res_len);
result = res_len;
diff --git a/drivers/nfc/st95hf/spi.c b/drivers/nfc/st95hf/spi.c
index ffaf2789c4069..a69f7c5e3695e 100644
--- a/drivers/nfc/st95hf/spi.c
+++ b/drivers/nfc/st95hf/spi.c
@@ -66,7 +66,7 @@ EXPORT_SYMBOL_GPL(st95hf_spi_send);
/* Function to Receive command Response */
int st95hf_spi_recv_response(struct st95hf_spi_context *spicontext,
- unsigned char *receivebuff)
+ unsigned char *receivebuff, int buff_len)
{
int len = 0;
struct spi_transfer tx_takedata;
@@ -106,9 +106,16 @@ int st95hf_spi_recv_response(struct st95hf_spi_context *spicontext,
else
len += receivebuff[1];
- /* Now make a transfer to read only relevant bytes */
+ /*
+ * Now make a transfer to read only relevant bytes, clamped to
+ * the caller's buffer. The transfer must run even when the
+ * device overreports the length: the header transfer above kept
+ * chip select asserted via cs_change, and only completing this
+ * message lets the SPI core deassert it. The device discards
+ * the unread bytes at the chip select rising edge.
+ */
tx_takedata.rx_buf = &receivebuff[2];
- tx_takedata.len = len - 2;
+ tx_takedata.len = min(len, buff_len) - 2;
spi_message_init(&m);
spi_message_add_tail(&tx_takedata, &m);
@@ -122,6 +129,9 @@ int st95hf_spi_recv_response(struct st95hf_spi_context *spicontext,
return ret;
}
+ if (len > buff_len)
+ return -E2BIG;
+
return len;
}
EXPORT_SYMBOL_GPL(st95hf_spi_recv_response);
diff --git a/drivers/nfc/st95hf/spi.h b/drivers/nfc/st95hf/spi.h
index 3ab678734c174..443a5053c0128 100644
--- a/drivers/nfc/st95hf/spi.h
+++ b/drivers/nfc/st95hf/spi.h
@@ -45,7 +45,7 @@ int st95hf_spi_send(struct st95hf_spi_context *spicontext,
enum req_type reqtype);
int st95hf_spi_recv_response(struct st95hf_spi_context *spicontext,
- unsigned char *receivebuff);
+ unsigned char *receivebuff, int buff_len);
int st95hf_spi_recv_echo_res(struct st95hf_spi_context *spicontext,
unsigned char *receivebuff);
--
2.51.0
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] nfc: st95hf: validate device-reported response length before reading
2026-09-20 23:02 ` David Heidelberg
@ 2026-09-23 8:20 ` Tianchu Chen
0 siblings, 0 replies; 5+ messages in thread
From: Tianchu Chen @ 2026-09-23 8:20 UTC (permalink / raw)
To: David Heidelberg, sameo, shikha.singh; +Cc: oe-linux-nfc, linux-kernel
September 21, 2026 at 7:02 AM, "David Heidelberg" <david@ixit.cz mailto:david@ixit.cz?to=%22David%20Heidelberg%22%20%3Cdavid%40ixit.cz%3E > wrote:
>
> On 19/08/2026 16:55, Tianchu Chen wrote:
>
> >
> > From: Tianchu Chen <flynnnchen@tencent.com>
> > st95hf_spi_recv_response() reads a 2-byte header from the device,
> > derives the total response length from it (up to 1025 bytes via the
> > long-frame encoding) and then blindly reads len - 2 more bytes into
> > the caller's buffer, whose size it knows nothing about.
> > Call sites can be overflowed by a device reporting a large length,
> > for example, st95hf_irq_thread_handler() passes the data area of a
> > 280-byte skb.
> > ST95HF datasheet states "In Reader mode it is possible to receive up to
> > 528 bytes of frame data from VICC and TypeB cards", so technically a bogus
> > NFC tag may trigger the OOB-write if the card-reader's firmware allows
> > large packets.
> > Add a buff_len parameter and reject a device-reported length that does
> > not fit into the caller's buffer before issuing the second SPI
> > transfer. Also modify callers to pass their real buffer sizes.
> > Discovered by Atuin - Automated Vulnerability Discovery Engine.
> > Fixes: cab47333f0f75 ("NFC: Add STMicroelectronics ST95HF driver")
> > Cc: stable@vger.kernel.org
> > Signed-off-by: Tianchu Chen <flynnnchen@tencent.com>
> > ---
> > drivers/nfc/st95hf/core.c | 6 ++++--
> > drivers/nfc/st95hf/spi.c | 7 ++++++-
> > drivers/nfc/st95hf/spi.h | 2 +-
> > 3 files changed, 11 insertions(+), 4 deletions(-)
> >
> Hello Tianchu,
>
> please check and address the Sashiko output.
>
> Thank you
> David
>
> [ ... ]
> > @@ -106,6 +106,11 @@ int st95hf_spi_recv_response(struct st95hf_spi_context *spicontext,
> > else
> > len += receivebuff[1];
> >
> > + if (len > buff_len) {
> > + mutex_unlock(&spicontext->spi_lock);
> > + return -E2BIG;
> > + }
> > +
>
> Does this early return path leave the spi chip select (CS) permanently
> asserted?
>
> The first spi transfer uses .cs_change = 1 to keep the chip select active.
> Skipping the second transfer would prevent the chip select from being
> deasserted, potentially deadlocking the device.
>
> > /* Now make a transfer to read only relevant bytes */
> > tx_takedata.rx_buf = &receivebuff[2];
> > tx_takedata.len = len - 2;
>
Hi David,
Yes, Sashiko was right on this case, we should gracefully end the spi transfer.
V2 patch has been sent: https://lore.kernel.org/all/fe549eb46f9f666f2778e7c50198528028cf9528@linux.dev/
Best regards,
Tianchu
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH v2] nfc: st95hf: validate device-reported response length before reading
2026-09-23 8:18 ` [PATCH v2] " Tianchu Chen
@ 2026-09-23 9:56 ` David Heidelberg
0 siblings, 0 replies; 5+ messages in thread
From: David Heidelberg @ 2026-09-23 9:56 UTC (permalink / raw)
To: Tianchu Chen, sameo, shikha.singh; +Cc: oe-linux-nfc, linux-kernel
On 23/09/2026 10:18, Tianchu Chen wrote:
> From: Tianchu Chen <flynnnchen@tencent.com>
>
> st95hf_spi_recv_response() reads a 2-byte header from the device,
> derives the total response length from it (up to 1025 bytes via the
> long-frame encoding) and then blindly reads len - 2 more bytes into
> the caller's buffer, whose size it knows nothing about.
>
> Call sites can be overflowed by a device reporting a large length,
> for example, st95hf_irq_thread_handler() passes the data area of a
> 280-byte skb.
>
> ST95HF datasheet states "In Reader mode it is possible to receive up to
> 528 bytes of frame data from VICC and TypeB cards", so technically a bogus
> NFC tag may trigger the OOB-write if the card-reader's firmware allows
> large packets.
>
> Add a buff_len parameter and clamp the device-reported length that does
> not fit into the caller's buffer, Also modify callers to pass their
> real buffer sizes.
>
> Note that the clamped transfer must still run rather than bailing
> out early: the header transfer keeps chip select asserted via
> cs_change, and only completing a follow-up message lets the SPI
> core deassert it. The device simply discards the unread bytes
> at the chip select rising edge, then report the overflow with -E2BIG
> after the transfer completes.
>
> Discovered by Atuin - Automated Vulnerability Discovery Engine.
>
> Fixes: cab47333f0f75 ("NFC: Add STMicroelectronics ST95HF driver")
> Cc: stable@vger.kernel.org
> Assisted-by: LLM
> Signed-off-by: Tianchu Chen <flynnnchen@tencent.com>
> ---
> v2: clamp the payload read to the caller's buffer and let the second
> SPI transfer complete instead of returning early, so chip select (kept
> asserted by the header transfer's cs_change) is deasserted by the SPI
> core; report the overflow with -E2BIG after the transfer completes.
Hello Tianchu,
please never send follow-ups as part of the thread. Always send them as a
separate message (otherwise they may get lost and we already have patchwork wich
can track the submissions.
Even better, try to use tools such as b4 to submit patches (easier for you, sent
in expected format for us).
Thanks
David
>
> drivers/nfc/st95hf/core.c | 6 ++++--
> drivers/nfc/st95hf/spi.c | 16 +++++++++++++---
> drivers/nfc/st95hf/spi.h | 2 +-
> 3 files changed, 18 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/nfc/st95hf/core.c b/drivers/nfc/st95hf/core.c
> index 4d772a308bfff..0fbb60deada23 100644
> --- a/drivers/nfc/st95hf/core.c
> +++ b/drivers/nfc/st95hf/core.c
> @@ -283,7 +283,8 @@ static int st95hf_send_recv_cmd(struct st95hf_context *st95context,
> unsigned char st95hf_response_arr[2];
>
> ret = st95hf_spi_recv_response(&st95context->spicontext,
> - st95hf_response_arr);
> + st95hf_response_arr,
> + sizeof(st95hf_response_arr));
> if (ret < 0) {
> dev_err(dev, "spi error from st95hf_spi_recv_response(), err = 0x%x\n",
> ret);
> @@ -800,7 +801,8 @@ static irqreturn_t st95hf_irq_thread_handler(int irq, void *st95hfcontext)
>
> mutex_lock(&stcontext->rm_lock);
> res_len = st95hf_spi_recv_response(&stcontext->spicontext,
> - skb_resp->data);
> + skb_resp->data,
> + skb_tailroom(skb_resp));
> if (res_len < 0) {
> dev_err(spidevice, "TISR spi response err = 0x%x\n", res_len);
> result = res_len;
> diff --git a/drivers/nfc/st95hf/spi.c b/drivers/nfc/st95hf/spi.c
> index ffaf2789c4069..a69f7c5e3695e 100644
> --- a/drivers/nfc/st95hf/spi.c
> +++ b/drivers/nfc/st95hf/spi.c
> @@ -66,7 +66,7 @@ EXPORT_SYMBOL_GPL(st95hf_spi_send);
>
> /* Function to Receive command Response */
> int st95hf_spi_recv_response(struct st95hf_spi_context *spicontext,
> - unsigned char *receivebuff)
> + unsigned char *receivebuff, int buff_len)
> {
> int len = 0;
> struct spi_transfer tx_takedata;
> @@ -106,9 +106,16 @@ int st95hf_spi_recv_response(struct st95hf_spi_context *spicontext,
> else
> len += receivebuff[1];
>
> - /* Now make a transfer to read only relevant bytes */
> + /*
> + * Now make a transfer to read only relevant bytes, clamped to
> + * the caller's buffer. The transfer must run even when the
> + * device overreports the length: the header transfer above kept
> + * chip select asserted via cs_change, and only completing this
> + * message lets the SPI core deassert it. The device discards
> + * the unread bytes at the chip select rising edge.
> + */
> tx_takedata.rx_buf = &receivebuff[2];
> - tx_takedata.len = len - 2;
> + tx_takedata.len = min(len, buff_len) - 2;
>
> spi_message_init(&m);
> spi_message_add_tail(&tx_takedata, &m);
> @@ -122,6 +129,9 @@ int st95hf_spi_recv_response(struct st95hf_spi_context *spicontext,
> return ret;
> }
>
> + if (len > buff_len)
> + return -E2BIG;
> +
> return len;
> }
> EXPORT_SYMBOL_GPL(st95hf_spi_recv_response);
> diff --git a/drivers/nfc/st95hf/spi.h b/drivers/nfc/st95hf/spi.h
> index 3ab678734c174..443a5053c0128 100644
> --- a/drivers/nfc/st95hf/spi.h
> +++ b/drivers/nfc/st95hf/spi.h
> @@ -45,7 +45,7 @@ int st95hf_spi_send(struct st95hf_spi_context *spicontext,
> enum req_type reqtype);
>
> int st95hf_spi_recv_response(struct st95hf_spi_context *spicontext,
> - unsigned char *receivebuff);
> + unsigned char *receivebuff, int buff_len);
>
> int st95hf_spi_recv_echo_res(struct st95hf_spi_context *spicontext,
> unsigned char *receivebuff);
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2026-09-23 9:56 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2026-08-19 14:55 [PATCH] nfc: st95hf: validate device-reported response length before reading Tianchu Chen
2026-09-20 23:02 ` David Heidelberg
2026-09-23 8:20 ` Tianchu Chen
2026-09-23 8:18 ` [PATCH v2] " Tianchu Chen
2026-09-23 9:56 ` David Heidelberg
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox
all inboxes | Powered by JetHome®