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