[Intel-wired-lan] [PATCH net] i40e: limit the DDP profile count returned by the firmware

[Intel-wired-lan] [PATCH net] i40e: limit the DDP profile count returned by the firmware

[Linkui Xiao]

From: Linkui Xiao <xiaolinkui@kylinos.cn>

i40e_aq_get_ddp_list() writes into a I40E_PROFILE_LIST_SIZE buffer,
which is sized for I40E_MAX_PROFILE_NUM (16) i40e_profile_info entries
plus the 4 byte p_count header. i40e_ddp_does_profile_exist() and
i40e_ddp_does_profile_overlap() then loop over profile_list->p_count
without bounding it, so a firmware reporting more than 16 profiles makes
both helpers walk past the end of the on-stack buff[] and compare
against whatever happens to follow it on the stack.

Clamp the count to the number of entries the buffer can actually hold
and make the loop counter unsigned to match the field type.

Fixes: cdc594e00370 ("i40e: Implement DDP support in i40e driver")
Signed-off-by: Linkui Xiao <xiaolinkui@kylinos.cn>
---
 drivers/net/ethernet/intel/i40e/i40e_ddp.c | 18 ++++++++++++++----
 1 file changed, 14 insertions(+), 4 deletions(-)

diff --git a/drivers/net/ethernet/intel/i40e/i40e_ddp.c b/drivers/net/ethernet/intel/i40e/i40e_ddp.c
index daa9f2c42f70..1d6d8b3835a3 100644
--- a/drivers/net/ethernet/intel/i40e/i40e_ddp.c
+++ b/drivers/net/ethernet/intel/i40e/i40e_ddp.c
@@ -55,7 +55,7 @@ static int i40e_ddp_does_profile_exist(struct i40e_hw *hw,
 	struct i40e_ddp_profile_list *profile_list;
 	u8 buff[I40E_PROFILE_LIST_SIZE];
 	int status;
-	int i;
+	u32 i, p_count;
 
 	status = i40e_aq_get_ddp_list(hw, buff, I40E_PROFILE_LIST_SIZE, 0,
 				      NULL);
@@ -63,7 +63,12 @@ static int i40e_ddp_does_profile_exist(struct i40e_hw *hw,
 		return -1;
 
 	profile_list = (struct i40e_ddp_profile_list *)buff;
-	for (i = 0; i < profile_list->p_count; i++) {
+	/* Never walk past the end of buff[], the profile count reported by
+	 * the firmware is not guaranteed to fit into the buffer we gave it.
+	 */
+	p_count = min_t(u32, profile_list->p_count, I40E_MAX_PROFILE_NUM);
+
+	for (i = 0; i < p_count; i++) {
 		if (i40e_ddp_profiles_eq(pinfo, &profile_list->p_info[i]))
 			return 1;
 	}
@@ -110,7 +115,7 @@ static int i40e_ddp_does_profile_overlap(struct i40e_hw *hw,
 	struct i40e_ddp_profile_list *profile_list;
 	u8 buff[I40E_PROFILE_LIST_SIZE];
 	int status;
-	int i;
+	u32 i, p_count;
 
 	status = i40e_aq_get_ddp_list(hw, buff, I40E_PROFILE_LIST_SIZE, 0,
 				      NULL);
@@ -118,7 +123,12 @@ static int i40e_ddp_does_profile_overlap(struct i40e_hw *hw,
 		return -EIO;
 
 	profile_list = (struct i40e_ddp_profile_list *)buff;
-	for (i = 0; i < profile_list->p_count; i++) {
+	/* Never walk past the end of buff[], the profile count reported by
+	 * the firmware is not guaranteed to fit into the buffer we gave it.
+	 */
+	p_count = min_t(u32, profile_list->p_count, I40E_MAX_PROFILE_NUM);
+
+	for (i = 0; i < p_count; i++) {
 		if (i40e_ddp_profiles_overlap(pinfo,
 					      &profile_list->p_info[i]))
 			return 1;
-- 
2.25.1

RE: [Intel-wired-lan] [PATCH net] i40e: limit the DDP profile count returned by the firmware

[Loktionov, Aleksandr]

> -----Original Message-----
> From: Linkui Xiao <xiaolinkui@126.com>
> Sent: Tuesday, September 15, 2026 2:04 PM
> To: Nguyen, Anthony L <anthony.l.nguyen@intel.com>; Kitszel,
> Przemyslaw <przemyslaw.kitszel@intel.com>; andrew+netdev@lunn.ch;
> davem@davemloft.net; edumazet@google.com; kuba@kernel.org;
> pabeni@redhat.com
> Cc: intel-wired-lan@lists.osuosl.org; netdev@vger.kernel.org; linux-
> kernel@vger.kernel.org; Linkui Xiao <xiaolinkui@kylinos.cn>
> Subject: [Intel-wired-lan] [PATCH net] i40e: limit the DDP profile
> count returned by the firmware
> 
> From: Linkui Xiao <xiaolinkui@kylinos.cn>
> 
> i40e_aq_get_ddp_list() writes into a I40E_PROFILE_LIST_SIZE buffer,
> which is sized for I40E_MAX_PROFILE_NUM (16) i40e_profile_info entries
> plus the 4 byte p_count header. i40e_ddp_does_profile_exist() and
> i40e_ddp_does_profile_overlap() then loop over profile_list->p_count
> without bounding it, so a firmware reporting more than 16 profiles
> makes both helpers walk past the end of the on-stack buff[] and
> compare against whatever happens to follow it on the stack.
> 
> Clamp the count to the number of entries the buffer can actually hold
> and make the loop counter unsigned to match the field type.
> 
> Fixes: cdc594e00370 ("i40e: Implement DDP support in i40e driver")
> Signed-off-by: Linkui Xiao <xiaolinkui@kylinos.cn>
> ---
>  drivers/net/ethernet/intel/i40e/i40e_ddp.c | 18 ++++++++++++++----
>  1 file changed, 14 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/net/ethernet/intel/i40e/i40e_ddp.c
> b/drivers/net/ethernet/intel/i40e/i40e_ddp.c
> index daa9f2c42f70..1d6d8b3835a3 100644
> --- a/drivers/net/ethernet/intel/i40e/i40e_ddp.c
> +++ b/drivers/net/ethernet/intel/i40e/i40e_ddp.c
> @@ -55,7 +55,7 @@ static int i40e_ddp_does_profile_exist(struct
> i40e_hw *hw,
>  	struct i40e_ddp_profile_list *profile_list;
>  	u8 buff[I40E_PROFILE_LIST_SIZE];
>  	int status;
> -	int i;
> +	u32 i, p_count;
RCT please

> 
>  	status = i40e_aq_get_ddp_list(hw, buff, I40E_PROFILE_LIST_SIZE,
> 0,
>  				      NULL);
> @@ -63,7 +63,12 @@ static int i40e_ddp_does_profile_exist(struct
> i40e_hw *hw,
>  		return -1;
> 
>  	profile_list = (struct i40e_ddp_profile_list *)buff;
> -	for (i = 0; i < profile_list->p_count; i++) {
> +	/* Never walk past the end of buff[], the profile count
> reported by
> +	 * the firmware is not guaranteed to fit into the buffer we
> gave it.
> +	 */
> +	p_count = min_t(u32, profile_list->p_count,
> I40E_MAX_PROFILE_NUM);
> +
> +	for (i = 0; i < p_count; i++) {
>  		if (i40e_ddp_profiles_eq(pinfo, &profile_list-
> >p_info[i]))
>  			return 1;
>  	}
> @@ -110,7 +115,7 @@ static int i40e_ddp_does_profile_overlap(struct
> i40e_hw *hw,
>  	struct i40e_ddp_profile_list *profile_list;
>  	u8 buff[I40E_PROFILE_LIST_SIZE];
>  	int status;
> -	int i;
> +	u32 i, p_count;
RCT please

Reviewed-by: Aleksandr Loktionov <aleksandr.loktionov@intel.com>

> 
>  	status = i40e_aq_get_ddp_list(hw, buff, I40E_PROFILE_LIST_SIZE,
> 0,
>  				      NULL);
> @@ -118,7 +123,12 @@ static int i40e_ddp_does_profile_overlap(struct
> i40e_hw *hw,
>  		return -EIO;
> 
>  	profile_list = (struct i40e_ddp_profile_list *)buff;
> -	for (i = 0; i < profile_list->p_count; i++) {
> +	/* Never walk past the end of buff[], the profile count
> reported by
> +	 * the firmware is not guaranteed to fit into the buffer we
> gave it.
> +	 */
> +	p_count = min_t(u32, profile_list->p_count,
> I40E_MAX_PROFILE_NUM);
> +
> +	for (i = 0; i < p_count; i++) {
>  		if (i40e_ddp_profiles_overlap(pinfo,
>  					      &profile_list->p_info[i]))
>  			return 1;
> --
> 2.25.1

Re: [Intel-wired-lan] [PATCH net] i40e: limit the DDP profile count returned by the firmware

[Linkui Xiao]

On 2026/9/16 14:03, Loktionov, Aleksandr wrote:
> 
> 
>> -----Original Message-----
>> From: Linkui Xiao <xiaolinkui@126.com>
>> Sent: Tuesday, September 15, 2026 2:04 PM
>> To: Nguyen, Anthony L <anthony.l.nguyen@intel.com>; Kitszel,
>> Przemyslaw <przemyslaw.kitszel@intel.com>; andrew+netdev@lunn.ch;
>> davem@davemloft.net; edumazet@google.com; kuba@kernel.org;
>> pabeni@redhat.com
>> Cc: intel-wired-lan@lists.osuosl.org; netdev@vger.kernel.org; linux-
>> kernel@vger.kernel.org; Linkui Xiao <xiaolinkui@kylinos.cn>
>> Subject: [Intel-wired-lan] [PATCH net] i40e: limit the DDP profile
>> count returned by the firmware
>>
>> From: Linkui Xiao <xiaolinkui@kylinos.cn>
>>
>> i40e_aq_get_ddp_list() writes into a I40E_PROFILE_LIST_SIZE buffer,
>> which is sized for I40E_MAX_PROFILE_NUM (16) i40e_profile_info entries
>> plus the 4 byte p_count header. i40e_ddp_does_profile_exist() and
>> i40e_ddp_does_profile_overlap() then loop over profile_list->p_count
>> without bounding it, so a firmware reporting more than 16 profiles
>> makes both helpers walk past the end of the on-stack buff[] and
>> compare against whatever happens to follow it on the stack.
>>
>> Clamp the count to the number of entries the buffer can actually hold
>> and make the loop counter unsigned to match the field type.
>>
>> Fixes: cdc594e00370 ("i40e: Implement DDP support in i40e driver")
>> Signed-off-by: Linkui Xiao <xiaolinkui@kylinos.cn>
>> ---
>>   drivers/net/ethernet/intel/i40e/i40e_ddp.c | 18 ++++++++++++++----
>>   1 file changed, 14 insertions(+), 4 deletions(-)
>>
>> diff --git a/drivers/net/ethernet/intel/i40e/i40e_ddp.c
>> b/drivers/net/ethernet/intel/i40e/i40e_ddp.c
>> index daa9f2c42f70..1d6d8b3835a3 100644
>> --- a/drivers/net/ethernet/intel/i40e/i40e_ddp.c
>> +++ b/drivers/net/ethernet/intel/i40e/i40e_ddp.c
>> @@ -55,7 +55,7 @@ static int i40e_ddp_does_profile_exist(struct
>> i40e_hw *hw,
>>   	struct i40e_ddp_profile_list *profile_list;
>>   	u8 buff[I40E_PROFILE_LIST_SIZE];
>>   	int status;
>> -	int i;
>> +	u32 i, p_count;
> RCT please
> 
>>
>>   	status = i40e_aq_get_ddp_list(hw, buff, I40E_PROFILE_LIST_SIZE,
>> 0,
>>   				      NULL);
>> @@ -63,7 +63,12 @@ static int i40e_ddp_does_profile_exist(struct
>> i40e_hw *hw,
>>   		return -1;
>>
>>   	profile_list = (struct i40e_ddp_profile_list *)buff;
>> -	for (i = 0; i < profile_list->p_count; i++) {
>> +	/* Never walk past the end of buff[], the profile count
>> reported by
>> +	 * the firmware is not guaranteed to fit into the buffer we
>> gave it.
>> +	 */
>> +	p_count = min_t(u32, profile_list->p_count,
>> I40E_MAX_PROFILE_NUM);
>> +
>> +	for (i = 0; i < p_count; i++) {
>>   		if (i40e_ddp_profiles_eq(pinfo, &profile_list-
>>> p_info[i]))
>>   			return 1;
>>   	}
>> @@ -110,7 +115,7 @@ static int i40e_ddp_does_profile_overlap(struct
>> i40e_hw *hw,
>>   	struct i40e_ddp_profile_list *profile_list;
>>   	u8 buff[I40E_PROFILE_LIST_SIZE];
>>   	int status;
>> -	int i;
>> +	u32 i, p_count;
> RCT please
> 
> Reviewed-by: Aleksandr Loktionov <aleksandr.loktionov@intel.com>
Hi,

Thanks for the review and for the Reviewed-by tag.

Just to make sure I understand the "RCT please" comments correctly: are 
they referring to the Reverse Christmas Tree (RCT) variable declaration 
ordering convention, i.e. the local variables should be declared in 
decreasing order of line length? In both functions I currently have:
         struct i40e_ddp_profile_list *profile_list;
         u8 buff[I40E_PROFILE_LIST_SIZE];
         int status;
         u32 i, p_count;

If I understand RCT correctly, the expected order would be something like:
         struct i40e_ddp_profile_list *profile_list;
         u8 buff[I40E_PROFILE_LIST_SIZE];
         u32 i, p_count;
         int status;

Could you please confirm whether that is what is being asked for, and 
whether there are any other RCT-related issues in this patch?
Also, should I send a v2 with the RCT ordering fixed, or would you 
prefer to keep the current version as-is since the change is purely 
stylistic? I am happy to send a v2 if that is preferred.

Thanks,
Linkui
> 
>>
>>   	status = i40e_aq_get_ddp_list(hw, buff, I40E_PROFILE_LIST_SIZE,
>> 0,
>>   				      NULL);
>> @@ -118,7 +123,12 @@ static int i40e_ddp_does_profile_overlap(struct
>> i40e_hw *hw,
>>   		return -EIO;
>>
>>   	profile_list = (struct i40e_ddp_profile_list *)buff;
>> -	for (i = 0; i < profile_list->p_count; i++) {
>> +	/* Never walk past the end of buff[], the profile count
>> reported by
>> +	 * the firmware is not guaranteed to fit into the buffer we
>> gave it.
>> +	 */
>> +	p_count = min_t(u32, profile_list->p_count,
>> I40E_MAX_PROFILE_NUM);
>> +
>> +	for (i = 0; i < p_count; i++) {
>>   		if (i40e_ddp_profiles_overlap(pinfo,
>>   					      &profile_list->p_info[i]))
>>   			return 1;
>> --
>> 2.25.1
>