[PATCH] keys: serialise instantiation against keyctl_chown_key

[Sinan Turunc <enscrpt@pm.me>]

keyctl_chown_key() replaces key->user under key->sem, but the
instantiation path reads key->user without it: __key_instantiate_and_link()
and key_reject_and_link() bump key->user->nikeys under
key_construction_mutex only, and key_payload_reserve(), called from the
type's instantiate op, reads key->user once for spin_lock_irqsave() and
again for spin_unlock_irqrestore().

A chown of an under-construction key that lands between those two reads
locks the old owner's quota spinlock and unlocks the new owner's.  The
paravirt unlock slow path reports the unheld unlock and the old lock is
never released, so the next quota operation on that uid spins with
interrupts disabled:

  pvqspinlock: lock 0xffff88800f94dc38 has corrupted value 0x0!
  WARNING: CPU: 2 PID: 215 at kernel/locking/qspinlock_paravirt.h:504 __pv_queued_spin_unlock_slowpath+0x1dc/0x2e0
  Call Trace:
   _raw_spin_unlock_irqrestore+0xe/0x30
   key_payload_reserve+0x230/0x300
   generic_key_instantiate+0x47/0x230
   __key_instantiate_and_link+0xbd/0x310
   key_instantiate_and_link+0x3f0/0x490
   keyctl_instantiate_key_common+0x246/0x410

The same unsynchronised read lets the instantiator touch the old
key_user after the chown has dropped the last reference to it.

Hold key->sem for write across the instantiate op and the owner-record
updates so that instantiation and chown serialise on the lock that
already protects key->user.  key->sem nests inside key_construction_mutex
and the destination keyring's semaphore here; nothing takes the
construction mutex under a key's semaphore.  The acquisition is
annotated as nested because a keyring instantiated into a keyring shares
the destination's lock class; the key under construction cannot be a
destination itself, so no cycle exists.

Reproducer: a request_key() helper that chowns the target key between
two uids from one thread while instantiating it from another, as root.
Triggers within tens of milliseconds on 6.12.104 and on mainline.

Fixes: 1da177e4c3f4 ("Linux-2.6.12-rc2")
Cc: stable@vger.kernel.org
Signed-off-by: Sinan Turunc <enscrpt@pm.me>
---
An AI tool helped with the analysis and the patch. I reviewed and
tested it myself: the unpatched kernel hangs within 40 ms, the patched
one ran the same workload for 60 s with lockdep on and no report.

 security/keys/key.c | 13 +++++++++++--
 1 file changed, 11 insertions(+), 2 deletions(-)

diff --git a/security/keys/key.c b/security/keys/key.c
--- a/security/keys/key.c
+++ b/security/keys/key.c
@@ -439,7 +439,13 @@
 
 	mutex_lock(&key_construction_mutex);
 
-	/* can't instantiate twice */
+	/* can't instantiate twice; key->sem keeps key->user stable against
+	 * keyctl_chown_key() for the instantiate op and the quota update.
+	 * Nested: the caller may hold the destination keyring's sem and the
+	 * key under construction may be a keyring too, but it cannot be
+	 * anyone's destination yet, so there is no cycle.
+	 */
+	down_write_nested(&key->sem, 1);
 	if (key->state == KEY_IS_UNINSTANTIATED) {
 		/* instantiate the key */
 		ret = key->type->instantiate(key, prep);
@@ -469,6 +475,7 @@
 				key_set_expiry(key, prep->expiry);
 		}
 	}
+	up_write(&key->sem);
 
 	mutex_unlock(&key_construction_mutex);
 
@@ -600,7 +607,8 @@
 
 	mutex_lock(&key_construction_mutex);
 
-	/* can't instantiate twice */
+	/* can't instantiate twice; see __key_instantiate_and_link() */
+	down_write_nested(&key->sem, 1);
 	if (key->state == KEY_IS_UNINSTANTIATED) {
 		/* mark the key as being negatively instantiated */
 		atomic_inc(&key->user->nikeys);
@@ -621,6 +629,7 @@
 		if (authkey)
 			key_invalidate(authkey);
 	}
+	up_write(&key->sem);
 
 	mutex_unlock(&key_construction_mutex);
 
-- 
2.56.0