linux-kernel - Re: [PATCH] evm: check return values of crypto

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>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <aYNprpzxppKE0Gf2@fb.com>
Date: Wed, 4 Feb 2026 07:47:04 -0800
From: Daniel Hodges <hodgesd@...a.com>
To: Roberto Sassu <roberto.sassu@...weicloud.com>
CC: <zohar@...ux.ibm.com>, <roberto.sassu@...wei.com>,
        <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>
Subject: Re: [PATCH] evm: check return values of crypto_shash functions

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@...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);
> 
> 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.