[<prev] [next>] [<thread-prev] [day] [month] [year] [list]
Message-ID: <095dd1c7-58fe-4fa7-9ed5-dbfa22911e90@hale.at>
Date: Mon, 12 Jan 2026 09:55:27 +0100
From: Michael Thalmeier <michael.thalmeier@...e.at>
To: Jakub Kicinski <kuba@...nel.org>
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
Am 08.01.26 um 03:15 schrieb Jakub Kicinski:
> 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?
Will be sending an updated patch with validation in the helpers.
>
> 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.
Yes, we are using the Linux NFC implementation with the NCI driver (with
a NXP PN7150 NFC chip) in production devices. It was actually working
quite well and stable until the referenced commit completely broke it.
Powered by blists - more mailing lists