public inbox for linux-kernel@vger.kernel.org
 help / color / mirror / Atom feed
* [PATCH] evm: check return values of crypto_shash functions
@ 2026-01-31 18:22 Daniel Hodges
  2026-02-04 12:50 ` Roberto Sassu
  0 siblings, 1 reply; 9+ messages in thread
From: Daniel Hodges @ 2026-01-31 18:22 UTC (permalink / raw)
  To: zohar, roberto.sassu
  Cc: dmitry.kasatkin, eric.snowberg, paul, jmorris, serge,
	linux-integrity, linux-security-module, linux-kernel,
	Daniel Hodges

The crypto_shash_update() and crypto_shash_final() functions can fail
and return error codes, but their return values were being ignored in
several places in evm_crypto.c:

  - hmac_add_misc(): ignores returns from crypto_shash_update() and
    crypto_shash_final()
  - evm_calc_hmac_or_hash(): ignores returns from crypto_shash_update()
  - evm_init_hmac(): ignores returns from crypto_shash_update()

If these hash operations fail silently, the resulting HMAC could be
invalid or incomplete. This could potentially allow integrity
verification to pass with incorrect HMACs, weakening EVM's security
guarantees.

Fix this by:
  - Changing hmac_add_misc() from void to int return type
  - Checking and propagating error codes from all crypto_shash calls
  - Updating all callers to check the return values

Fixes: 66dbc325afce ("evm: re-release")
Signed-off-by: Daniel Hodges <hodgesd@meta.com>
---
 security/integrity/evm/evm_crypto.c | 45 +++++++++++++++++++----------
 1 file changed, 30 insertions(+), 15 deletions(-)

diff --git a/security/integrity/evm/evm_crypto.c b/security/integrity/evm/evm_crypto.c
index a5e730ffda57..286f23a1a26b 100644
--- a/security/integrity/evm/evm_crypto.c
+++ b/security/integrity/evm/evm_crypto.c
@@ -132,58 +132,65 @@ static struct shash_desc *init_desc(char type, uint8_t hash_algo)
 	}
 	return desc;
 }
 
 /* Protect against 'cutting & pasting' security.evm xattr, include inode
  * specific info.
  *
  * (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 {
 		unsigned long ino;
 		__u32 generation;
 		uid_t uid;
 		gid_t gid;
 		umode_t mode;
 	} hmac_misc;
+	int ret;
 
 	memset(&hmac_misc, 0, sizeof(hmac_misc));
 	/* Don't include the inode or generation number in portable
 	 * signatures
 	 */
 	if (type != EVM_XATTR_PORTABLE_DIGSIG) {
 		hmac_misc.ino = inode->i_ino;
 		hmac_misc.generation = inode->i_generation;
 	}
 	/* The hmac uid and gid must be encoded in the initial user
 	 * namespace (not the filesystems user namespace) as encoding
 	 * them in the filesystems user namespace allows an attack
 	 * where first they are written in an unprivileged fuse mount
 	 * of a filesystem and then the system is tricked to mount the
 	 * filesystem for real on next boot and trust it because
 	 * everything is signed.
 	 */
 	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));
+	ret = crypto_shash_update(desc, (const u8 *)&hmac_misc, sizeof(hmac_misc));
+	if (ret)
+		return ret;
 	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) {
+		ret = crypto_shash_update(desc, (u8 *)&inode->i_sb->s_uuid, UUID_SIZE);
+		if (ret)
+			return ret;
+	}
+	ret = crypto_shash_final(desc, digest);
 
 	pr_debug("hmac_misc: (%zu) [%*phN]\n", sizeof(struct h_misc),
 		 (int)sizeof(struct h_misc), &hmac_misc);
+	return ret;
 }
 
 /*
  * Dump large security xattr values as a continuous ascii hexadecimal string.
  * (pr_debug is limited to 64 bytes.)
  */
 static void dump_security_xattr_l(const char *prefix, const void *src,
 				  size_t count)
 {
 #if defined(DEBUG) || defined(CONFIG_DYNAMIC_DEBUG)
@@ -253,23 +260,24 @@ static int evm_calc_hmac_or_hash(struct dentry *dentry,
 
 		/*
 		 * Skip non-enabled xattrs for locally calculated
 		 * signatures/HMACs.
 		 */
 		if (type != EVM_XATTR_PORTABLE_DIGSIG && !xattr->enabled)
 			continue;
 
 		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)
+				goto out;
 			if (is_ima)
 				ima_present = true;
 
 			dump_security_xattr(req_xattr_name,
 					    req_xattr_value,
 					    req_xattr_value_len);
 			continue;
 		}
 		size = vfs_getxattr_alloc(&nop_mnt_idmap, dentry, xattr->name,
 					  &xattr_value, xattr_size, GFP_NOFS);
@@ -279,29 +287,32 @@ static int evm_calc_hmac_or_hash(struct dentry *dentry,
 		}
 		if (size < 0)
 			continue;
 
 		user_space_size = vfs_getxattr(&nop_mnt_idmap, dentry,
 					       xattr->name, NULL, 0);
 		if (user_space_size != size)
 			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)
+			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))
 			i_version = inode_query_iversion(inode);
 		integrity_inode_attrs_store(&iint->metadata_inode, i_version,
 					    inode);
 	}
 
 	/* Portable EVM signatures must include an IMA hash */
 	if (type == EVM_XATTR_PORTABLE_DIGSIG && !ima_present)
@@ -394,37 +405,41 @@ int evm_update_evmxattr(struct dentry *dentry, const char *xattr_name,
 		rc = __vfs_removexattr(&nop_mnt_idmap, dentry, XATTR_NAME_EVM);
 	}
 	return rc;
 }
 
 int evm_init_hmac(struct inode *inode, const struct xattr *xattrs,
 		  char *hmac_val)
 {
 	struct shash_desc *desc;
 	const struct xattr *xattr;
+	int ret;
 
 	desc = init_desc(EVM_XATTR_HMAC, HASH_ALGO_SHA1);
 	if (IS_ERR(desc)) {
 		pr_info("init_desc failed\n");
 		return PTR_ERR(desc);
 	}
 
 	for (xattr = xattrs; xattr->name; xattr++) {
 		if (!evm_protected_xattr(xattr->name))
 			continue;
 
-		crypto_shash_update(desc, xattr->value, xattr->value_len);
+		ret = crypto_shash_update(desc, xattr->value, xattr->value_len);
+		if (ret)
+			goto out;
 	}
 
-	hmac_add_misc(desc, inode, EVM_XATTR_HMAC, hmac_val);
+	ret = hmac_add_misc(desc, inode, EVM_XATTR_HMAC, hmac_val);
+out:
 	kfree(desc);
-	return 0;
+	return ret;
 }
 
 /*
  * Get the key from the TPM for the SHA1-HMAC
  */
 int evm_init_key(void)
 {
 	struct key *evm_key;
 	struct encrypted_key_payload *ekp;
 	int rc;
-- 
2.47.3


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

end of thread, other threads:[~2026-03-09 15:04 UTC | newest]

Thread overview: 9+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-01-31 18:22 [PATCH] evm: check return values of crypto_shash functions Daniel Hodges
2026-02-04 12:50 ` Roberto Sassu
2026-02-04 15:47   ` Daniel Hodges
2026-02-06  2:42     ` [PATCH v2 v2] " Daniel Hodges
2026-02-19  9:26       ` Roberto Sassu
2026-02-19 12:36         ` Roberto Sassu
2026-02-19 15:01           ` Daniel Hodges
2026-02-20  9:06             ` Roberto Sassu
2026-03-09 15:03         ` Mimi Zohar

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox