* [PATCH net] nfc: fdp: bound the device-supplied read size in fdp_nci_i2c_read()
@ 2026-07-11 12:36 Doruk Tan Ozturk
2026-07-20 10:16 ` Simon Horman
0 siblings, 1 reply; 4+ messages in thread
From: Doruk Tan Ozturk @ 2026-07-11 12:36 UTC (permalink / raw)
To: david; +Cc: oe-linux-nfc, netdev, linux-kernel, stable
fdp_nci_i2c_read() reads a "length packet" from the FDP I2C controller and
computes the size of the next I2C transfer from two device-supplied bytes:
phy->next_read_size = (tmp[2] << 8) + tmp[3] + 3;
next_read_size is a u16 (up to 65535) and is never bounded. On the next
loop iteration it is used directly as the length passed to
i2c_master_recv(client, tmp, len);
which reads into the fixed 261-byte stack buffer
tmp[FDP_NCI_I2C_MAX_PAYLOAD]. A malicious or malfunctioning controller
that reports a large length thus overflows the stack buffer -- the
r != len check runs only after the read has already happened.
Reject a next-read size larger than the buffer and resynchronize.
Found by 0sec (https://0sec.ai) using automated source analysis; the
missing bound is evident from source. Compile-tested.
Fixes: a06347c04c13 ("NFC: Add Intel Fields Peak NFC solution driver")
Cc: stable@vger.kernel.org
Assisted-by: 0sec:claude-opus-4-8
Signed-off-by: Doruk Tan Ozturk <doruk@0sec.ai>
---
drivers/nfc/fdp/i2c.c | 9 ++++++++-
1 file changed, 8 insertions(+), 1 deletion(-)
diff --git a/drivers/nfc/fdp/i2c.c b/drivers/nfc/fdp/i2c.c
index c1896a1d978c..581f85f0dfa8 100644
--- a/drivers/nfc/fdp/i2c.c
+++ b/drivers/nfc/fdp/i2c.c
@@ -128,7 +128,7 @@ static const struct nfc_phy_ops i2c_phy_ops = {
static int fdp_nci_i2c_read(struct fdp_i2c_phy *phy, struct sk_buff **skb)
{
- int r, len;
+ int r = -EREMOTEIO, len;
u8 tmp[FDP_NCI_I2C_MAX_PAYLOAD], lrc, k;
u16 i;
struct i2c_client *client = phy->i2c_dev;
@@ -140,6 +140,13 @@ static int fdp_nci_i2c_read(struct fdp_i2c_phy *phy, struct sk_buff **skb)
len = phy->next_read_size;
+ if (len > FDP_NCI_I2C_MAX_PAYLOAD) {
+ dev_dbg(&client->dev, "%s: read size %d too large\n",
+ __func__, len);
+ phy->next_read_size = FDP_NCI_I2C_MIN_PAYLOAD;
+ goto flush;
+ }
+
r = i2c_master_recv(client, tmp, len);
if (r != len) {
dev_dbg(&client->dev, "%s: i2c recv err: %d\n",
--
2.43.0
^ permalink raw reply related [flat|nested] 4+ messages in thread
* Re: [PATCH net] nfc: fdp: bound the device-supplied read size in fdp_nci_i2c_read()
2026-07-11 12:36 [PATCH net] nfc: fdp: bound the device-supplied read size in fdp_nci_i2c_read() Doruk Tan Ozturk
@ 2026-07-20 10:16 ` Simon Horman
2026-07-20 10:19 ` David Heidelberg
0 siblings, 1 reply; 4+ messages in thread
From: Simon Horman @ 2026-07-20 10:16 UTC (permalink / raw)
To: Doruk Tan Ozturk; +Cc: david, oe-linux-nfc, netdev, linux-kernel, stable
On Sat, Jul 11, 2026 at 02:36:41PM +0200, Doruk Tan Ozturk wrote:
> fdp_nci_i2c_read() reads a "length packet" from the FDP I2C controller and
> computes the size of the next I2C transfer from two device-supplied bytes:
>
> phy->next_read_size = (tmp[2] << 8) + tmp[3] + 3;
>
> next_read_size is a u16 (up to 65535) and is never bounded. On the next
> loop iteration it is used directly as the length passed to
>
> i2c_master_recv(client, tmp, len);
>
> which reads into the fixed 261-byte stack buffer
> tmp[FDP_NCI_I2C_MAX_PAYLOAD]. A malicious or malfunctioning controller
> that reports a large length thus overflows the stack buffer -- the
> r != len check runs only after the read has already happened.
>
> Reject a next-read size larger than the buffer and resynchronize.
>
> Found by 0sec (https://0sec.ai) using automated source analysis; the
> missing bound is evident from source. Compile-tested.
>
> Fixes: a06347c04c13 ("NFC: Add Intel Fields Peak NFC solution driver")
> Cc: stable@vger.kernel.org
> Assisted-by: 0sec:claude-opus-4-8
> Signed-off-by: Doruk Tan Ozturk <doruk@0sec.ai>
> ---
> drivers/nfc/fdp/i2c.c | 9 ++++++++-
> 1 file changed, 8 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/nfc/fdp/i2c.c b/drivers/nfc/fdp/i2c.c
> index c1896a1d978c..581f85f0dfa8 100644
> --- a/drivers/nfc/fdp/i2c.c
> +++ b/drivers/nfc/fdp/i2c.c
> @@ -128,7 +128,7 @@ static const struct nfc_phy_ops i2c_phy_ops = {
>
> static int fdp_nci_i2c_read(struct fdp_i2c_phy *phy, struct sk_buff **skb)
> {
> - int r, len;
> + int r = -EREMOTEIO, len;
> u8 tmp[FDP_NCI_I2C_MAX_PAYLOAD], lrc, k;
> u16 i;
> struct i2c_client *client = phy->i2c_dev;
> @@ -140,6 +140,13 @@ static int fdp_nci_i2c_read(struct fdp_i2c_phy *phy, struct sk_buff **skb)
>
> len = phy->next_read_size;
>
> + if (len > FDP_NCI_I2C_MAX_PAYLOAD) {
> + dev_dbg(&client->dev, "%s: read size %d too large\n",
> + __func__, len);
> + phy->next_read_size = FDP_NCI_I2C_MIN_PAYLOAD;
I think it would be more robust to explicitly set r here.
Because it is assigned a little later in the loop, overriding
the default assignment made by the first hunk of this patch.
> + goto flush;
> + }
> +
> r = i2c_master_recv(client, tmp, len);
> if (r != len) {
> dev_dbg(&client->dev, "%s: i2c recv err: %d\n",
> --
> 2.43.0
>
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH net] nfc: fdp: bound the device-supplied read size in fdp_nci_i2c_read()
2026-07-20 10:16 ` Simon Horman
@ 2026-07-20 10:19 ` David Heidelberg
2026-07-20 13:21 ` Doruk Tan Ozturk
0 siblings, 1 reply; 4+ messages in thread
From: David Heidelberg @ 2026-07-20 10:19 UTC (permalink / raw)
To: Simon Horman, Doruk Tan Ozturk; +Cc: oe-linux-nfc, netdev, linux-kernel, stable
On 20/07/2026 12:16, Simon Horman wrote:
> On Sat, Jul 11, 2026 at 02:36:41PM +0200, Doruk Tan Ozturk wrote:
>> fdp_nci_i2c_read() reads a "length packet" from the FDP I2C controller and
>> computes the size of the next I2C transfer from two device-supplied bytes:
>>
>> phy->next_read_size = (tmp[2] << 8) + tmp[3] + 3;
>>
>> next_read_size is a u16 (up to 65535) and is never bounded. On the next
>> loop iteration it is used directly as the length passed to
>>
>> i2c_master_recv(client, tmp, len);
>>
>> which reads into the fixed 261-byte stack buffer
>> tmp[FDP_NCI_I2C_MAX_PAYLOAD]. A malicious or malfunctioning controller
>> that reports a large length thus overflows the stack buffer -- the
>> r != len check runs only after the read has already happened.
>>
>> Reject a next-read size larger than the buffer and resynchronize.
>>
>> Found by 0sec (https://0sec.ai) using automated source analysis; the
>> missing bound is evident from source. Compile-tested.
>>
>> Fixes: a06347c04c13 ("NFC: Add Intel Fields Peak NFC solution driver")
>> Cc: stable@vger.kernel.org
>> Assisted-by: 0sec:claude-opus-4-8
>> Signed-off-by: Doruk Tan Ozturk <doruk@0sec.ai>
>> ---
>> drivers/nfc/fdp/i2c.c | 9 ++++++++-
>> 1 file changed, 8 insertions(+), 1 deletion(-)
>>
>> diff --git a/drivers/nfc/fdp/i2c.c b/drivers/nfc/fdp/i2c.c
>> index c1896a1d978c..581f85f0dfa8 100644
>> --- a/drivers/nfc/fdp/i2c.c
>> +++ b/drivers/nfc/fdp/i2c.c
>> @@ -128,7 +128,7 @@ static const struct nfc_phy_ops i2c_phy_ops = {
>>
>> static int fdp_nci_i2c_read(struct fdp_i2c_phy *phy, struct sk_buff **skb)
>> {
>> - int r, len;
>> + int r = -EREMOTEIO, len;
>> u8 tmp[FDP_NCI_I2C_MAX_PAYLOAD], lrc, k;
>> u16 i;
>> struct i2c_client *client = phy->i2c_dev;
>> @@ -140,6 +140,13 @@ static int fdp_nci_i2c_read(struct fdp_i2c_phy *phy, struct sk_buff **skb)
>>
>> len = phy->next_read_size;
>>
>> + if (len > FDP_NCI_I2C_MAX_PAYLOAD) {
>> + dev_dbg(&client->dev, "%s: read size %d too large\n",
>> + __func__, len);
>> + phy->next_read_size = FDP_NCI_I2C_MIN_PAYLOAD;
>
> I think it would be more robust to explicitly set r here.
> Because it is assigned a little later in the loop, overriding
> the default assignment made by the first hunk of this patch.
I agree, I was looking at this patch yesterday and the current format feels
unfavorable.
Please send next revision with Simon suggestion :)
Thank you both!
David
>
>> + goto flush;
>> + }
>> +
>> r = i2c_master_recv(client, tmp, len);
>> if (r != len) {
>> dev_dbg(&client->dev, "%s: i2c recv err: %d\n",
>> --
>> 2.43.0
>>
--
David Heidelberg
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH net] nfc: fdp: bound the device-supplied read size in fdp_nci_i2c_read()
2026-07-20 10:19 ` David Heidelberg
@ 2026-07-20 13:21 ` Doruk Tan Ozturk
0 siblings, 0 replies; 4+ messages in thread
From: Doruk Tan Ozturk @ 2026-07-20 13:21 UTC (permalink / raw)
To: david, horms; +Cc: oe-linux-nfc, hexlabsecurity, netdev, linux-kernel
On 20/07/2026, David Heidelberg wrote:
> I agree, I was looking at this patch yesterday and the current format
> feels unfavorable. Please send next revision with Simon suggestion :)
Will do. One heads-up before I resend: Bryam Vargas posted a v2 for the
same bug back in June [1] that is more complete than mine. It also
rejects len < FDP_NCI_I2C_MIN_PAYLOAD (the lower bound Simon flagged,
reachable via the u16 truncation) and fixes an skb leak in the same
loop, with a KASAN reproducer, and it carries Simon's Suggested-by. It
looks like it just stalled.
If you would rather pick that one up, it covers more than mine does.
Happy to send my (narrower) v2 with Simon's suggestion if you prefer it
instead, just let me know.
-Doruk
[1] https://lore.kernel.org/all/20260616-b4-disp-b1f8ab4c-v2-1-2d1fe5955325@proton.me/
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2026-07-20 13:21 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-07-11 12:36 [PATCH net] nfc: fdp: bound the device-supplied read size in fdp_nci_i2c_read() Doruk Tan Ozturk
2026-07-20 10:16 ` Simon Horman
2026-07-20 10:19 ` David Heidelberg
2026-07-20 13:21 ` Doruk Tan Ozturk
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.