Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18;
From:   Xiaoming Ni <nixiaoming@huawei.com>
To:     <paul@paul-moore.com>, <edumazet@google.com>,
        <peterz@infradead.org>, <paulmck@kernel.org>,
        <dhowells@redhat.com>, <keescook@chromium.org>,
        <shakeelb@google.com>, <jamorris@linux.microsoft.com>
CC:     <nixiaoming@huawei.com>, <alex.huangjianhui@huawei.com>,
        <dylix.dailei@huawei.com>, <chenzefeng2@huawei.com>,
        <linux-kernel@vger.kernel.org>
Subject: [PATCH RFC] cred: Add WARN to detect wrong use of get/put_cred
Date:   Fri, 12 Jun 2020 18:28:15 +0800
Message-ID: <1591957695-118312-1-git-send-email-nixiaoming@huawei.com>
MIME-Version: 1.0
Content-Type: text/plain
Sender: linux-kernel-owner@vger.kernel.org
Precedence: bulk

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@huawei.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