Re: [PATCH] evm: check return values of crypto_shash functions

Roberto Sassu Mon, 10 Aug 2026 18:58:12 -0700

On 8/7/2026 7:42 PM, Roberto Sassu wrote:

On Tue, 2026-08-04 at 16:07 -0400, Mimi Zohar wrote:

From: Daniel Hodges <[email protected]>


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 <[email protected]>
Reviewed-by: Roberto Sassu <[email protected]>


Looks good to me. I confirm the Reviewed-by.

We got a report from Sashiko. As it pointed out, we should not overridethe error when calling hmac_add_misc() if no xattr is present

(-ENODATA).

Roberto

Thanks

Roberto

Link: https://github.com/linux-integrity/linux/issues/17
[[email protected]: fixed commit 0496fc9cdc38 merge conflict,
checkpatch.pl warnings]
Signed-off-by: Mimi Zohar <[email protected]>
---
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

Re: [PATCH] evm: check return values of crypto_shash functions

Reply via email to