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

* Re: [PATCH] evm: check return values of crypto_shash functions
  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
  0 siblings, 1 reply; 9+ messages in thread
From: Roberto Sassu @ 2026-02-04 12:50 UTC (permalink / raw)
  To: Daniel Hodges, zohar, roberto.sassu
  Cc: dmitry.kasatkin, eric.snowberg, paul, jmorris, serge,
	linux-integrity, linux-security-module, linux-kernel

On Sat, 2026-01-31 at 10:22 -0800, Daniel Hodges wrote:
> 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);

Maybe we should also indicate if an error occurred, with a separate
error message, or adding the result in the message below.

Thanks

Roberto
 
>  	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;


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

* Re: [PATCH] evm: check return values of crypto_shash functions
  2026-02-04 12:50 ` Roberto Sassu
@ 2026-02-04 15:47   ` Daniel Hodges
  2026-02-06  2:42     ` [PATCH v2 v2] " Daniel Hodges
  0 siblings, 1 reply; 9+ messages in thread
From: Daniel Hodges @ 2026-02-04 15:47 UTC (permalink / raw)
  To: Roberto Sassu
  Cc: zohar, roberto.sassu, dmitry.kasatkin, eric.snowberg, paul,
	jmorris, serge, linux-integrity, linux-security-module,
	linux-kernel

On Wed, Feb 04, 2026 at 01:50:29PM +0100, Roberto Sassu wrote:
> On Sat, 2026-01-31 at 10:22 -0800, Daniel Hodges wrote:
> > 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);
> 
> Maybe we should also indicate if an error occurred, with a separate
> error message, or adding the result in the message below.
> 
> Thanks
> 
> Roberto

That makes sense, I'll send a V2. I'm having trouble with my corporate
email mail delivery so it might come from my personal email.

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

* [PATCH v2 v2] evm: check return values of crypto_shash functions
  2026-02-04 15:47   ` Daniel Hodges
@ 2026-02-06  2:42     ` Daniel Hodges
  2026-02-19  9:26       ` Roberto Sassu
  0 siblings, 1 reply; 9+ messages in thread
From: Daniel Hodges @ 2026-02-06  2:42 UTC (permalink / raw)
  To: Roberto Sassu
  Cc: zohar, roberto.sassu, 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 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>
---
 security/integrity/evm/evm_crypto.c | 55 ++++++++++++++++++++++-------
 1 file changed, 42 insertions(+), 13 deletions(-)

