[PATCH net v3] i40e: limit the DDP profile count returned by the firmware

[PATCH net v3] 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.

The same buffer is handed to the firmware as an indirect admin queue
buffer, and the admin queue code copies all of it into the DMA bounce
buffer before submitting the command, so its uninitialized contents were
visible to the device as well.

Zero initialize buff[] and reject the list when the firmware reports more
profiles than the buffer can hold, instead of answering from a list that
was only partially read. Both helpers already report errors to
i40e_ddp_load(), which aborts the operation.

Fixes: cdc594e00370 ("i40e: Implement DDP support in i40e driver")
Signed-off-by: Linkui Xiao <xiaolinkui@kylinos.cn>
---
Changes in v3:
- Zero initialize buff[] in both helpers: the whole buffer is copied into
  the admin queue DMA bounce buffer and copied back afterwards, so its
  contents were exposed to the device, and entries the firmware never
  wrote were compared against. (Sashiko AI review)
- Reject the list when the firmware reports more profiles than buff[] can
  hold, instead of silently clamping the scan and then answering from a
  list that was only partially read. (Sashiko AI review)
- Dropped the Reviewed-by tag, as the code changed after the review.

 drivers/net/ethernet/intel/i40e/i40e_ddp.c | 22 ++++++++++++++++++----
 1 file changed, 18 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..49a98c0e001a 100644
--- a/drivers/net/ethernet/intel/i40e/i40e_ddp.c
+++ b/drivers/net/ethernet/intel/i40e/i40e_ddp.c
@@ -53,9 +53,9 @@ static int i40e_ddp_does_profile_exist(struct i40e_hw *hw,
 				       struct i40e_profile_info *pinfo)
 {
 	struct i40e_ddp_profile_list *profile_list;
-	u8 buff[I40E_PROFILE_LIST_SIZE];
+	u8 buff[I40E_PROFILE_LIST_SIZE] = {};
 	int status;
-	int i;
+	u32 i;
 
 	status = i40e_aq_get_ddp_list(hw, buff, I40E_PROFILE_LIST_SIZE, 0,
 				      NULL);
@@ -63,6 +63,13 @@ static int i40e_ddp_does_profile_exist(struct i40e_hw *hw,
 		return -1;
 
 	profile_list = (struct i40e_ddp_profile_list *)buff;
+	/* The firmware is not required to report a profile count that fits
+	 * into the buffer we gave it; refuse to read such a list instead of
+	 * walking past the end of buff[].
+	 */
+	if (profile_list->p_count > I40E_MAX_PROFILE_NUM)
+		return -EIO;
+
 	for (i = 0; i < profile_list->p_count; i++) {
 		if (i40e_ddp_profiles_eq(pinfo, &profile_list->p_info[i]))
 			return 1;
@@ -108,9 +115,9 @@ static int i40e_ddp_does_profile_overlap(struct i40e_hw *hw,
 					 struct i40e_profile_info *pinfo)
 {
 	struct i40e_ddp_profile_list *profile_list;
-	u8 buff[I40E_PROFILE_LIST_SIZE];
+	u8 buff[I40E_PROFILE_LIST_SIZE] = {};
 	int status;
-	int i;
+	u32 i;
 
 	status = i40e_aq_get_ddp_list(hw, buff, I40E_PROFILE_LIST_SIZE, 0,
 				      NULL);
@@ -118,6 +125,13 @@ static int i40e_ddp_does_profile_overlap(struct i40e_hw *hw,
 		return -EIO;
 
 	profile_list = (struct i40e_ddp_profile_list *)buff;
+	/* The firmware is not required to report a profile count that fits
+	 * into the buffer we gave it; refuse to read such a list instead of
+	 * walking past the end of buff[].
+	 */
+	if (profile_list->p_count > I40E_MAX_PROFILE_NUM)
+		return -EIO;
+
 	for (i = 0; i < profile_list->p_count; i++) {
 		if (i40e_ddp_profiles_overlap(pinfo,
 					      &profile_list->p_info[i]))
-- 
2.25.1

Re: [PATCH net v3] i40e: limit the DDP profile count returned by the firmware

[Simon Horman]

On Tue, Sep 22, 2026 at 05:11:23PM +0800, Linkui Xiao wrote:
> 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.
> 
> The same buffer is handed to the firmware as an indirect admin queue
> buffer, and the admin queue code copies all of it into the DMA bounce
> buffer before submitting the command, so its uninitialized contents were
> visible to the device as well.
> 
> Zero initialize buff[] and reject the list when the firmware reports more
> profiles than the buffer can hold, instead of answering from a list that
> was only partially read. Both helpers already report errors to
> i40e_ddp_load(), which aborts the operation.
> 
> Fixes: cdc594e00370 ("i40e: Implement DDP support in i40e driver")
> Signed-off-by: Linkui Xiao <xiaolinkui@kylinos.cn>
> ---
> Changes in v3:
> - Zero initialize buff[] in both helpers: the whole buffer is copied into
>   the admin queue DMA bounce buffer and copied back afterwards, so its
>   contents were exposed to the device, and entries the firmware never
>   wrote were compared against. (Sashiko AI review)
> - Reject the list when the firmware reports more profiles than buff[] can
>   hold, instead of silently clamping the scan and then answering from a
>   list that was only partially read. (Sashiko AI review)
> - Dropped the Reviewed-by tag, as the code changed after the review.

Thanks for the updates.

Reviewed-by: Simon Horman <horms@kernel.org>