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

Paolo Abeni
2026-04-14T07:34:11Z

On 4/9/26 8:59 PM, 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")
> 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;

This does not look the right fix: nci_store_general_bytes_nfc_dep() will
return success to the caller, and processing will proceed even if the
packet is malformed.

Looking at the (rather incomplete) error handling in
nci_rf_intf_activated_ntf_packet(), the latter function should error out
with EINVAL for truncated/malformed packets.

You should return a proper error code here _and_ handle such error in
nci_rf_intf_activated_ntf_packet().

The same comment applies to the simlar check below.

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

Please do not include style-related changes in 'net' fix: it should
include the minimal delta to address the issue.

Other similar chuncks below.

/P