linux-kernel - [PATCH] KEYS: fix key refcount leak in keyctl_assume

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

Message-Id: <20170918183612.113941-1-ebiggers3@gmail.com>
Date:   Mon, 18 Sep 2017 11:36:12 -0700
From:   Eric Biggers <ebiggers3@...il.com>
To:     keyrings@...r.kernel.org
Cc:     David Howells <dhowells@...hat.com>,
        Michael Halcrow <mhalcrow@...gle.com>,
        linux-security-module@...r.kernel.org,
        linux-kernel@...r.kernel.org, Eric Biggers <ebiggers@...gle.com>
Subject: [PATCH] KEYS: fix key refcount leak in keyctl_assume_authority()

From: Eric Biggers <ebiggers@...gle.com>

In keyctl_assume_authority(), if keyctl_change_reqkey_auth() were to
fail, we would leak the reference to the 'authkey'.  Currently this can
only happen if prepare_creds() fails to allocate memory.  But it still
should be fixed, as it is a more severe bug waiting to happen.

This patch also moves the read of 'authkey->serial' to before the
reference to the authkey is dropped.  Doing the read after dropping the
reference is very fragile because it assumes we still hold another
reference to the key.  (Which we do, in current->cred->request_key_auth,
but there's no reason not to write it in the "obviously correct" way.)

Fixes: d84f4f992cbd ("CRED: Inaugurate COW credentials")
Signed-off-by: Eric Biggers <ebiggers@...gle.com>
---
 security/keys/keyctl.c | 6 ++----
 1 file changed, 2 insertions(+), 4 deletions(-)

diff --git a/security/keys/keyctl.c b/security/keys/keyctl.c
index 6a82090c7fc1..552e4460683b 100644
--- a/security/keys/keyctl.c
+++ b/security/keys/keyctl.c
@@ -1411,11 +1411,9 @@ long keyctl_assume_authority(key_serial_t id)
 	}

 	ret = keyctl_change_reqkey_auth(authkey);
-	if (ret < 0)
-		goto error;
+	if (ret == 0)
+		ret = authkey->serial;
 	key_put(authkey);
-
-	ret = authkey->serial;
 error:
 	return ret;
 }
-- 
2.14.1.690.gbb1197296e-goog