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>] [day] [month] [year] [list]
Message-ID: <20260131182233.2365157-1-hodgesd@meta.com>
Date: Sat, 31 Jan 2026 10:22:33 -0800
From: Daniel Hodges <hodgesd@...a.com>
To: <zohar@...ux.ibm.com>, <roberto.sassu@...wei.com>
CC: <dmitry.kasatkin@...il.com>, <eric.snowberg@...cle.com>,
        <paul@...l-moore.com>, <jmorris@...ei.org>, <serge@...lyn.com>,
        <linux-integrity@...r.kernel.org>,
        <linux-security-module@...r.kernel.org>,
        <linux-kernel@...r.kernel.org>, Daniel Hodges <hodgesd@...a.com>
Subject: [PATCH] evm: check return values of crypto_shash functions

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@...a.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


Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