Re: [PATCH] nfc: pn533: fix OOB read in pn533_acr122_is_rx_frame_valid()

[Johan Hovold <johan@kernel.org>]

On Sun, Sep 06, 2026 at 11:13:36AM +0530, Deepanshu Kartikey wrote:
> frame->ccid.datalen is read directly from the USB response frame
> and used, unchecked, as an index into frame->data[]. A malicious or
> malfunctioning device can set this field to an arbitrary value,
> causing the driver to read far outside the received buffer.
> 
> Bound ccid.datalen against the maximum possible ACR122 frame size
> before using it, and reject values less than 2 to avoid the
> "datalen - 2" underflowing.
> 
> Fixes: 9815c7cf22da ("NFC: pn533: Separate physical layer from the core implementation")
> Reported-by: syzbot+1853daab1a47603d4678@syzkaller.appspotmail.com
> Closes: https://syzkaller.appspot.com/bug?extid=1853daab1a47603d4678
> Tested-by: syzbot+1853daab1a47603d4678@syzkaller.appspotmail.com
> Signed-off-by: Deepanshu Kartikey <kartikey406@gmail.com>

Are you missing an Assisted-by tag here?

> ---
>  drivers/nfc/pn533/usb.c | 5 +++++
>  1 file changed, 5 insertions(+)
> 
> diff --git a/drivers/nfc/pn533/usb.c b/drivers/nfc/pn533/usb.c
> index efb07f944fce..d08d3b0df988 100644
> --- a/drivers/nfc/pn533/usb.c
> +++ b/drivers/nfc/pn533/usb.c
> @@ -322,6 +322,11 @@ static bool pn533_acr122_is_rx_frame_valid(void *_frame, struct pn533 *dev)
>  	if (!frame->ccid.datalen)
>  		return false;
>  
> +	if (frame->ccid.datalen < 2 ||
> +	    frame->ccid.datalen > PN533_ACR122_FRAME_MAX_PAYLOAD_LEN +
> +	    PN533_ACR122_RX_FRAME_TAIL_LEN)
> +		return false;
> +

Just a drive-by comment, but should this not replace the existing
datalen == 0 check?

>  	if (frame->data[frame->ccid.datalen - 2] == 0x63)
>  		return false;

Johan