diff --git a/security/integrity/evm/evm_crypto.c b/security/integrity/evm/evm_crypto.c
index a5e730ffda57..402eb1ca64ce 100644
--- a/security/integrity/evm/evm_crypto.c
+++ b/security/integrity/evm/evm_crypto.c
@@ -139,7 +139,7 @@ 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,
+static int hmac_add_misc(struct shash_desc *desc, struct inode *inode,
 			  char type, char *digest)
 {
 	struct h_misc {
@@ -149,6 +149,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
@@ -169,14 +170,28 @@ 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;
 }
 
 /*
@@ -260,9 +275,12 @@ 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,
+			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;
 
@@ -286,15 +304,20 @@ 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))
@@ -401,6 +424,7 @@ int evm_init_hmac(struct inode *inode, const struct xattr *xattrs,
 {
 	struct shash_desc *desc;
 	const struct xattr *xattr;
+	int error;
 
 	desc = init_desc(EVM_XATTR_HMAC, HASH_ALGO_SHA1);
 	if (IS_ERR(desc)) {
@@ -412,12 +436,17 @@ int evm_init_hmac(struct inode *inode, const struct xattr *xattrs,
 		if (!evm_protected_xattr(xattr->name))
 			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.52.0


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

* Re: [PATCH v2 v2] evm: check return values of crypto_shash functions
  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-03-09 15:03         ` Mimi Zohar
  0 siblings, 2 replies; 9+ messages in thread
From: Roberto Sassu @ 2026-02-19  9:26 UTC (permalink / raw)
  To: Daniel Hodges
  Cc: zohar, roberto.sassu, dmitry.kasatkin, eric.snowberg, paul,
	jmorris, serge, linux-integrity, linux-security-module,
	linux-kernel

On Thu, 2026-02-05 at 21:42 -0500, Daniel Hodges wrote:
> 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>

After fixing the minor issue below:

Reviewed-by: Roberto Sassu <roberto.sassu@huawei.com>

> ---
>  security/integrity/evm/evm_crypto.c | 55 ++++++++++++++++++++++-------
>  1 file changed, 42 insertions(+), 13 deletions(-)
> 
> diff --git a/security/integrity/evm/evm_crypto.c b/security/integrity/evm/evm_crypto.c
> index a5e730ffda57..402eb1ca64ce 100644
> --- a/security/integrity/evm/evm_crypto.c
> +++ b/security/integrity/evm/evm_crypto.c
> @@ -139,7 +139,7 @@ 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,
> +static int hmac_add_misc(struct shash_desc *desc, struct inode *inode,
>  			  char type, char *digest)
>  {
>  	struct h_misc {
> @@ -149,6 +149,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
> @@ -169,14 +170,28 @@ 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;
>  }
>  
>  /*
> @@ -260,9 +275,12 @@ 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,
> +			error = crypto_shash_update(desc, (const u8 *)req_xattr_value,
>  					     req_xattr_value_len);

Please align this.

Thanks

Roberto

> +			if (error) {
> +				pr_err("crypto_shash_update() failed: %d\n", error);
> +				goto out;
> +			}
>  			if (is_ima)
>  				ima_present = true;
>  
> @@ -286,15 +304,20 @@ 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))
> @@ -401,6 +424,7 @@ int evm_init_hmac(struct inode *inode, const struct xattr *xattrs,
>  {
>  	struct shash_desc *desc;
>  	const struct xattr *xattr;
> +	int error;
>  
>  	desc = init_desc(EVM_XATTR_HMAC, HASH_ALGO_SHA1);
>  	if (IS_ERR(desc)) {
> @@ -412,12 +436,17 @@ int evm_init_hmac(struct inode *inode, const struct xattr *xattrs,
>  		if (!evm_protected_xattr(xattr->name))
>  			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;
>  }
>  
>  /*


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

* Re: [PATCH v2 v2] evm: check return values of crypto_shash functions
  2026-02-19  9:26       ` Roberto Sassu
@ 2026-02-19 12:36         ` Roberto Sassu
  2026-02-19 15:01           ` Daniel Hodges
  2026-03-09 15:03         ` Mimi Zohar
  1 sibling, 1 reply; 9+ messages in thread
From: Roberto Sassu @ 2026-02-19 12:36 UTC (permalink / raw)
  To: Daniel Hodges
  Cc: zohar, roberto.sassu, dmitry.kasatkin, eric.snowberg, paul,
	jmorris, serge, linux-integrity, linux-security-module,
	linux-kernel

On Thu, 2026-02-19 at 10:26 +0100, Roberto Sassu wrote:
> On Thu, 2026-02-05 at 21:42 -0500, Daniel Hodges wrote:
> > 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>
> 
> After fixing the minor issue below:

Already did it. The patch is here (after fixing a conflict with
0496fc9cdc38 "evm: Use ordered xattrs list to calculate HMAC in
evm_init_hmac()"):

https://github.com/robertosassu/linux/commit/d5aba42198b602c6de002ef02a4e6cc1d75652d7

Roberto

> Reviewed-by: Roberto Sassu <roberto.sassu@huawei.com>
> 
> > ---
> >  security/integrity/evm/evm_crypto.c | 55 ++++++++++++++++++++++-------
> >  1 file changed, 42 insertions(+), 13 deletions(-)
> > 
> > diff --git a/security/integrity/evm/evm_crypto.c b/security/integrity/evm/evm_crypto.c
> > index a5e730ffda57..402eb1ca64ce 100644
> > --- a/security/integrity/evm/evm_crypto.c
> > +++ b/security/integrity/evm/evm_crypto.c
> > @@ -139,7 +139,7 @@ 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,
> > +static int hmac_add_misc(struct shash_desc *desc, struct inode *inode,
> >  			  char type, char *digest)
> >  {
> >  	struct h_misc {
> > @@ -149,6 +149,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
> > @@ -169,14 +170,28 @@ 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;
> >  }
> >  
> >  /*
> > @@ -260,9 +275,12 @@ 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,
> > +			error = crypto_shash_update(desc, (const u8 *)req_xattr_value,
> >  					     req_xattr_value_len);
> 
> Please align this.
> 
> Thanks
> 
> Roberto
> 
> > +			if (error) {
> > +				pr_err("crypto_shash_update() failed: %d\n", error);
> > +				goto out;
> > +			}
> >  			if (is_ima)
> >  				ima_present = true;
> >  
> > @@ -286,15 +304,20 @@ 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))
> > @@ -401,6 +424,7 @@ int evm_init_hmac(struct inode *inode, const struct xattr *xattrs,
> >  {
> >  	struct shash_desc *desc;
> >  	const struct xattr *xattr;
> > +	int error;
> >  
> >  	desc = init_desc(EVM_XATTR_HMAC, HASH_ALGO_SHA1);
> >  	if (IS_ERR(desc)) {
> > @@ -412,12 +436,17 @@ int evm_init_hmac(struct inode *inode, const struct xattr *xattrs,
> >  		if (!evm_protected_xattr(xattr->name))
> >  			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;
> >  }
> >  
> >  /*


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

* Re: [PATCH v2 v2] evm: check return values of crypto_shash functions
  2026-02-19 12:36         ` Roberto Sassu
@ 2026-02-19 15:01           ` Daniel Hodges
  2026-02-20  9:06             ` Roberto Sassu
  0 siblings, 1 reply; 9+ messages in thread
From: Daniel Hodges @ 2026-02-19 15:01 UTC (permalink / raw)
  To: Roberto Sassu
  Cc: Daniel Hodges, zohar, roberto.sassu, dmitry.kasatkin,
	eric.snowberg, paul, jmorris, serge, linux-integrity,
	linux-security-module, linux-kernel

On Thu, Feb 19, 2026 at 01:36:39PM +0100, Roberto Sassu wrote:
> On Thu, 2026-02-19 at 10:26 +0100, Roberto Sassu wrote:
> > On Thu, 2026-02-05 at 21:42 -0500, Daniel Hodges wrote:
> > > 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>
> > 
> > After fixing the minor issue below:
> 
> Already did it. The patch is here (after fixing a conflict with
> 0496fc9cdc38 "evm: Use ordered xattrs list to calculate HMAC in
> evm_init_hmac()"):
> 
> https://github.com/robertosassu/linux/commit/d5aba42198b602c6de002ef02a4e6cc1d75652d7
> 
> Roberto

Nice, thanks for handling that!

-Daniel

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

* Re: [PATCH v2 v2] evm: check return values of crypto_shash functions
  2026-02-19 15:01           ` Daniel Hodges
@ 2026-02-20  9:06             ` Roberto Sassu
  0 siblings, 0 replies; 9+ messages in thread
From: Roberto Sassu @ 2026-02-20  9:06 UTC (permalink / raw)
  To: Daniel Hodges
  Cc: Daniel Hodges, zohar, roberto.sassu, dmitry.kasatkin,
	eric.snowberg, paul, jmorris, serge, linux-integrity,
	linux-security-module, linux-kernel

On Thu, 2026-02-19 at 10:01 -0500, Daniel Hodges wrote:
> On Thu, Feb 19, 2026 at 01:36:39PM +0100, Roberto Sassu wrote:
> > On Thu, 2026-02-19 at 10:26 +0100, Roberto Sassu wrote:
> > > On Thu, 2026-02-05 at 21:42 -0500, Daniel Hodges wrote:
> > > > 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>
> > > 
> > > After fixing the minor issue below:
> > 
> > Already did it. The patch is here (after fixing a conflict with
> > 0496fc9cdc38 "evm: Use ordered xattrs list to calculate HMAC in
> > evm_init_hmac()"):
> > 
> > https://github.com/robertosassu/linux/commit/d5aba42198b602c6de002ef02a4e6cc1d75652d7
> > 
> > Roberto
> 
> Nice, thanks for handling that!

Welcome!

Roberto


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

* Re: [PATCH v2 v2] evm: check return values of crypto_shash functions
  2026-02-19  9:26       ` Roberto Sassu
  2026-02-19 12:36         ` Roberto Sassu
@ 2026-03-09 15:03         ` Mimi Zohar
  1 sibling, 0 replies; 9+ messages in thread
From: Mimi Zohar @ 2026-03-09 15:03 UTC (permalink / raw)
  To: Roberto Sassu, Daniel Hodges
  Cc: roberto.sassu, dmitry.kasatkin, eric.snowberg, paul, jmorris,
	serge, linux-integrity, linux-security-module, linux-kernel

On Thu, 2026-02-19 at 10:26 +0100, Roberto Sassu wrote:
> On Thu, 2026-02-05 at 21:42 -0500, Daniel Hodges wrote:
> > 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>
> 
> After fixing the minor issue below:
> 
> Reviewed-by: Roberto Sassu <roberto.sassu@huawei.com>

Thanks Daniel, Roberto.  Daniel there are a couple of places where the line
length is greater than 80.  To see them, add "--max-line-length=80" to
scripts/checkpatch.pl.  I'd appreciate your fixing them.  Otherwise, the patch
looks good.

Thanks, 

Mimi

^ permalink raw reply	[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