archive mirror
 help / color / mirror / Atom feed
From: Linus Torvalds <>
To: Hillf Danton <>
Cc: "Eric W. Biederman" <>,
	Linux Kernel Mailing List <>,
	Alexey Gladkov <>
Subject: Re: [GIT PULL] ucount fix for v5.14-rc
Date: Sat, 7 Aug 2021 19:05:21 -0700	[thread overview]
Message-ID: <> (raw)
In-Reply-To: <>

On Sat, Aug 7, 2021 at 6:45 PM Linus Torvalds
<> wrote:
> Because that fix matches the syzbot reports I saw. It explains them
> 100%, and the fix is clearly the right thing.

Just to clarify: the exact symptoms of the race before that fix can
vary. The simplest form was the one I described:

> Thread (a) on CPU1: starting out _without_ a reference to the
> uncounts, look up entry under the lock, but don't increment the count,
> release lock.
> Thread (b) on CPU2: have a reference, do a put_ucounts(). Count goes
> to zero, take the lock, unhash it, free the entry
> Thread (a) continues, increments the count on a UAF entry, triggers KASAN.

because that one happens immediately. But a more likely one is
actually that the "Thread (a) continues" thing happens _before_ the
entry is free'd (but after thread (b) has decremented the count to
zero), and then what happens is that thread (a) doesn't actually
trigger UAF and a KASAN report at that point yet.

Instead, thread (a) will install the pointer to the - free'd - ucounts
somewhere, and the UAF will trigger only rather much later.

Which makes the KASAN reports much harder to read, of course, because
by the time you actually access the invalid uncounts pointer, the
original *cause* of the problem is gone.

So I described the "easy" case to see, not the only one that that the
bug in b6c336528926 ("Use atomic_t for ucounts reference counting")
would trigger.

But commit 345daff2e994 ("ucounts: Fix race condition between
alloc_ucounts and put_ucounts") really is the obvious fix for an
obvious bug.

Could there be _another_ bug? Sure. Fixing one bug - no matter how
obvious the fix - doesn't guarantee there isn't something else lurking
too. But if so, I haven't seen the syzbot reports for it.


      reply	other threads:[~2021-08-08  2:05 UTC|newest]

Thread overview: 9+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-08-05 17:15 [GIT PULL] ucount fix for v5.14-rc Eric W. Biederman
2021-08-05 19:26 ` pr-tracker-bot
     [not found] ` <>
2021-08-06  3:38   ` Eric W. Biederman
     [not found]     ` <>
2021-08-06 17:37       ` Linus Torvalds
     [not found]         ` <>
2021-08-07  8:23           ` Linus Torvalds
     [not found]             ` <>
2021-08-07 15:10               ` Linus Torvalds
     [not found]                 ` <>
2021-08-08  1:00                   ` Linus Torvalds
     [not found]                     ` <>
2021-08-08  1:45                       ` Linus Torvalds
2021-08-08  2:05                         ` Linus Torvalds [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:

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to='' \ \ \ \ \ \

* 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;
as well as URLs for NNTP newsgroup(s).