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

[David Heidelberg <david@ixit.cz>]

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;