[<prev] [next>] [<thread-prev] [day] [month] [year] [list]
Message-ID: <20260107181519.1e6dbfc6@kernel.org>
Date: Wed, 7 Jan 2026 18:15:19 -0800
From: Jakub Kicinski <kuba@...nel.org>
To: Michael Thalmeier <michael.thalmeier@...e.at>
Cc: Deepak Sharma <deepak.sharma.472935@...il.com>, Krzysztof Kozlowski
<krzk@...nel.org>, Vadim Fedorenko <vadim.fedorenko@...ux.dev>, Simon
Horman <horms@...nel.org>, Paolo Abeni <pabeni@...hat.com>,
linux-kernel@...r.kernel.org, netdev@...r.kernel.org, Michael Thalmeier
<michael@...lmeier.at>, stable@...r.kernel.org
Subject: Re: [PATCH net v4] net: nfc: nci: Fix parameter validation for
packet data
On Wed, 7 Jan 2026 11:06:27 +0100 Michael Thalmeier wrote:
> >> @@ -380,6 +384,10 @@ static int nci_rf_discover_ntf_packet(struct nci_dev *ndev,
> >> pr_debug("rf_tech_specific_params_len %d\n",
> >> ntf.rf_tech_specific_params_len);
> >>
> >> + if (skb->len < (data - skb->data) +
> >> + ntf.rf_tech_specific_params_len + sizeof(ntf.ntf_type))
> >> + return -EINVAL;
> >
> > Are we validating ntf.rf_tech_specific_params_len against the
> > extraction logic in nci_extract_rf_params_nfca_passive_poll()
> > and friends?
>
> You are right. The current patch is only validating that the received
> packet is consistent in the way that the rf_tech_specific_params_len
> number of bytes is also contained in the buffer.
>
> There is currently no code that validates that
> nci_extract_rf_params_nfca_passive_poll and friends only access the
> given number of bytes in their logic.
> And to be frank, I do not know how to implement this without either
> cluttering the code with validation logic or re-implementing half the
> parsing logic for length validation.
Don't think there's a magic bullet, we'd either have to pass the skb
or "remaining length" to all the parsing helpers and have them ensure
they are not going out of bounds.
There doesn't seem to be a huge number of those helpers but if you
don't wanna tackle trying to validate the accesses maybe just add a
couple of comments that the validation is missing in the helpers called
in the switch statement?
BTW are you actually using the Linux NFC implementation or just playing
with it? I'm not sure I'd trust this code for anything but accumulating
C-V-Es, TBH.
Powered by blists - more mailing lists