linux-kernel - [PATCH] integrity: Fix possible multiple allocation in integrity_inode

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

Message-Id: <20230530121453.10249-1-tianjia.zhang@linux.alibaba.com>
Date:   Tue, 30 May 2023 20:14:53 +0800
From:   Tianjia Zhang <tianjia.zhang@...ux.alibaba.com>
To:     Mimi Zohar <zohar@...ux.ibm.com>,
        Dmitry Kasatkin <dmitry.kasatkin@...il.com>,
        Paul Moore <paul@...l-moore.com>,
        James Morris <jmorris@...ei.org>,
        "Serge E. Hallyn" <serge@...lyn.com>,
        linux-integrity@...r.kernel.org,
        linux-security-module@...r.kernel.org, linux-kernel@...r.kernel.org
Cc:     Tianjia Zhang <tianjia.zhang@...ux.alibaba.com>
Subject: [PATCH] integrity: Fix possible multiple allocation in integrity_inode_get()

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@...ux.alibaba.com>
Cc: Dmitry Kasatkin <dmitry.kasatkin@...il.com>
Cc: <stable@...r.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)