linux-kernel - Re: Modular BIG_KEYS (was: Re: [PATCH v4] security/keys: rewrite all of big

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] [day] [month] [year] [list]

Message-ID: <22433.1507021470@warthog.procyon.org.uk>
Date:   Tue, 03 Oct 2017 10:04:30 +0100
From:   David Howells <dhowells@...hat.com>
To:     Eric Biggers <ebiggers3@...il.com>
Cc:     dhowells@...hat.com, Geert Uytterhoeven <geert@...ux-m68k.org>,
        "Jason A. Donenfeld" <Jason@...c4.com>,
        linux-security-module@...r.kernel.org, keyrings@...r.kernel.org,
        kernel-hardening@...ts.openwall.com,
        "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
        Herbert Xu <herbert@...dor.apana.org.au>,
        Kirill Marinushkin <k.marinushkin@...il.com>,
        Ard Biesheuvel <ard.biesheuvel@...aro.org>,
        Ilhan Gurel <ilhan.gurel@...il.com>, security@...nel.org,
        stable <stable@...r.kernel.org>
Subject: Re: Modular BIG_KEYS (was: Re: [PATCH v4] security/keys: rewrite all of big_key crypto)

Eric Biggers <ebiggers3@...il.com> wrote:

> (I also still need to convince myself that there aren't any race conditions
> in key type unregistering.  It's a little weird how it changes the key type
> to the ".dead" key type, rather than pinning the key type in memory while
> it's still used.)

Keys are converted to the dead type and their destructors called by the gc
whilst it holds a write lock on the key's semaphore, so keyctl() calls
shouldn't be a problem as they all hold a read-lock or a write-lock on the
content.

/proc/keys does things under the key_serial_lock and RCU, so if a key type's
->describe() function looks at the payload, then it should use RCU conditions.
Note that this doesn't affect the keyring_describe() as that only looks at a
value that is stored in the key struct - and the same for user_describe() and
big_key_describe().  asymmetric_key_describe(), OTOH...  Maybe the simplest
thing to do is to take key_serial_lock in key_garbage_collector() around the
code that changes the key type.

Inside the kernel, things are murkier as the kernel can reach inside a key
without taking the semaphore.  If it does so, it must use RCU or some other
mechanism to prevent the key payload from being destroyed under it.

With AF_RXRPC, I think I'm doing this wrong.  I think I need to move the
destruction of the key type to after I've unregistered the network namespace
handling as the latter destroys connections which might be using the key.

David