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  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

Powered by blists - more mailing lists