Linux-Security-Module Archive on lore.kernel.org
 help / color / Atom feed
* [PATCH 1/5] ima: Set file->f_mode instead of file->f_flags in ima_calc_file_hash()
@ 2020-03-25 16:11 Roberto Sassu
  2020-03-25 16:11 ` [PATCH 2/5] evm: Check also if *tfm is an error pointer in init_desc() Roberto Sassu
                   ` (2 more replies)
  0 siblings, 3 replies; 5+ messages in thread
From: Roberto Sassu @ 2020-03-25 16:11 UTC (permalink / raw)
  To: zohar
  Cc: linux-integrity, linux-security-module, linux-kernel,
	krzysztof.struczynski, silviu.vlasceanu, Roberto Sassu, stable

Commit a408e4a86b36 ("ima: open a new file instance if no read
permissions") tries to create a new file descriptor to calculate a file
digest if the file has not been opened with O_RDONLY flag. However, if a
new file descriptor cannot be obtained, it sets the FMODE_READ flag to
file->f_flags instead of file->f_mode.

This patch fixes this issue by replacing f_flags with f_mode as it was
before that commit.

Cc: stable@vger.kernel.org # 4.20.x
Fixes: a408e4a86b36 ("ima: open a new file instance if no read permissions")
Signed-off-by: Roberto Sassu <roberto.sassu@huawei.com>
---
 security/integrity/ima/ima_crypto.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/security/integrity/ima/ima_crypto.c b/security/integrity/ima/ima_crypto.c
index 423c84f95a14..8ab17aa867dd 100644
--- a/security/integrity/ima/ima_crypto.c
+++ b/security/integrity/ima/ima_crypto.c
@@ -436,7 +436,7 @@ int ima_calc_file_hash(struct file *file, struct ima_digest_data *hash)
 			 */
 			pr_info_ratelimited("Unable to reopen file for reading.\n");
 			f = file;
-			f->f_flags |= FMODE_READ;
+			f->f_mode |= FMODE_READ;
 			modified_flags = true;
 		} else {
 			new_file_instance = true;
@@ -456,7 +456,7 @@ int ima_calc_file_hash(struct file *file, struct ima_digest_data *hash)
 	if (new_file_instance)
 		fput(f);
 	else if (modified_flags)
-		f->f_flags &= ~FMODE_READ;
+		f->f_mode &= ~FMODE_READ;
 	return rc;
 }
 
-- 
2.17.1


^ permalink raw reply	[flat|nested] 5+ messages in thread

* [PATCH 2/5] evm: Check also if *tfm is an error pointer in init_desc()
  2020-03-25 16:11 [PATCH 1/5] ima: Set file->f_mode instead of file->f_flags in ima_calc_file_hash() Roberto Sassu
@ 2020-03-25 16:11 ` Roberto Sassu
  2020-03-25 16:11 ` [PATCH 3/5] ima: Fix ima digest hash table key calculation Roberto Sassu
  2020-03-25 16:14 ` [PATCH 4/5] ima: Remove redundant policy rule set in add_rules() Roberto Sassu
  2 siblings, 0 replies; 5+ messages in thread
From: Roberto Sassu @ 2020-03-25 16:11 UTC (permalink / raw)
  To: zohar
  Cc: linux-integrity, linux-security-module, linux-kernel,
	krzysztof.struczynski, silviu.vlasceanu, Roberto Sassu, stable

The mutex in init_desc(), introduced by commit 97426f985729 ("evm: prevent
racing during tfm allocation") prevents two tasks to concurrently set *tfm.
However, checking if *tfm is NULL is not enough, as crypto_alloc_shash()
can return an error pointer. The following sequence can happen:

Task A: *tfm = crypto_alloc_shash() <= error pointer
Task B: if (*tfm == NULL) <= *tfm is not NULL, use it
Task B: rc = crypto_shash_init(desc) <= panic
Task A: *tfm = NULL

This patch uses the IS_ERR_OR_NULL macro to determine whether or not a new
crypto context must be created.

Cc: stable@vger.kernel.org
Fixes: 97426f985729 ("evm: prevent racing during tfm allocation")
Co-developed-by: Krzysztof Struczynski <krzysztof.struczynski@huawei.com>
Signed-off-by: Krzysztof Struczynski <krzysztof.struczynski@huawei.com>
Signed-off-by: Roberto Sassu <roberto.sassu@huawei.com>
---
 security/integrity/evm/evm_crypto.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/security/integrity/evm/evm_crypto.c b/security/integrity/evm/evm_crypto.c
index 35682852ddea..77ad1e5a93e4 100644
--- a/security/integrity/evm/evm_crypto.c
+++ b/security/integrity/evm/evm_crypto.c
@@ -91,7 +91,7 @@ static struct shash_desc *init_desc(char type, uint8_t hash_algo)
 		algo = hash_algo_name[hash_algo];
 	}
 
-	if (*tfm == NULL) {
+	if (IS_ERR_OR_NULL(*tfm)) {
 		mutex_lock(&mutex);
 		if (*tfm)
 			goto out;
-- 
2.17.1


^ permalink raw reply	[flat|nested] 5+ messages in thread

* [PATCH 3/5] ima: Fix ima digest hash table key calculation
  2020-03-25 16:11 [PATCH 1/5] ima: Set file->f_mode instead of file->f_flags in ima_calc_file_hash() Roberto Sassu
  2020-03-25 16:11 ` [PATCH 2/5] evm: Check also if *tfm is an error pointer in init_desc() Roberto Sassu
