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

[Tianchu Chen]

September 21, 2026 at 7:02 AM, "David Heidelberg" <david@xxxxxxx mailto:david@xxxxxxx?to=%22David%20Heidelberg%22%20%3Cdavid%40ixit.cz%3E > wrote:

> 
> 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;
>

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@xxxxxxxxx/

Best regards,

Tianchu