[PATCH] net: ixgbe: drop the SA reference if secpath_set() fails

[PATCH] net: ixgbe: drop the SA reference if secpath_set() fails

[Wentao Liang]

ixgbe_ipsec_rx() takes a reference on the xfrm_state returned by
ixgbe_ipsec_find_rx_state(). On success that reference is handed over
to the skb's security path, which releases it when the skb is freed.
When secpath_set() cannot allocate the extension it returns NULL, but
the function returns without releasing the xfrm_state, so the SA
reference is leaked.

Put the reference before bailing out.

Fixes: 92103199f10a ("ixgbe: process the Rx ipsec offload")
Cc: stable@vger.kernel.org
Assisted-by: DeepSeek-V4.1-Flash and OpenAI Codex
Signed-off-by: Wentao Liang <vulab@iscas.ac.cn>
---
 drivers/net/ethernet/intel/ixgbe/ixgbe_ipsec.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/drivers/net/ethernet/intel/ixgbe/ixgbe_ipsec.c b/drivers/net/ethernet/intel/ixgbe/ixgbe_ipsec.c
index bd397b3d7dea..e44d5eed9ab5 100644
--- a/drivers/net/ethernet/intel/ixgbe/ixgbe_ipsec.c
+++ b/drivers/net/ethernet/intel/ixgbe/ixgbe_ipsec.c
@@ -1197,8 +1197,10 @@ void ixgbe_ipsec_rx(struct ixgbe_ring *rx_ring,
 		return;
 
 	sp = secpath_set(skb);
-	if (unlikely(!sp))
+	if (unlikely(!sp)) {
+		xfrm_state_put(xs);
 		return;
+	}
 
 	sp->xvec[sp->len++] = xs;
 	sp->olen++;
-- 
2.34.1

Re: [PATCH] net: ixgbe: drop the SA reference if secpath_set() fails

[netdev-bot+sinfo]

Hi!

This is an automated message. This series looks like a fix, but its
commit messages seem to be missing some information:

 - How the issue was discovered, e.g. hit in production, hit during
   development, syzbot report, manual code inspection, LLM or static
   analysis tool scan.

 - Whether the issue was actually triggered, or is only theoretical
   (e.g. found by code inspection). If it was triggered please include
   the symptoms, like the stack trace or error messages.

 - What hardware the change was tested on. For driver fixes please
   mention the device (and if relevant firmware version) used for
   testing, or say that the change was not tested on real hardware.

Please do not repost the series just to address the above. Instead,
reply to this email with the missing information, so that reviewers
can take it into account. If the series needs another revision for
other reasons, please include the information in the commit messages
then.

The evaluation is done by an LLM so it may be wrong, if you think
that is the case please reply and explain.

Re: [PATCH] net: ixgbe: drop the SA reference if secpath_set() fails

[Krzysztof Kozlowski]

On Tue, 03 Nov 2026 10:05:34 +0000, Wentao Liang wrote:
> ixgbe_ipsec_rx() takes a reference on the xfrm_state returned by
> ixgbe_ipsec_find_rx_state(). On success that reference is handed over
> to the skb's security path, which releases it when the skb is freed.
> When secpath_set() cannot allocate the extension it returns NULL, but
> the function returns without releasing the xfrm_state, so the SA
> reference is leaked.
> 
> Put the reference before bailing out.
> 
> Fixes: 92103199f10a ("ixgbe: process the Rx ipsec offload")
> Cc: stable@vger.kernel.org
> Assisted-by: DeepSeek-V4.1-Flash and OpenAI Codex
> Signed-off-by: Wentao Liang <vulab@iscas.ac.cn>
> ---
>  drivers/net/ethernet/intel/ixgbe/ixgbe_ipsec.c | 4 +++-
>  1 file changed, 3 insertions(+), 1 deletion(-)
> 



Multiple things here:
1. Your team ignored completely previous feedback. Multiple times.

2. You use multiple identities with this email address, thus I doubt that we
   speak with an actual person.

3. Your bot does not even bother running checkpatch, right?

4. Nothing of this was reproduced nor tested.

5. Finally, the same feedback:
You sent multiple independent patches, to multiple independent
subsystems. The amount of these patches clearly suggest this was
AI generated and most likely not tested.

More importantly, you sent all this work without properly organizing
relevant patches into patchsets. This makes reviewing difficult
and might cause multiple reviewers to address the same issue.
Replying to the entire set is impossible and requires handling each
patch independently, instead of applying or discarding the set.
Maintainers also won't see the bigger picture of your work. Quite
worrying.

This is on the verge of hostile patch: bomb us with so many
contributions, we won't be able to handle them in efficient manner,
like responding ONCE to ask you to slow down.  Considering all this
is untested and LLM generated, I have even more doubts whether this
should be considered for review.

Please read kernel documentation BEFORE posting more work. It will
explain you how to identify subsystems, how to organize your work per
subsystem (so a patchset grouping multiple patches with a short cover
letter), how to document the issue discovery method and what was
or was not tested.

Best regards,
Krzysztof

Re: [PATCH] net: ixgbe: drop the SA reference if secpath_set() fails

[netdev-bot+sashiko]

Thank you for your contribution! Sashiko AI review found 1 potential
issue(s) to consider.

Critical: 0 · High: 0 (2) · Medium: 1 · Low: 0

If you believe none of the issues are legitimate, please reply to each
issue of Medium severity or higher and explain why the AI is wrong.
If any of them are legitimate and you plan to respin, please let patchwork
know by including "pw-bot: cr" as a separate line at the end of your reply
(one such reply per series is enough).

- [Medium] This patch fixes the SA reference leak in only one of the two
  copies of this Rx code.

Pre-existing issues:
- [High] ixgbe_ipsec_del_sa() unlinks the rx_sa from the RCU hash table
  with hash_del_rcu(&rsa->hlist) (line 783) and then zeroes the same…
- [High] ixgbe_ipsec_find_rx_state() finds rsa->xs only under
  rcu_read_lock() and takes its reference with xfrm_state_hold(), which…

--

net: ixgbe: drop the SA reference if secpath_set() fails

ixgbe_ipsec_find_rx_state() takes a reference on an xfrm_state. With this
patch, ixgbe_ipsec_rx() releases that reference when secpath_set() fails
to allocate the skb extension. Before this patch the reference was leaked
on that error path.

> diff --git a/drivers/net/ethernet/intel/ixgbe/ixgbe_ipsec.c b/drivers/net/ethernet/intel/ixgbe/ixgbe_ipsec.c
> index bd397b3d7deab..e44d5eed9ab59 100644
> --- a/drivers/net/ethernet/intel/ixgbe/ixgbe_ipsec.c
> +++ b/drivers/net/ethernet/intel/ixgbe/ixgbe_ipsec.c
> @@ -1197,8 +1197,10 @@ void ixgbe_ipsec_rx(struct ixgbe_ring *rx_ring,
>  		return;
>  
>  	sp = secpath_set(skb);
> -	if (unlikely(!sp))
> +	if (unlikely(!sp)) {
> +		xfrm_state_put(xs);
>  		return;
> +	}

[Severity: Medium]
Does ixgbevf need the same fix? The VF driver has a copy of this Rx code
in drivers/net/ethernet/intel/ixgbevf/ipsec.c. After this patch,
ixgbevf_ipsec_rx() still returns without dropping the reference:

drivers/net/ethernet/intel/ixgbevf/ipsec.c:ixgbevf_ipsec_rx() {
    ...
	sp = secpath_set(skb);
	if (unlikely(!sp))
		return;
    ...
}

ixgbevf_ipsec_find_rx_state() takes the reference the same way the PF
helper does:

			ret = rsa->xs;
			xfrm_state_hold(ret);
			break;

secpath_set()->skb_ext_add() is a GFP_ATOMIC allocation. Suppose a VF gets
inline-offloaded ESP/AH traffic while memory is tight. Would one
xfrm_state reference leak each time the allocation fails? If so, the state
would never be freed after the SA is deleted.

Could ixgbevf get a matching fix with its own Fixes: tag?

>  
>  	sp->xvec[sp->len++] = xs;
>  	sp->olen++;

[Severity: High]
This isn't a bug introduced by this patch, but is the rx_sa entry walked
by ixgbe_ipsec_find_rx_state() safe against ixgbe_ipsec_del_sa()?

ixgbe_ipsec_del_sa() unlinks the entry and zeroes it straight away. There
is no grace period in between:

drivers/net/ethernet/intel/ixgbe/ixgbe_ipsec.c:ixgbe_ipsec_del_sa() {
    ...
		hash_del_rcu(&rsa->hlist);
    ...
		memset(rsa, 0, sizeof(struct rx_sa));
		ipsec->num_rx_sa--;
    ...
}

The NAPI reader runs under rcu_read_lock() and loads rsa->mode and
rsa->xs with plain loads:

ixgbe_ipsec_find_rx_state() {
    ...
		if (rsa->mode & IXGBE_RXTXMOD_VF)
			continue;
		if (spi == rsa->xs->id.spi &&
		    ((ip4 && *daddr == rsa->xs->id.daddr.a4) ||
    ...
}

Suppose a reader picked up rsa from the bucket before hash_del_rcu(). Can
it then see rsa->mode == 0, pass the VF check, and dereference
rsa->xs == NULL in softirq? If ixgbe_ipsec_add_sa() reuses the slot, the
reader could also see a mix of old and new SA fields.

The trigger would be an SA deletion (XFRM_MSG_DELSA, IKE rekey or
lifetime expiry) racing with Rx traffic whose SPI hashes to the same
bucket.

ixgbevf_ipsec_del_sa() has the same pattern:

		hash_del_rcu(&ipsec->rx_tbl[sa_idx].hlist);
		memset(&ipsec->rx_tbl[sa_idx], 0, sizeof(struct rx_sa));

[Severity: High]
This is a pre-existing issue, but is xfrm_state_hold() safe on an
xfrm_state that was found only under rcu_read_lock()?

ixgbe_ipsec_find_rx_state() does this:

			ret = rsa->xs;
			xfrm_state_hold(ret);

xfrm_state_hold() is an unconditional refcount_inc(). The rx_sa entry
holds no reference of its own on xs. __xfrm_state_delete() calls
xfrm_dev_state_delete() and then drops the reference right away:

net/xfrm/xfrm_state.c:__xfrm_state_delete() {
    ...
		xfrm_dev_state_delete(x);

		xfrm_state_delete_tunnel(x);
		...
		xfrm_state_put(x);
    ...
}

Can the refcount reach zero while a NAPI reader that has already loaded
rsa->xs is still in its RCU section? If so, refcount_inc() would run on 0.
That warns and saturates, but the pointer still ends up in sp->xvec[]
above.

xfrm_state_gc_task() frees the state after synchronize_rcu(). That waits
for RCU readers, but not for skbs that hold the secpath.

Suppose the skb outlives the grace period, for example when RPS queues it
to another CPU's backlog. Would xfrm_input() and
secpath_put()->xfrm_state_put() then touch freed memory?

The xfrm core handles this case with xfrm_state_hold_rcu(), which uses
refcount_inc_not_zero(). Should the driver take its reference with an
inc-not-zero as well? ixgbevf_ipsec_find_rx_state() has the same pattern.

-- 
Sashiko AI review · https://netdev-ai.bots.linux.dev/sashiko/#/patchset/20261103100534.1310039-1-vulab%40iscas.ac.cn