@ 2020-03-25 16:11 ` Roberto Sassu
  2020-03-25 16:14 ` [PATCH 4/5] ima: Remove redundant policy rule set in add_rules() Roberto Sassu
  2 siblings, 0 replies; 5+ messages in thread
From: Roberto Sassu @ 2020-03-25 16:11 UTC (permalink / raw)
  To: zohar
  Cc: linux-integrity, linux-security-module, linux-kernel,
	krzysztof.struczynski, silviu.vlasceanu, stable

From: Krzysztof Struczynski <krzysztof.struczynski@huawei.com>

Function hash_long() accepts unsigned long, while currently only one byte
is passed from ima_hash_key(), which calculates a key for ima_htable. Use
more bytes to avoid frequent collisions.

Length of the buffer is not explicitly passed as a function parameter,
because this function expects a digest whose length is greater than the
size of unsigned long.

Cc: stable@vger.kernel.org
Fixes: 3323eec921ef ("integrity: IMA as an integrity service provider")
Signed-off-by: Krzysztof Struczynski <krzysztof.struczynski@huawei.com>
---
 security/integrity/ima/ima.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/security/integrity/ima/ima.h b/security/integrity/ima/ima.h
index 64317d95363e..cf0022c2bc14 100644
--- a/security/integrity/ima/ima.h
+++ b/security/integrity/ima/ima.h
@@ -177,7 +177,7 @@ extern struct ima_h_table ima_htable;
 
 static inline unsigned long ima_hash_key(u8 *digest)
 {
-	return hash_long(*digest, IMA_HASH_BITS);
+	return hash_long(*((unsigned long *)digest), IMA_HASH_BITS);
 }
 
 #define __ima_hooks(hook)		\
-- 
2.17.1


^ permalink raw reply	[flat|nested] 5+ messages in thread

* [PATCH 4/5] ima: Remove redundant policy rule set in add_rules()
  2020-03-25 16:11 [PATCH 1/5] ima: Set file->f_mode instead of file->f_flags in ima_calc_file_hash() Roberto Sassu
  2020-03-25 16:11 ` [PATCH 2/5] evm: Check also if *tfm is an error pointer in init_desc() Roberto Sassu
  2020-03-25 16:11 ` [PATCH 3/5] ima: Fix ima digest hash table key calculation Roberto Sassu
@ 2020-03-25 16:14 ` Roberto Sassu
  2020-03-25 16:14   ` [PATCH 5/5] ima: Remove unused build_ima_appraise variable Roberto Sassu
  2 siblings, 1 reply; 5+ messages in thread
From: Roberto Sassu @ 2020-03-25 16:14 UTC (permalink / raw)
  To: zohar
  Cc: linux-integrity, linux-security-module, linux-kernel,
	krzysztof.struczynski, silviu.vlasceanu

From: Krzysztof Struczynski <krzysztof.struczynski@huawei.com>

Function ima_appraise_flag() returns the flag to be set in
temp_ima_appraise depending on the hook identifier passed as an argument.
It is not necessary to set the flag again for the POLICY_CHECK hook.

