Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
From:   Tianjia Zhang <tianjia.zhang@linux.alibaba.com>
To:     Mimi Zohar <zohar@linux.ibm.com>,
        Dmitry Kasatkin <dmitry.kasatkin@gmail.com>,
        Paul Moore <paul@paul-moore.com>,
        James Morris <jmorris@namei.org>,
        "Serge E. Hallyn" <serge@hallyn.com>,
        linux-integrity@vger.kernel.org,
        linux-security-module@vger.kernel.org, linux-kernel@vger.kernel.org
Cc:     Tianjia Zhang <tianjia.zhang@linux.alibaba.com>
Subject: [PATCH] integrity: Fix possible multiple allocation in integrity_inode_get()
Date:   Tue, 30 May 2023 20:14:53 +0800
Message-Id: <20230530121453.10249-1-tianjia.zhang@linux.alibaba.com>
MIME-Version: 1.0
Content-Transfer-Encoding: 8bit
Precedence: bulk

When integrity_inode_get() is querying and inserting the cache, there
is a conditional race in the concurrent environment.

Query iint within the read-lock. If there is no result, allocate iint
first and insert the iint cache in the write-lock protection. When the
iint cache does not exist, and when multiple execution streams come at
the same time, there will be a race condition, and multiple copies of
iint will be allocated at the same time, and then put into the cache
one by one under the write-lock protection.

This is mainly because the red-black tree insertion does not perform
duplicate detection. This is not the desired result, when this
happens, the repeated allocation should be freed and the existing
iint cache should be returned.

Fixes: bf2276d10ce5 ("ima: allocating iint improvements")
Signed-off-by: Tianjia Zhang <tianjia.zhang@linux.alibaba.com>
Cc: Dmitry Kasatkin <dmitry.kasatkin@gmail.com>
Cc: <stable@vger.kernel.org> # v3.10+
---
 security/integrity/iint.c | 13 ++++++++-----
 1 file changed, 8 insertions(+), 5 deletions(-)

diff --git a/security/integrity/iint.c b/security/integrity/iint.c
index c73858e8c6d5..d49c843a88ee 100644
--- a/security/integrity/iint.c
+++ b/security/integrity/iint.c
@@ -43,12 +43,10 @@ static struct integrity_iint_cache *__integrity_iint_find(struct inode *inode)
 		else if (inode > iint->inode)
 			n = n->rb_right;
 		else
-			break;
+			return iint;
 	}
-	if (!n)
-		return NULL;
 
-	return iint;
+	return NULL;
 }
 
 /*
@@ -115,8 +113,13 @@ struct integrity_iint_cache *integrity_inode_get(struct inode *inode)
 				     rb_node);
 		if (inode < test_iint->inode)
 			p = &(*p)->rb_left;
-		else
+		else if (inode > test_iint->inode)
 			p = &(*p)->rb_right;
+		else {
+			write_unlock(&integrity_iint_lock);
+			kmem_cache_free(iint_cache, iint);
+			return test_iint;
+		}
 	}
 
 	iint->inode = inode;
-- 
2.24.3 (Apple Git-128)