nfc: fdp: bound the device-supplied read size in fdp_nci_i2c_read()

[PATCH net] nfc: fdp: bound the device-supplied read size in fdp_nci_i2c_read()

Posted by Doruk Tan Ozturk 2 weeks ago

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

Re: [PATCH net] nfc: fdp: bound the device-supplied read size in fdp_nci_i2c_read()

Posted by Simon Horman 5 days, 12 hours ago

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
>

Re: [PATCH net] nfc: fdp: bound the device-supplied read size in fdp_nci_i2c_read()

Posted by David Heidelberg 5 days, 12 hours ago

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

Re: [PATCH net] nfc: fdp: bound the device-supplied read size in fdp_nci_i2c_read()

Posted by Doruk Tan Ozturk 5 days, 9 hours ago

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/