From: Linkui Xiao <xiaolinkui@126.com>
To: aleksandr.loktionov@intel.com, anthony.l.nguyen@intel.com,
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: [PATCH net v3] i40e: limit the DDP profile count returned by the firmware
Date: Tue, 22 Sep 2026 17:11:23 +0800 [thread overview]
Message-ID: <20260922091123.506598-1-xiaolinkui@126.com> (raw)
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
next reply other threads:[~2026-09-22 9:12 UTC|newest]
Thread overview: 2+ messages / expand[flat|nested] mbox.gz Atom feed top
2026-09-22 9:11 Linkui Xiao [this message]
2026-09-24 13:29 ` Simon Horman
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20260922091123.506598-1-xiaolinkui@126.com \
--to=xiaolinkui@126.com \
--cc=aleksandr.loktionov@intel.com \
--cc=andrew+netdev@lunn.ch \
--cc=anthony.l.nguyen@intel.com \
--cc=davem@davemloft.net \
--cc=edumazet@google.com \
--cc=intel-wired-lan@lists.osuosl.org \
--cc=kuba@kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=netdev@vger.kernel.org \
--cc=pabeni@redhat.com \
--cc=przemyslaw.kitszel@intel.com \
--cc=xiaolinkui@kylinos.cn \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox
all inboxes | Powered by JetHome®