Re: [PATCH] nfc: st95hf: validate device-reported response length before reading

[David Heidelberg]

On 19/08/2026 16:55, Tianchu Chen wrote:
> From: Tianchu Chen <flynnnchen@xxxxxxxxxxx>
>
> 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@xxxxxxxxxxxxxxx
> Signed-off-by: Tianchu Chen <flynnnchen@xxxxxxxxxxx>
> ---
>   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;