Signed-off-by: Krzysztof Struczynski <krzysztof.struczynski@huawei.com>
---
 security/integrity/ima/ima_policy.c | 5 +----
 1 file changed, 1 insertion(+), 4 deletions(-)

diff --git a/security/integrity/ima/ima_policy.c b/security/integrity/ima/ima_policy.c
index c334e0dc6083..ea9b991f0232 100644
--- a/security/integrity/ima/ima_policy.c
+++ b/security/integrity/ima/ima_policy.c
@@ -643,11 +643,8 @@ static void add_rules(struct ima_rule_entry *entries, int count,
 
 			list_add_tail(&entry->list, &ima_policy_rules);
 		}
-		if (entries[i].action == APPRAISE) {
+		if (entries[i].action == APPRAISE)
 			temp_ima_appraise |= ima_appraise_flag(entries[i].func);
-			if (entries[i].func == POLICY_CHECK)
-				temp_ima_appraise |= IMA_APPRAISE_POLICY;
-		}
 	}
 }
 
-- 
2.17.1


^ permalink raw reply	[flat|nested] 5+ messages in thread

* [PATCH 5/5] ima: Remove unused build_ima_appraise variable
  2020-03-25 16:14 ` [PATCH 4/5] ima: Remove redundant policy rule set in add_rules() Roberto Sassu
@ 2020-03-25 16:14   ` Roberto Sassu
  0 siblings, 0 replies; 5+ messages in thread
From: Roberto Sassu @ 2020-03-25 16:14 UTC (permalink / raw)
  To: zohar
  Cc: linux-integrity, linux-security-module, linux-kernel,
	krzysztof.struczynski, silviu.vlasceanu

From: Krzysztof Struczynski <krzysztof.struczynski@huawei.com>

After adding the new add_rule() function in commit c52657d93b05
("ima: refactor ima_init_policy()"), all appraisal flags are added to the
temp_ima_appraise variable. Remove build_ima_appraise that is not set
anymore.

Signed-off-by: Krzysztof Struczynski <krzysztof.struczynski@huawei.com>
---
 security/integrity/ima/ima_policy.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/security/integrity/ima/ima_policy.c b/security/integrity/ima/ima_policy.c
index ea9b991f0232..fcc26bddd7fc 100644
--- a/security/integrity/ima/ima_policy.c
+++ b/security/integrity/ima/ima_policy.c
@@ -48,7 +48,6 @@
 
 int ima_policy_flag;
 static int temp_ima_appraise;
-static int build_ima_appraise __ro_after_init;
 
 #define MAX_LSM_RULES 6
 enum lsm_rule_types { LSM_OBJ_USER, LSM_OBJ_ROLE, LSM_OBJ_TYPE,
@@ -606,7 +605,7 @@ void ima_update_policy_flag(void)
 			ima_policy_flag |= entry->action;
 	}
 
-	ima_appraise |= (build_ima_appraise | temp_ima_appraise);
+	ima_appraise |= temp_ima_appraise;
 	if (!ima_appraise)
 		ima_policy_flag &= ~IMA_APPRAISE;
 }
-- 
2.17.1


^ permalink raw reply	[flat|nested] 5+ messages in thread

end of thread, back to index

Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-03-25 16:11 [PATCH 1/5] ima: Set file->f_mode instead of file->f_flags in ima_calc_file_hash() Roberto Sassu
2020-03-25 16:11 ` [PATCH 2/5] evm: Check also if *tfm is an error pointer in init_desc() Roberto Sassu
2020-03-25 16:11 ` [PATCH 3/5] ima: Fix ima digest hash table key calculation Roberto Sassu
2020-03-25 16:14 ` [PATCH 4/5] ima: Remove redundant policy rule set in add_rules() Roberto Sassu
2020-03-25 16:14   ` [PATCH 5/5] ima: Remove unused build_ima_appraise variable Roberto Sassu

Linux-Security-Module Archive on lore.kernel.org

Archives are clonable:
	git clone --mirror https://lore.kernel.org/linux-security-module/0 linux-security-module/git/0.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 linux-security-module linux-security-module/ https://lore.kernel.org/linux-security-module \
		linux-security-module@vger.kernel.org
	public-inbox-index linux-security-module

Example config snippet for mirrors

Newsgroup available over NNTP:
	nntp://nntp.lore.kernel.org/org.kernel.vger.linux-security-module


AGPL code for this site: git clone https://public-inbox.org/public-inbox.git