All of lore.kernel.org
 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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.