linux-hardening - [PATCH v2] evm: Refector struct evm

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

Date:   Wed, 18 May 2022 14:32:32 +0800
From:   GUO Zihua <guozihua@...wei.com>
To:     <linux-integrity@...r.kernel.org>
CC:     <zohar@...ux.ibm.com>, <dmitry.kasatkin@...il.com>,
        <gustavoars@...nel.org>, <linux-hardening@...r.kernel.org>
Subject: [PATCH v2] evm: Refector struct evm_xattr

struct evm_xattr is only used for EVM_XATTR_HMAC type evm digest and
glues together one flexible array and one fixed length array. The
original intention might be shortening the code as the "data" field
would always be a SHA1 digest.

This implementation is not complying with GCC's specification about
flexible array which requires that flexible should be the last member of
a structure and structure of flexible array should not be a sub
structure.

Fix it by:
1. Remove struct evm_xattr and use struct evm_ima_xattr_data directly.
2. Get array size with struct_size instead of sizeof as suggested by
Linus.

Reference: https://lore.kernel.org/all/CAHk-=wiGWjxs7EVUpccZEi6esvjpHJdgHQ=vtUeJ5crL62hx9A@mail.gmail.com/

Fixes: 6be5cc5246f80 ("evm: add support for different security.evm data types")
Signed-off-by: GUO Zihua <guozihua@...wei.com>

---

v2:
    Change the patch subject to PATCH instead of PATCH -next. Update
    commit message based on feedback from Gustavo on another patch.

---
 security/integrity/evm/evm_main.c | 14 ++++++++------
 security/integrity/integrity.h    |  6 ------
 2 files changed, 8 insertions(+), 12 deletions(-)

diff --git a/security/integrity/evm/evm_main.c b/security/integrity/evm/evm_main.c
index 7d87772f0ce6..f2c4501a287a 100644
--- a/security/integrity/evm/evm_main.c
+++ b/security/integrity/evm/evm_main.c
@@ -211,7 +211,8 @@ static enum integrity_status evm_verify_hmac(struct dentry *dentry,
 	/* check value type */
 	switch (xattr_data->type) {
 	case EVM_XATTR_HMAC:
-		if (xattr_len != sizeof(struct evm_xattr)) {
+		if (xattr_len != struct_size(*xattr_data, data,
+					     SHA1_DIGEST_SIZE)) {
 			evm_status = INTEGRITY_FAIL;
 			goto out;
 		}
@@ -842,24 +843,25 @@ int evm_inode_init_security(struct inode *inode,
 				 const struct xattr *lsm_xattr,
 				 struct xattr *evm_xattr)
 {
-	struct evm_xattr *xattr_data;
+	struct evm_ima_xattr_data *xattr_data;
 	int rc;
 
 	if (!(evm_initialized & EVM_INIT_HMAC) ||
 	    !evm_protected_xattr(lsm_xattr->name))
 		return 0;
 
-	xattr_data = kzalloc(sizeof(*xattr_data), GFP_NOFS);
+	xattr_data = kzalloc(struct_size(*xattr_data, data,
+			     SHA1_DIGEST_SIZE), GFP_NOFS);
 	if (!xattr_data)
 		return -ENOMEM;
 
-	xattr_data->data.type = EVM_XATTR_HMAC;
-	rc = evm_init_hmac(inode, lsm_xattr, xattr_data->digest);
+	xattr_data->type = EVM_XATTR_HMAC;
+	rc = evm_init_hmac(inode, lsm_xattr, xattr_data->data);
 	if (rc < 0)
 		goto out;
 
 	evm_xattr->value = xattr_data;
-	evm_xattr->value_len = sizeof(*xattr_data);
+	evm_xattr->value_len = struct_size(*xattr_data, data, SHA1_DIGEST_SIZE);
 	evm_xattr->name = XATTR_EVM_SUFFIX;
 	return 0;
 out:
diff --git a/security/integrity/integrity.h b/security/integrity/integrity.h
index 3510e413ea17..91b16d620dd9 100644
--- a/security/integrity/integrity.h
+++ b/security/integrity/integrity.h
@@ -86,12 +86,6 @@ struct evm_ima_xattr_data {
 	u8 data[];
 } __packed;
 
-/* Only used in the EVM HMAC code. */
-struct evm_xattr {
-	struct evm_ima_xattr_data data;
-	u8 digest[SHA1_DIGEST_SIZE];
-} __packed;
-
 #define IMA_MAX_DIGEST_SIZE	64
 
 struct ima_digest_data {
-- 
2.36.0