linux-kernel - [PATCH RFC] cred: Add WARN to detect wrong use of get/put

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 for Android: free password hash cracker in your pocket

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

Date:   Fri, 12 Jun 2020 18:28:15 +0800
From:   Xiaoming Ni <nixiaoming@...wei.com>
To:     <paul@...l-moore.com>, <edumazet@...gle.com>,
        <peterz@...radead.org>, <paulmck@...nel.org>,
        <dhowells@...hat.com>, <keescook@...omium.org>,
        <shakeelb@...gle.com>, <jamorris@...ux.microsoft.com>
CC:     <nixiaoming@...wei.com>, <alex.huangjianhui@...wei.com>,
        <dylix.dailei@...wei.com>, <chenzefeng2@...wei.com>,
        <linux-kernel@...r.kernel.org>
Subject: [PATCH RFC] cred: Add WARN to detect wrong use of get/put_cred

Cred release and usage check code flow:
	1. put_cred()
		if (atomic_dec_and_test(&(cred)->usage))
			__put_cred(cred);

	2. __put_cred()
		BUG_ON(atomic_read(&cred->usage) != 0);
		call_rcu(&cred->rcu, put_cred_rcu);

	3. put_cred_rcu()
		if (atomic_read(&cred->usage) != 0)
			panic("CRED: put_cred_rcu() sees %p with usage %d\n",
			       cred, atomic_read(&cred->usage));
		kmem_cache_free(cred_jar, cred);

If panic is triggered on put_cred_rcu(), there are two possibilities
	1. Call get_cred() after __put_cred(), usage > 0
	2. Call put_cred() after __put_cred(), usage < 0
Since put_cred_rcu is an asynchronous behavior, it is no longer the first
scene when panic, there is no information about the murderer in the panic
call stack...

So, add WARN() in get_cred()/put_cred(), and pray to catch the murderer
at the first scene.

Signed-off-by: Xiaoming Ni <nixiaoming@...wei.com>
---
 include/linux/cred.h | 14 +++++++++++---
 1 file changed, 11 insertions(+), 3 deletions(-)

diff --git a/include/linux/cred.h b/include/linux/cred.h
index 18639c0..c00d5a1 100644
--- a/include/linux/cred.h
+++ b/include/linux/cred.h
@@ -224,11 +224,16 @@ static inline bool cap_ambient_invariant_ok(const struct cred *cred)
  *
  * Get a reference on the specified set of new credentials.  The caller must
  * release the reference.
+ *
+ * Initialize usage to 1 during cred resource allocation,
+ * so when calling get_cred, usage cannot be 0.
  */
 static inline struct cred *get_new_cred(struct cred *cred)
 {
-	atomic_inc(&cred->usage);
-	return cred;
+	if (atomic_inc_not_zero(&cred->usage))
+		return cred;
+	WARN(1, "get_new_cred after __put_cred");
+	return NULL;
 }
 
 /**
@@ -280,11 +285,14 @@ static inline const struct cred *get_cred_rcu(const struct cred *cred)
 static inline void put_cred(const struct cred *_cred)
 {
 	struct cred *cred = (struct cred *) _cred;
+	int usage;
 
 	if (cred) {
 		validate_creds(cred);
-		if (atomic_dec_and_test(&(cred)->usage))
+		usage = atomic_dec_return(&(cred)->usage);
+		if (usage == 0)
 			__put_cred(cred);
+		WARN(usage < 0, "put_cred after __put_cred");
 	}
 }
 
-- 
1.8.5.6