linux-kernel - Re: [BUG] Use of probe_kernel_address() in task_rcu_dereference() without checking return value

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <CAHk-=whtj+aSYftniMRG4xvFE8dmmYyrqcJyPmzStsfj5w9r=w@mail.gmail.com>
Date:   Fri, 30 Aug 2019 09:58:15 -0700
From:   Linus Torvalds <torvalds@...ux-foundation.org>
To:     Oleg Nesterov <oleg@...hat.com>
Cc:     Russell King - ARM Linux admin <linux@...linux.org.uk>,
        Peter Zijlstra <peterz@...radead.org>,
        Chris Metcalf <cmetcalf@...hip.com>,
        Christoph Lameter <cl@...ux.com>,
        Kirill Tkhai <tkhai@...dex.ru>, Mike Galbraith <efault@....de>,
        Thomas Gleixner <tglx@...utronix.de>,
        Ingo Molnar <mingo@...nel.org>,
        Linux List Kernel Mailing <linux-kernel@...r.kernel.org>
Subject: Re: [BUG] Use of probe_kernel_address() in task_rcu_dereference()
 without checking return value

On Fri, Aug 30, 2019 at 9:44 AM Oleg Nesterov <oleg@...hat.com> wrote:
>
> ->curr is not protected by RCU, the last schedule does put_task_struct()
> in finish_task_switch().

Right you are.

It's only the sighand allocation itself that is RCU-allocated (using
SLAB_TYPESAFE_BY_RCU, so only the backing page freeing is RCU-delayed,
it can be re-used immediately).

For some reason I thought the main thread struct was too, but that was
just my fevered imagination.

> Of course we can change this and add another call_rcu (actually we can do
> better), and after that we do not need task_rcu_dereference() at all.

No, we wouldn't do another RCU call, we'd just make task_struct_cachep
be SLAB_TYPESAFE_BY_RCU too. In the reuse case, you have no cost at
all.

However, the overhead of RCU freeing is real. It's much less for the
SLAB_TYPESAFE_BY_RCU case (at least for small allocations) than for
"free every single allocaiton by RCU", but it's still very real.

(For small allocations, you only take the RCU hit when you free the
backing store of the pages, which is much less frequent - but for
something big like the task_struct, I don't know how much of a
buffering the slab allocation ends up being)

So it's probably better to take the hit at task_rcu_dereference() than
add RCU logic to the task freeing.

              Linus