From: Daniel Hodges <hodgesd@meta.com>
To: Roberto Sassu <roberto.sassu@huaweicloud.com>
Cc: <zohar@linux.ibm.com>, <roberto.sassu@huawei.com>,
<dmitry.kasatkin@gmail.com>, <eric.snowberg@oracle.com>,
<paul@paul-moore.com>, <jmorris@namei.org>, <serge@hallyn.com>,
<linux-integrity@vger.kernel.org>,
<linux-security-module@vger.kernel.org>,
<linux-kernel@vger.kernel.org>
Subject: Re: [PATCH] evm: check return values of crypto_shash functions
Date: Wed, 4 Feb 2026 07:47:04 -0800 [thread overview]
Message-ID: <aYNprpzxppKE0Gf2@fb.com> (raw)
In-Reply-To: <d1d63c522a3842ccaf74c4462ee06bf82ce3b3f5.camel@huaweicloud.com>
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.
next prev parent reply other threads:[~2026-02-04 15:50 UTC|newest]
Thread overview: 9+ messages / expand[flat|nested] mbox.gz Atom feed top
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 [this message]
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
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=aYNprpzxppKE0Gf2@fb.com \
--to=hodgesd@meta.com \
--cc=dmitry.kasatkin@gmail.com \
--cc=eric.snowberg@oracle.com \
--cc=jmorris@namei.org \
--cc=linux-integrity@vger.kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-security-module@vger.kernel.org \
--cc=paul@paul-moore.com \
--cc=roberto.sassu@huawei.com \
--cc=roberto.sassu@huaweicloud.com \
--cc=serge@hallyn.com \
--cc=zohar@linux.ibm.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.