[PATCH] evm: check return values of crypto_shash functions

[Mimi Zohar <zohar@linux.ibm.com>]

From: Daniel Hodges <git@danielhodges.dev>

The crypto_shash_update() and crypto_shash_final() functions can fail
and return error codes, but their return values were not being checked
in several places in security/integrity/evm/evm_crypto.c:

- hmac_add_misc() ignored returns from crypto_shash_update() and
  crypto_shash_final()
- evm_calc_hmac_or_hash() ignored returns from crypto_shash_update()
- evm_init_hmac() ignored returns from crypto_shash_update()

If these hash operations fail silently, the resulting HMAC could be
invalid or incomplete, which could weaken the integrity verification
security that EVM provides.

This patch converts hmac_add_misc() from void to int return type and
adds proper error checking and propagation for all crypto_shash_*
function calls. All callers are updated to handle the new return values.
Additionally, error messages are logged when cryptographic operations
fail to provide visibility into the failure rather than silently
returning error codes.

Fixes: 66dbc325afce ("evm: re-release")
Signed-off-by: Daniel Hodges <git@danielhodges.dev>
Reviewed-by: Roberto Sassu <roberto.sassu@huawei.com>
Link: https://github.com/linux-integrity/linux/issues/17
[zohar@linux.ibm.com: fixed commit 0496fc9cdc38 merge conflict,
checkpatch.pl warnings]
Signed-off-by: Mimi Zohar <zohar@linux.ibm.com>
---
Daniel, Roberto, please review this version of the patch.

 security/integrity/evm/evm_crypto.c | 67 ++++++++++++++++++++++-------
 1 file changed, 51 insertions(+), 16 deletions(-)

diff --git a/security/integrity/evm/evm_crypto.c b/security/integrity/evm/evm_crypto.c
index 1c41af2f91a6..aef0f8f3d9d6 100644
--- a/security/integrity/evm/evm_crypto.c
+++ b/security/integrity/evm/evm_crypto.c
@@ -140,8 +140,8 @@ static struct shash_desc *init_desc(char type, uint8_t hash_algo)
  * (Additional directory/file metadata needs to be added for more complete
  * protection.)
  */
-static void hmac_add_misc(struct shash_desc *desc, struct inode *inode,
-			  char type, char *digest)
+static int hmac_add_misc(struct shash_desc *desc, struct inode *inode,
+			 char type, char *digest)
 {
 	struct h_misc {
 		/*
@@ -156,6 +156,7 @@ static void hmac_add_misc(struct shash_desc *desc, struct inode *inode,
 		gid_t gid;
 		umode_t mode;
 	} hmac_misc;
+	int error;

 	memset(&hmac_misc, 0, sizeof(hmac_misc));
 	/* Don't include the inode or generation number in portable
@@ -176,14 +177,30 @@ static void hmac_add_misc(struct shash_desc *desc, struct inode *inode,
 	hmac_misc.uid = from_kuid(&init_user_ns, inode->i_uid);
 	hmac_misc.gid = from_kgid(&init_user_ns, inode->i_gid);
 	hmac_misc.mode = inode->i_mode;
-	crypto_shash_update(desc, (const u8 *)&hmac_misc, sizeof(hmac_misc));
+	error = crypto_shash_update(desc, (const u8 *)&hmac_misc,
+				    sizeof(hmac_misc));
+	if (error) {
+		pr_err("crypto_shash_update() failed: %d\n", error);
+		return error;
+	}
 	if ((evm_hmac_attrs & EVM_ATTR_FSUUID) &&
-	    type != EVM_XATTR_PORTABLE_DIGSIG)
-		crypto_shash_update(desc, (u8 *)&inode->i_sb->s_uuid, UUID_SIZE);
-	crypto_shash_final(desc, digest);
+	    type != EVM_XATTR_PORTABLE_DIGSIG) {
+		error = crypto_shash_update(desc, (u8 *)&inode->i_sb->s_uuid,
+					    UUID_SIZE);
+		if (error) {
+			pr_err("crypto_shash_update() failed: %d\n", error);
+			return error;
+		}
+	}
+	error = crypto_shash_final(desc, digest);
+	if (error) {
+		pr_err("crypto_shash_final() failed: %d\n", error);
+		return error;
+	}

 	pr_debug("hmac_misc: (%zu) [%*phN]\n", sizeof(struct h_misc),
 		 (int)sizeof(struct h_misc), &hmac_misc);
+	return 0;
 }

 /*
@@ -267,9 +284,14 @@ static int evm_calc_hmac_or_hash(struct dentry *dentry,

 		if ((req_xattr_name && req_xattr_value)
 		    && !strcmp(xattr->name, req_xattr_name)) {
-			error = 0;
-			crypto_shash_update(desc, (const u8 *)req_xattr_value,
-					     req_xattr_value_len);
+			error = crypto_shash_update(desc,
+						    (const u8 *)req_xattr_value,
+						    req_xattr_value_len);
+			if (error) {
+				pr_err("crypto_shash_update() failed: %d\n",
+				       error);
+				goto out;
+			}
 			if (is_ima)
 				ima_present = true;

@@ -293,15 +315,21 @@ static int evm_calc_hmac_or_hash(struct dentry *dentry,
 			pr_debug("file %s: xattr %s size mismatch (kernel: %d, user: %d)\n",
 				 dentry->d_name.name, xattr->name, size,
 				 user_space_size);
-		error = 0;
 		xattr_size = size;
-		crypto_shash_update(desc, (const u8 *)xattr_value, xattr_size);
+		error = crypto_shash_update(desc, (const u8 *)xattr_value,
+					    xattr_size);
+		if (error) {
+			pr_err("crypto_shash_update() failed: %d\n", error);
+			goto out;
+		}
 		if (is_ima)
 			ima_present = true;

 		dump_security_xattr(xattr->name, xattr_value, xattr_size);
 	}
-	hmac_add_misc(desc, inode, type, data->digest);
+	error = hmac_add_misc(desc, inode, type, data->digest);
+	if (error)
+		goto out;

 	if (inode != d_backing_inode(dentry) && iint) {
 		if (IS_I_VERSION(inode))
@@ -409,6 +437,7 @@ int evm_init_hmac(struct inode *inode, const struct xattr *xattrs,
 	struct shash_desc *desc;
 	const struct xattr *xattr;
 	struct xattr_list *xattr_entry;
+	int error;

 	desc = init_desc(EVM_XATTR_HMAC, HASH_ALGO_SHA1);
 	if (IS_ERR(desc)) {
@@ -423,14 +452,20 @@ int evm_init_hmac(struct inode *inode, const struct xattr *xattrs,
 				   XATTR_SECURITY_PREFIX_LEN, xattr->name) != 0)
 				continue;

-			crypto_shash_update(desc, xattr->value,
-					    xattr->value_len);
+			error = crypto_shash_update(desc, xattr->value,
+						    xattr->value_len);
+			if (error) {
+				pr_err("crypto_shash_update() failed: %d\n",
+				       error);
+				goto out;
+			}
 		}
 	}

-	hmac_add_misc(desc, inode, EVM_XATTR_HMAC, hmac_val);
+	error = hmac_add_misc(desc, inode, EVM_XATTR_HMAC, hmac_val);
+out:
 	kfree(desc);
-	return 0;
+	return error;
 }

 /*
--
2.55.0