Re: [PATCH v2] nfc: pn533: bounds-check the ATR_RES response before parsing it

[Simon Horman]

On Mon, Nov 09, 2026 at 08:00:16PM +0900, hyunjungg wrote:
> From: Hyunjung Ko <hj351016@xxxxxxxxx>
> 
> pn533_poll_dep_complete(), pn533_in_dep_link_up_complete() and
> pn533_activate_target_nfcdep() cast resp->data to the fixed ATR_RES
> header (struct pn533_cmd_jump_dep_response / pn533_cmd_activate_response)
> and, on a successful response, read nfcid3t[] and compute the general-
> bytes length as resp->len - sizeof(header). A short response leaves that
> read past the end of the skb data, an out-of-bounds read; the copied
> nfcid3t reaches userspace via nfc_targets_found() and netlink.
> 
> Only the success path needs the full fixed header: a no-peer/failure
> reply is just the status byte, and the existing failure handling (turn
> the radio off and re-arm polling, or print the error) must keep running
> for it. So validate resp->len >= sizeof(*rsp) after the status check, on
> the success path, in all three handlers, before nfcid3t and the general
> bytes are read.
> 
> Found by code review; not reproduced.
> 
> Fixes: 361f3cb7f9cf ("NFC: DEP link hook implementation for pn533")
> Fixes: 673088fb42d0 ("NFC: pn533: Send ATR_REQ directly for active device detection")
> Cc: stable@xxxxxxxxxxxxxxx
> Signed-off-by: Hyunjung Ko <hj351016@xxxxxxxxx>
> ---
> Changes in v2 (thanks to the Sashiko AI review):
>  - move the length check to the SUCCESS path (after the status check), so
>    a no-peer/timeout reply (resp->len == 1) still takes the existing
>    "turn radio off, re-arm polling" / error path instead of returning
>    -EINVAL and stalling the poll loop
>  - also guard pn533_activate_target_nfcdep(), the third ATR_RES parser
>  - add Fixes: tags for the two commits that introduced the parsing
>  - drop the inaccurate claim that the other handlers already length-check
>    resp->len (only pn533_init_target_complete() does)

Reviewed-by: Simon Horman <horms@xxxxxxxxxx>