[PATCH v4 2/8] ima: define ima_max_digest_data struct without a flexible... | linux-integrity

[PATCH v4 2/8] ima: define ima_max_digest_data struct without a flexible array variable

From: Mimi Zohar <zohar@linux.ibm.com>
Date: 2022-02-08 01:48:26
Also in: linux-fscrypt, lkml
Subsystem: extended verification module (evm), integrity measurement architecture (ima), security subsystem, the rest · Maintainers: Mimi Zohar, Roberto Sassu, Dmitry Kasatkin, Paul Moore, James Morris, "Serge E. Hallyn", Linus Torvalds

To support larger hash digests in the 'iint' cache, instead of defining
the 'digest' field as the maximum digest size, the 'digest' field was
defined as a flexible array variable and was dynamically allocated.
This resulted in wrapping the "ima_digest_data" struct inside a local
structure with the maximum digest size in a number of places.

The original reason for defining the 'digest' field as a flexible array
variable is still valid for the 'iint' cache use case.  In addition,
define 'ima_max_digest_data' struct to be use instead of the (ugly)
local wrapping of the "ima_digest_data" struct.

Signed-off-by: Mimi Zohar <zohar@linux.ibm.com>
---
 security/integrity/ima/ima_api.c  | 20 ++++++++++----------
 security/integrity/ima/ima_init.c | 10 ++++------
 security/integrity/integrity.h    | 24 ++++++++++++++++++++++++
 3 files changed, 38 insertions(+), 16 deletions(-)

diff --git a/security/integrity/ima/ima_api.c b/security/integrity/ima/ima_api.c
index 5b220a2fe573..45294f18dabc 100644
--- a/security/integrity/ima/ima_api.c
+++ b/security/integrity/ima/ima_api.c

@@ -217,14 +217,11 @@ int ima_collect_measurement(struct integrity_iint_cache *iint,
 	const char *audit_cause = "failed";
 	struct inode *inode = file_inode(file);
 	const char *filename = file->f_path.dentry->d_name.name;
+	struct ima_max_digest_data hash;
 	int result = 0;
 	int length;
 	void *tmpbuf;
 	u64 i_version;
-	struct {
-		struct ima_digest_data hdr;
-		char digest[IMA_MAX_DIGEST_SIZE];
-	} hash;
 
 	/*
 	 * Always collect the modsig, because IMA might have already collected

@@ -239,24 +236,27 @@ int ima_collect_measurement(struct integrity_iint_cache *iint,
 
 	/*
 	 * Detecting file change is based on i_version. On filesystems
-	 * which do not support i_version, support is limited to an initial
-	 * measurement/appraisal/audit.
+	 * which do not support i_version, support was originally limited
+	 * to an initial measurement/appraisal/audit, but was modified to
+	 * assume the file changed.
 	 */
 	i_version = inode_query_iversion(inode);
-	hash.hdr.algo = algo;
+	hash.algo = algo;
 
 	/* Initialize hash digest to 0's in case of failure */
 	memset(&hash.digest, 0, sizeof(hash.digest));
 
 	if (buf)
-		result = ima_calc_buffer_hash(buf, size, &hash.hdr);
+		result = ima_calc_buffer_hash(buf, size,
+					      (struct ima_digest_data *)&hash);
 	else
-		result = ima_calc_file_hash(file, &hash.hdr);
+		result = ima_calc_file_hash(file,
+					    (struct ima_digest_data *)&hash);
 
 	if (result && result != -EBADF && result != -EINVAL)
 		goto out;
 
-	length = sizeof(hash.hdr) + hash.hdr.length;
+	length = sizeof(struct ima_digest_data) + hash.length;
 	tmpbuf = krealloc(iint->ima_hash, length, GFP_NOFS);
 	if (!tmpbuf) {
 		result = -ENOMEM;

diff --git a/security/integrity/ima/ima_init.c b/security/integrity/ima/ima_init.c
index b26fa67476b4..890821af08dd 100644
--- a/security/integrity/ima/ima_init.c
+++ b/security/integrity/ima/ima_init.c

@@ -47,16 +47,13 @@ static int __init ima_add_boot_aggregate(void)
 	struct integrity_iint_cache tmp_iint, *iint = &tmp_iint;
 	struct ima_event_data event_data = { .iint = iint,
 					     .filename = boot_aggregate_name };
+	struct ima_max_digest_data hash;
 	int result = -ENOMEM;
 	int violation = 0;
-	struct {
-		struct ima_digest_data hdr;
-		char digest[TPM_MAX_DIGEST_SIZE];
-	} hash;
 
 	memset(iint, 0, sizeof(*iint));
 	memset(&hash, 0, sizeof(hash));
-	iint->ima_hash = &hash.hdr;
+	iint->ima_hash = (struct ima_digest_data *)&hash;
 	iint->ima_hash->algo = ima_hash_algo;
 	iint->ima_hash->length = hash_digest_size[ima_hash_algo];

@@ -73,7 +70,8 @@ static int __init ima_add_boot_aggregate(void)
 	 * is not found.
 	 */
 	if (ima_tpm_chip) {
-		result = ima_calc_boot_aggregate(&hash.hdr);
+		result = ima_calc_boot_aggregate((struct ima_digest_data *)
+						  &hash);
 		if (result < 0) {
 			audit_cause = "hashing_error";
 			goto err_out;

diff --git a/security/integrity/integrity.h b/security/integrity/integrity.h
index d045dccd415a..ee2e6b7c7575 100644
--- a/security/integrity/integrity.h
+++ b/security/integrity/integrity.h

@@ -15,6 +15,7 @@
 #include <linux/types.h>
 #include <linux/integrity.h>
 #include <crypto/sha1.h>
+#include <crypto/hash.h>
 #include <linux/key.h>
 #include <linux/audit.h>

@@ -110,6 +111,29 @@ struct ima_digest_data {
 	u8 digest[];
 } __packed;
 
+/*
+ * Instead of dynamically allocating memory for the ima_digest_data struct
+ * with space for the specific hash algo or wrapping the ima_digest_data
+ * struct inside another local structure, define ima_max_digest_data struct
+ * with the maximum digest size.
+ */
+struct ima_max_digest_data {
+	u8 algo;
+	u8 length;
+	union {
+		struct {
+			u8 unused;
+			u8 type;
+		} sha1;
+		struct {
+			u8 type;
+			u8 algo;
+		} ng;
+		u8 data[2];
+	} xattr;
+	u8 digest[HASH_MAX_DIGESTSIZE];
+} __packed;
+
 /*
  * signature format v2 - for using with asymmetric keys
  */

-- 
2.27.0

`h`	back out one level
`j`	next message in thread
`k`	previous message in thread
`l`	drill in
`Esc`	close help / fold thread tree
`?`	toggle this help