Re: [RFC PATCH net-next 1/7] xfrm: allow packet offload drivers to own transmit

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [Critical] Bypassing `xfrm_dev_offload_ok()` allows packets to be transmitted without applying mandatory outer IPsec transformations for nested SAs.
- [High] Unchecked dereference of `x->xso.dev` races with asynchronous device unregistration, leading to a deterministic kernel panic.
--

commit c38a10fca161e28cf54507d8d670ee75e47687df
Author: Jihong Min <hurryman2212@gmail.com>

xfrm: allow packet offload drivers to own transmit

Packet offload drivers can currently program state and validate whether an
skb can be offloaded, but they cannot take ownership of a packet that needs
driver-specific transmit preparation before the regular XFRM output path
continues. This commit adds an optional xdo_dev_packet_xmit callback for
drivers to consume the skb and return the final transmit status.

[ ... ]

> diff --git a/net/xfrm/xfrm_output.c b/net/xfrm/xfrm_output.c
> index cc35c2fcbbe09..9f11559b02210 100644
> --- a/net/xfrm/xfrm_output.c
> +++ b/net/xfrm/xfrm_output.c
> @@ -770,6 +770,17 @@ int xfrm_output(struct sock *sk, struct sk_buff *skb)
>  	}
>  
>  	if (x->xso.type == XFRM_DEV_OFFLOAD_PACKET) {
> +#ifdef CONFIG_XFRM_OFFLOAD
> +		const struct xfrmdev_ops *ops;
> +#endif
> +
> +#ifdef CONFIG_XFRM_OFFLOAD
> +		ops = x->xso.dev->xfrmdev_ops;

[Severity: High]
Can this dereference of x->xso.dev crash if the device is concurrently
unregistered?

When a device is unregistered, xfrm_dev_state_flush() calls
xfrm_dev_state_free(), which clears x->xso.dev to null before updating
x->xso.type.

If xfrm_output() sees x->xso.type == XFRM_DEV_OFFLOAD_PACKET but reads
x->xso.dev after it was cleared, won't this result in a null pointer
dereference for in-flight packets?

> +		/* Callback validates, consumes skb and returns final TX status. */
> +		if (ops && ops->xdo_dev_packet_xmit)
> +			return ops->xdo_dev_packet_xmit(skb, x);

[Severity: Critical]
Does this early return bypass outer IPsec transformations for nested
associations?

By returning early here, the code skips the call to xfrm_dev_offload_ok(),
which performs validation including checking !xdst->child->xfrm. This check
normally ensures there are no nested IPsec transformations pending.

If a packet has nested IPsec associations, such as transport over tunnel,
would the hardware driver transmit the packet out of the physical port
applying only the innermost transformation, leaving the outer transformations
completely bypassed?

> +#endif
> +
>  		if (!xfrm_dev_offload_ok(skb, x)) {
>  			XFRM_INC_STATS(net, LINUX_MIB_XFRMOUTERROR);
>  			kfree_skb(skb);

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260614040032.1567994-1-hurryman2212@gmail.com?part=1