From: Simon Horman <horms@kernel.org>
To: "Lekë Hapçiu" <snowwlake@icloud.com>
Cc: netdev@vger.kernel.org, davem@davemloft.net, edumazet@google.com,
kuba@kernel.org, pabeni@redhat.com, linux-kernel@vger.kernel.org,
stable@vger.kernel.org, "Lekë Hapçiu" <framemain@outlook.com>
Subject: Re: [PATCH net v3 1/4] nfc: nci: fix u8 underflow in nci_store_general_bytes_nfc_dep
Date: Fri, 17 Apr 2026 14:00:22 +0100 [thread overview]
Message-ID: <20260417130022.GC31784@horms.kernel.org> (raw)
In-Reply-To: <20260414233534.55973-2-snowwlake@icloud.com>
On Wed, Apr 15, 2026 at 01:35:30AM +0200, Lekë Hapçiu wrote:
> From: Lekë Hapçiu <framemain@outlook.com>
>
> nci_store_general_bytes_nfc_dep() computes the General Bytes length by
> subtracting a fixed header offset from the peer-supplied atr_res_len
> (POLL) or atr_req_len (LISTEN) 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 peer sends an
> ATR_RES/ATR_REQ whose length is smaller than the fixed offset (< 15
> or < 14 respectively), the subtraction wraps:
>
> atr_res_len = 0 -> (u8)(0 - 15) = 241
> min_t(__u8, 241, NFC_ATR_RES_GB_MAXSIZE=47) = 47
>
> The subsequent memcpy then reads 47 bytes beyond the valid activation
> parameter data into ndev->remote_gb[]. This buffer is later fed to
> nfc_llcp_parse_gb_tlv() as a TLV array.
>
> Reject the frame with NCI_STATUS_RF_PROTOCOL_ERROR when the length is
> below the required offset, and propagate the error out of
> nci_rf_intf_activated_ntf_packet() instead of silently accepting the
> malformed packet.
This does not seem to be consistent with the handling of other in
nci_rf_intf_activated_ntf_packet() when it calls other functions similar to
nci_rf_intf_activated_ntf_packet().
I suggest dropping this part of the fix, and addressing
nci_rf_intf_activated_ntf_packet() in a more holistic manner
if this kind of change is desired.
>
> Reachable from any NFC peer within ~4 cm during RF activation, prior
> to any pairing.
I do not understand how this statement relates to this change.
Could you explain?
>
> Fixes: c4fbb6515709 ("NFC: NCI: Add NFC-DEP support to NCI data exchange")
I am unable to find a commit with either that hash or subject.
It seems to me that this problem was introduced in:
767f19ae698e ("NFC: Implement NCI dep_link_up and dep_link_down")
--
pw-bot: changes-requested
prev parent reply other threads:[~2026-04-17 13:00 UTC|newest]
Thread overview: 2+ messages / expand[flat|nested] mbox.gz Atom feed top
2026-04-14 23:35 [PATCH net v3 1/4] nfc: nci: fix u8 underflow in nci_store_general_bytes_nfc_dep Lekë Hapçiu
2026-04-17 13:00 ` Simon Horman [this message]
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20260417130022.GC31784@horms.kernel.org \
--to=horms@kernel.org \
--cc=davem@davemloft.net \
--cc=edumazet@google.com \
--cc=framemain@outlook.com \
--cc=kuba@kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=netdev@vger.kernel.org \
--cc=pabeni@redhat.com \
--cc=snowwlake@icloud.com \
--cc=stable@vger.kernel.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox