Re: [PATCH net 1/2] vti: fix tunnel device use-after-free across async crypto resumption

Re: [PATCH net 1/2] vti: fix tunnel device use-after-free across async crypto resumption

[Qihang]

All real, and vti6 has the same problems.

The dev_hold()/dev_put() idea assumed the callback runs exactly once
per skb, which xfrm_input() doesn't guarantee. IPTFS frees the outer
skb on its own without any callback. The early drop when the SA is no
longer valid happens while family is still AF_UNSPEC, so xfrm_rcv_cb()
can't find the afinfo and never calls us. And on the success path I'd
drop the last reference before gro_cells_receive() is done with
skb->dev.

So I'll drop this. Two options I can see for the respin:

Re-lookup the tunnel in the callback like xfrmi does. The only key I
have there is the outer addresses of the state, which won't reproduce
the original lookup for wildcard-source SAs, and during teardown it
can pick up the fallback device instead. The RCU section also ends at
gro_cells_receive(): the skb queued to the gro cell is processed by
NAPI afterwards with skb->dev still pointing at the tunnel device and
nothing holding a reference.

Keep a reference from vti_input() but release it when the skb is freed
instead of in the callback, so the paths where the callback never runs
can't leak. That looks like it needs xfrm core support, so I'd rather
not pick it unilaterally.

Which way do you want this to go?

pw-bot: cr

Re: [PATCH net 1/2] vti: fix tunnel device use-after-free across async crypto resumption

[Steffen Klassert]

On Thu, Oct 08, 2026 at 01:49:16AM +0000, Qihang wrote:
> All real, and vti6 has the same problems.
> 
> The dev_hold()/dev_put() idea assumed the callback runs exactly once
> per skb, which xfrm_input() doesn't guarantee. IPTFS frees the outer
> skb on its own without any callback.

This is a IPTFS bug. If IPTFS frees the skb, it has to make sure
to run the whole destruction path including all the callbacks.

> The early drop when the SA is no
> longer valid happens while family is still AF_UNSPEC, so xfrm_rcv_cb()
> can't find the afinfo and never calls us.

Assigning family before the early drop should fix this.

> And on the success path I'd
> drop the last reference before gro_cells_receive() is done with
> skb->dev.

I think we should unconditionally refcount skb->dev in the
xfrm input path to fix that for now.

Re: [PATCH net 1/2] vti: fix tunnel device use-after-free across async crypto resumption

[Qihang]

Hi Steffen,

Thanks a lot for the guidance, much appreciated.

Your three points make sense. For IPTFS, the skbs freed without the
receive callback are the outer xfrm skbs, and it's only a handful of
places in xfrm_iptfs.c -- a small shared helper would be cleaner than
open-coding it at each site.  The two xfrm_input changes (set family
before the early drop, and hold the skb->dev reference unconditionally)
look straightforward too.

Would you prefer to handle the IPTFS and xfrm_input fixes yourself
(Christian for IPTFS?), or should I send them as part of this series?
Either way is fine with me.

Thanks again,
Qihang

On Mon, Oct 12, 2026 at 3:54 PM Steffen Klassert
<steffen.klassert@secunet.com> wrote:
>
> On Thu, Oct 08, 2026 at 01:49:16AM +0000, Qihang wrote:
> > All real, and vti6 has the same problems.
> >
> > The dev_hold()/dev_put() idea assumed the callback runs exactly once
> > per skb, which xfrm_input() doesn't guarantee. IPTFS frees the outer
> > skb on its own without any callback.
>
> This is a IPTFS bug. If IPTFS frees the skb, it has to make sure
> to run the whole destruction path including all the callbacks.
>
> > The early drop when the SA is no
> > longer valid happens while family is still AF_UNSPEC, so xfrm_rcv_cb()
> > can't find the afinfo and never calls us.
>
> Assigning family before the early drop should fix this.
>
> > And on the success path I'd
> > drop the last reference before gro_cells_receive() is done with
> > skb->dev.
>
> I think we should unconditionally refcount skb->dev in the
> xfrm input path to fix that for now.
>

Re: [PATCH net 1/2] vti: fix tunnel device use-after-free across async crypto resumption

[Steffen Klassert]

On Mon, Oct 12, 2026 at 05:07:43PM +0800, Qihang wrote:
> Hi Steffen,
> 
> Thanks a lot for the guidance, much appreciated.
> 
> Your three points make sense. For IPTFS, the skbs freed without the
> receive callback are the outer xfrm skbs, and it's only a handful of
> places in xfrm_iptfs.c -- a small shared helper would be cleaner than
> open-coding it at each site.  The two xfrm_input changes (set family
> before the early drop, and hold the skb->dev reference unconditionally)
> look straightforward too.
> 
> Would you prefer to handle the IPTFS and xfrm_input fixes yourself
> (Christian for IPTFS?), or should I send them as part of this series?
> Either way is fine with me.

I'll handle the full patchset, but Cc Chris on IPTFS.

Re: [PATCH net 1/2] vti: fix tunnel device use-after-free across async crypto resumption

[Qihang]

Hi Steffen,

Sounds good -- thanks for taking the patchset.  Ccing Chris on the IPTFS
part makes sense.  I'll follow the thread, and I'm happy to review it
if that helps.

Qihang

On Mon, Oct 12, 2026 at 6:13 PM Steffen Klassert
<steffen.klassert@secunet.com> wrote:
>
> On Mon, Oct 12, 2026 at 05:07:43PM +0800, Qihang wrote:
> > Hi Steffen,
> >
> > Thanks a lot for the guidance, much appreciated.
> >
> > Your three points make sense. For IPTFS, the skbs freed without the
> > receive callback are the outer xfrm skbs, and it's only a handful of
> > places in xfrm_iptfs.c -- a small shared helper would be cleaner than
> > open-coding it at each site.  The two xfrm_input changes (set family
> > before the early drop, and hold the skb->dev reference unconditionally)
> > look straightforward too.
> >
> > Would you prefer to handle the IPTFS and xfrm_input fixes yourself
> > (Christian for IPTFS?), or should I send them as part of this series?
> > Either way is fine with me.
>
> I'll handle the full patchset, but Cc Chris on IPTFS.

Re: [PATCH net 1/2] vti: fix tunnel device use-after-free across async crypto resumption

[Steffen Klassert]

On Mon, Oct 12, 2026 at 07:21:57PM +0800, Qihang wrote:
> Hi Steffen,
> 
> Sounds good -- thanks for taking the patchset.  Ccing Chris on the IPTFS
> part makes sense.  I'll follow the thread, and I'm happy to review it
> if that helps.

I'll handle the patchset once you've sent an updated version.
I'm reviewing and applyig the patches, but you have to submit it.