Netdev List
 help / color / mirror / Atom feed
From: Sasha Levin <sashal@kernel.org>
To: Steffen Klassert <steffen.klassert@secunet.com>,
	Herbert Xu <herbert@gondor.apana.org.au>,
	"David S . Miller" <davem@davemloft.net>,
	David Ahern <dsahern@kernel.org>,
	Eric Dumazet <edumazet@google.com>,
	Jakub Kicinski <kuba@kernel.org>, Paolo Abeni <pabeni@redhat.com>,
	Simon Horman <horms@kernel.org>,
	netdev@vger.kernel.org, linux-kernel@vger.kernel.org,
	stable@vger.kernel.org,
	Simon Liebold <lieboldsimonpaul@gmail.com>
Cc: Qi Tang <tpluszz77@gmail.com>, Florian Westphal <fw@strlen.de>,
	Simon Liebold <simonlie@amazon.de>
Subject: Re: [PATCH 6.12.y v2] xfrm: hold dev ref until after transport_finish NF_HOOK
Date: Thu, 11 Jun 2026 11:44:08 -0400	[thread overview]
Message-ID: <airXyC2CaS0kO84h@laps> (raw)
In-Reply-To: <20260611-stable-reply-0102@kernel.org>

On Thu, Jun 11, 2026 at 11:26:20AM -0400, Sasha Levin wrote:
>On Thu, Jun 11, 2026 at 12:11:27PM +0000, Simon Liebold wrote:
>> [ Upstream commit 1c428b03840094410c5fb6a5db30640486bbbfcb ]
>>
>> After async crypto completes, xfrm_input_resume() calls dev_put()
>> immediately on re-entry before the skb reaches transport_finish.
>
>Queued for 6.12, thanks.

Ugh... Looking at it again, I've dropped it.

The problem is the assumption that "the dev_put in the encap_type == -1
async-resumption block does not exist" in 6.12.y. It's true there is no dev_put
inside the 'if (encap_type == -1)' block, but that is only because the early
drop lives somewhere else here: it's the dev_put right at the 'resume:' label.

Look at where 'resume:' sits relative to the per-iteration dev_put:

   mainline (post-fix):              6.12.y:
         dev_hold(skb->dev);               dev_hold(skb->dev);
         nexthdr = ...input(x, skb);       nexthdr = ...input(x, skb);
         if (nexthdr == -EINPROGRESS) {    if (nexthdr == -EINPROGRESS)
                 if (async)                        return 0;
                         dev_put(...);     resume:
                 return 0;                         dev_put(skb->dev);   <-- early drop
         }
         dev_put(skb->dev);
   resume:                                 [async re-entry does goto resume,
         ...                                so this dev_put runs immediately]

In mainline the fix works because 'resume:' is *after* the per-iteration
dev_put, so when xfrm_input_resume() re-enters and does 'goto resume', the
async ref taken at the loop-top dev_hold is *not* dropped - it is held
continuously until after the NF_HOOK (plus the inline 'if (async) dev_put()' it
adds at the decaps/gro/drop/secondary-EINPROGRESS exits).

In 6.12.y 'resume:' is *before* that dev_put, so the async 'goto resume' hits
'dev_put(skb->dev)' straight away and drops the ref at the very start of resume
processing. The fresh 'dev_hold(skb->dev)' added before transport_finish does
not save it:

   - between the early dev_put and the re-hold, skb->dev is held by no
     xfrm reference at all - the exact window device teardown can race; and
   - 'dev_hold(skb->dev)' itself dereferences skb->dev to bump the
     refcount, so if the device was already freed in that window the
     re-hold is itself a use-after-free.

So this is a lifetime bug, not a refcount-balance bug: every hold still has a
matching put, but the reference no longer covers the critical window.

-- 
Thanks,
Sasha

      reply	other threads:[~2026-06-11 15:44 UTC|newest]

Thread overview: 3+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-06-11 12:11 [PATCH 6.12.y v2] xfrm: hold dev ref until after transport_finish NF_HOOK Simon Liebold
2026-06-11 15:26 ` Sasha Levin
2026-06-11 15:44   ` Sasha Levin [this message]

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=airXyC2CaS0kO84h@laps \
    --to=sashal@kernel.org \
    --cc=davem@davemloft.net \
    --cc=dsahern@kernel.org \
    --cc=edumazet@google.com \
    --cc=fw@strlen.de \
    --cc=herbert@gondor.apana.org.au \
    --cc=horms@kernel.org \
    --cc=kuba@kernel.org \
    --cc=lieboldsimonpaul@gmail.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=netdev@vger.kernel.org \
    --cc=pabeni@redhat.com \
    --cc=simonlie@amazon.de \
    --cc=stable@vger.kernel.org \
    --cc=steffen.klassert@secunet.com \
    --cc=tpluszz77@gmail.com \
    /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