Re: [PATCH net v2 1/3] nfc: nci: fix u8 underflow in nci_store_general_bytes_nfc_dep

[Simon Horman <horms@kernel.org>]

On Thu, Apr 09, 2026 at 08:59:56PM +0200, Lekë Hapçiu wrote:
> From: Lekë Hapçiu <framemain@outlook.com>
> 
> nci_store_general_bytes_nfc_dep() computes the number of General Bytes
> to copy from an ATR_RES or ATR_REQ frame by subtracting a fixed header
> offset from the peer-supplied length field:
> 
>   ndev->remote_gb_len = min_t(__u8,
>       (atr_res_len - NFC_ATR_RES_GT_OFFSET),   /* offset = 15 */
>       NFC_ATR_RES_GB_MAXSIZE);
> 
> Both length fields are __u8.  When a malicious NFC-DEP target (POLL mode)
> or initiator (LISTEN mode) sends an ATR_RES/ATR_REQ whose length field is
> smaller than the fixed offset (< 15 or < 14 respectively), the subtraction
> wraps in unsigned u8 arithmetic:
> 
>   e.g. atr_res_len = 0 -> (u8)(0 - 15) = 241
> 
> min_t(__u8, 241, 47) then yields 47, so the subsequent memcpy reads
> 47 bytes from beyond the end of the valid activation parameter data into
> ndev->remote_gb[].  This buffer is later passed to nfc_llcp_parse_gb_tlv()
> as a TLV array, feeding directly into the TLV parser hardened by the
> companion patch.
> 
> Fix: add an explicit lower-bound check on each length field before the
> subtraction.  If the length is smaller than the required offset the frame
> is malformed; leave remote_gb_len at zero and skip the memcpy.
> 
> Both the POLL (atr_res_len / NFC_ATR_RES_GT_OFFSET = 15) and the LISTEN
> (atr_req_len / NFC_ATR_REQ_GT_OFFSET = 14) paths are affected; both are
> fixed symmetrically.
> 
> Reachability: the ATR_RES is sent by an NFC-DEP target during RF
> activation, before any authentication or pairing.  The bug is therefore
> reachable from any NFC peer within ~4 cm.
> 
> Fixes: a99903ec4566 ("NFC: NCI: Handle Target mode activation")

The above commit seems to move rather than add the logic in question.
It seems to me that the following would be the fixes tag corresponding
to the commit that introduced this problem.

Fixes: 767f19ae698e ("NFC: Implement NCI dep_link_up and dep_link_down")

> Cc: stable@vger.kernel.org
> Signed-off-by: Lekë Hapçiu <framemain@outlook.com>
> ---
>  net/nfc/nci/ntf.c | 22 ++++++++++++++--------
>  1 file changed, 14 insertions(+), 8 deletions(-)
> 
> diff --git a/net/nfc/nci/ntf.c b/net/nfc/nci/ntf.c
> index c96512bb8..8eb295580 100644
> --- a/net/nfc/nci/ntf.c
> +++ b/net/nfc/nci/ntf.c
> @@ -631,25 +631,31 @@ static int nci_store_general_bytes_nfc_dep(struct nci_dev *ndev,
>  	switch (ntf->activation_rf_tech_and_mode) {
>  	case NCI_NFC_A_PASSIVE_POLL_MODE:
>  	case NCI_NFC_F_PASSIVE_POLL_MODE:
> +		if (ntf->activation_params.poll_nfc_dep.atr_res_len <
> +		    NFC_ATR_RES_GT_OFFSET)
> +			break;
>  		ndev->remote_gb_len = min_t(__u8,
> -			(ntf->activation_params.poll_nfc_dep.atr_res_len
> -						- NFC_ATR_RES_GT_OFFSET),
> +			ntf->activation_params.poll_nfc_dep.atr_res_len
> +						- NFC_ATR_RES_GT_OFFSET,
>  			NFC_ATR_RES_GB_MAXSIZE);

I'm not suggesting changing this, at least not as part of this bug fix, so
this comment is FTR: As NFC_ATR_RES_GB_MAXSIZE is a compile time constant,
and the condition added by this patch ensures that the result of the
subtraction is not negative, I strongly suspect that using min() here
sufficient and thus more appropriate than min_t().

>  		memcpy(ndev->remote_gb,
> -		       (ntf->activation_params.poll_nfc_dep.atr_res
> -						+ NFC_ATR_RES_GT_OFFSET),
> +		       ntf->activation_params.poll_nfc_dep.atr_res
> +						+ NFC_ATR_RES_GT_OFFSET,
>  		       ndev->remote_gb_len);
>  		break;
>  
>  	case NCI_NFC_A_PASSIVE_LISTEN_MODE:
>  	case NCI_NFC_F_PASSIVE_LISTEN_MODE:
> +		if (ntf->activation_params.listen_nfc_dep.atr_req_len <
> +		    NFC_ATR_REQ_GT_OFFSET)
> +			break;
>  		ndev->remote_gb_len = min_t(__u8,
> -			(ntf->activation_params.listen_nfc_dep.atr_req_len
> -						- NFC_ATR_REQ_GT_OFFSET),
> +			ntf->activation_params.listen_nfc_dep.atr_req_len
> +						- NFC_ATR_REQ_GT_OFFSET,
>  			NFC_ATR_REQ_GB_MAXSIZE);
>  		memcpy(ndev->remote_gb,
> -		       (ntf->activation_params.listen_nfc_dep.atr_req
> -						+ NFC_ATR_REQ_GT_OFFSET),
> +		       ntf->activation_params.listen_nfc_dep.atr_req
> +						+ NFC_ATR_REQ_GT_OFFSET,
>  		       ndev->remote_gb_len);
>  		break;
>  
> -- 
> 2.51.0
>