Re: [PATCH net] xfrm: hold net_device reference under RCU in bundle creation

On Thu, Aug 20, 2026 at 04:02:45PM -0400, Cen Zhang (Microsoft) wrote:
> xfrm_bundle_create() and xfrm_create_dummy_bundle() read dst->dev into
> a local pointer without taking a device reference, then pass it to
> xfrm_fill_dst().  A concurrent RTM_DELLINK replaces dst->dev via
> dst_dev_put() and frees the old net_device, causing a use-after-free
> when xfrm6_fill_dst() later dereferences the stale dev pointer.
> 
>   BUG: KASAN: slab-use-after-free in xfrm6_fill_dst+0x82c/0x860
>   (net/ipv6/xfrm6_policy.c:86 netdev_hold())
>   Read of size 8 at addr ffff8880142fe588 by task exploit/153
>   Call Trace:
>    xfrm6_fill_dst+0x82c/0x860
>    xfrm_resolve_and_create_bundle+0x21d4/0x2bd0
>    xfrm_lookup_with_ifid+0x485/0x1640
>    ip6_dst_lookup_flow+0x19b/0x1e0
>    udpv6_sendmsg+0x1443/0x2dd0
> 
> Fix by reading dst->dev under rcu_read_lock() via dst_dev_rcu() and
> taking dev_hold() before leaving the RCU critical section.  To properly
> release the reference, add dev_put() on both the success and error
> paths, and initialize dev to NULL to prevent dev_put() on an
> uninitialized pointer when put_states falls through to free_dst before
> dev is assigned.
> 
> Fixes: 25ee3286dcbc ("[IPSEC]: Merge common code into xfrm_bundle_create")
> Fixes: a0073fe18e71 ("xfrm: Add a state resolution packet queue")
> Reported-by: [email protected]
> Reported-by: Xiang Mei (Microsoft) <[email protected]>
> Reported-by: Cen Zhang (Microsoft) <[email protected]>
> Signed-off-by: Cen Zhang (Microsoft) <[email protected]>
> ---
>  net/xfrm/xfrm_policy.c | 18 +++++++++++++++---
>  1 file changed, 15 insertions(+), 3 deletions(-)
> 
> diff --git a/net/xfrm/xfrm_policy.c b/net/xfrm/xfrm_policy.c
> index 932a313b9460..13585ff24aba 100644
> --- a/net/xfrm/xfrm_policy.c
> +++ b/net/xfrm/xfrm_policy.c
> @@ -2668,7 +2668,7 @@ static struct dst_entry *xfrm_bundle_create(struct xfrm_policy *policy,
>  	const struct xfrm_mode *inner_mode;
>  	struct net *net = xp_net(policy);
>  	unsigned long now = jiffies;
> -	struct net_device *dev;
> +	struct net_device *dev = NULL;
>  	struct xfrm_dst *xdst_prev = NULL;
>  	struct xfrm_dst *xdst0 = NULL;
>  	int i = 0;
> @@ -2770,7 +2770,11 @@ static struct dst_entry *xfrm_bundle_create(struct xfrm_policy *policy,
>  	xdst0->path = dst;
>  
>  	err = -ENODEV;
> -	dev = dst->dev;
> +	rcu_read_lock();
> +	dev = dst_dev_rcu(dst);
> +	if (dev)
> +		dev_hold(dev);
> +	rcu_read_unlock();

Can't we move the rcu_read_unlock() to the end of the function and
avoid the additional refcount?

>  	if (!dev)
>  		goto free_dst;
>  
> @@ -2789,6 +2793,7 @@ static struct dst_entry *xfrm_bundle_create(struct xfrm_policy *policy,
>  		trailer_len -= xdst_prev->u.dst.xfrm->props.trailer_len;
>  	}
>  
> +	dev_put(dev);
>  	return &xdst0->u.dst;
>  
>  put_states:
> @@ -2798,6 +2803,8 @@ static struct dst_entry *xfrm_bundle_create(struct xfrm_policy *policy,
>  	if (xdst0)
>  		dst_release_immediate(&xdst0->u.dst);
>  
> +	if (dev)
> +		dev_put(dev);
>  	return ERR_PTR(err);
>  }
>  
> @@ -3058,11 +3065,16 @@ static struct xfrm_dst *xfrm_create_dummy_bundle(struct net *net,
>  	xfrm_init_path((struct xfrm_dst *)dst1, dst, 0);
>  
>  	err = -ENODEV;
> -	dev = dst->dev;
> +	rcu_read_lock();
> +	dev = dst_dev_rcu(dst);
> +	if (dev)
> +		dev_hold(dev);
> +	rcu_read_unlock();

Same here.