All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH][next] integrity: Avoid -Wflex-array-member-not-at-end warnings
@ 2024-03-04 17:52 Gustavo A. R. Silva
  2024-03-19 18:44 ` Gustavo A. R. Silva
  2024-03-21  1:19 ` Mimi Zohar
  0 siblings, 2 replies; 4+ messages in thread
From: Gustavo A. R. Silva @ 2024-03-04 17:52 UTC (permalink / raw)
  To: Mimi Zohar, Roberto Sassu, Dmitry Kasatkin, Eric Snowberg,
	Paul Moore, James Morris, Serge E. Hallyn
  Cc: linux-integrity, linux-security-module, linux-kernel,
	Gustavo A. R. Silva, linux-hardening, Kees Cook

-Wflex-array-member-not-at-end is coming in GCC-14, and we are getting
ready to enable it globally.

There is currently an object (`hdr)` in `struct ima_max_digest_data`
that contains a flexible structure (`struct ima_digest_data`):

 struct ima_max_digest_data {
	struct ima_digest_data hdr;
        u8 digest[HASH_MAX_DIGESTSIZE];
 } __packed;

So, in order to avoid ending up with a flexible-array member in the
middle of another struct, we use the `struct_group_tagged()` helper to
separate the flexible array from the rest of the members in the flexible
structure:

struct ima_digest_data {
        struct_group_tagged(ima_digest_data_hdr, hdr,

	... the rest of the members

        );
        u8 digest[];
} __packed;

With the change described above, we can now declare an object of the
type of the tagged struct, without embedding the flexible array in the
middle of another struct:

 struct ima_max_digest_data {
        struct ima_digest_data_hdr hdr;
        u8 digest[HASH_MAX_DIGESTSIZE];
 } __packed;

We also use `container_of()` whenever we need to retrieve a pointer to
the flexible structure.

So, with these changes, fix the following warnings:

security/integrity/evm/evm.h:45:32: warning: structure containing a flexible array member is not at the end of another structure [-Wflex-array-member-not-at-end]
security/integrity/evm/evm.h:45:32: warning: structure containing a flexible array member is not at the end of another structure [-Wflex-array-member-not-at-end]
security/integrity/evm/evm.h:45:32: warning: structure containing a flexible array member is not at the end of another structure [-Wflex-array-member-not-at-end]

Signed-off-by: Gustavo A. R. Silva <gustavoars@kernel.org>
---
 security/integrity/ima/ima_api.c          |  6 ++++--
 security/integrity/ima/ima_appraise.c     |  4 +++-
 security/integrity/ima/ima_init.c         |  6 ++++--
 security/integrity/ima/ima_main.c         |  6 ++++--
 security/integrity/ima/ima_template_lib.c | 10 ++++++----
 security/integrity/integrity.h            |  4 +++-
 6 files changed, 24 insertions(+), 12 deletions(-)

diff --git a/security/integrity/ima/ima_api.c b/security/integrity/ima/ima_api.c
index b37d043d5748..c7c8d1bffb17 100644
--- a/security/integrity/ima/ima_api.c
+++ b/security/integrity/ima/ima_api.c
@@ -247,6 +247,8 @@ int ima_collect_measurement(struct ima_iint_cache *iint, struct file *file,
 	struct inode *real_inode = d_real_inode(file_dentry(file));
 	const char *filename = file->f_path.dentry->d_name.name;
 	struct ima_max_digest_data hash;
+	struct ima_digest_data *hash_hdr = container_of(&hash.hdr,
+						struct ima_digest_data, hdr);
 	struct kstat stat;
 	int result = 0;
 	int length;
@@ -286,9 +288,9 @@ int ima_collect_measurement(struct ima_iint_cache *iint, struct file *file,
 			result = -ENODATA;
 		}
 	} else if (buf) {
-		result = ima_calc_buffer_hash(buf, size, &hash.hdr);
+		result = ima_calc_buffer_hash(buf, size, hash_hdr);
 	} else {
-		result = ima_calc_file_hash(file, &hash.hdr);
+		result = ima_calc_file_hash(file, hash_hdr);
 	}
 
 	if (result && result != -EBADF && result != -EINVAL)
diff --git a/security/integrity/ima/ima_appraise.c b/security/integrity/ima/ima_appraise.c
index 3497741caea9..656c709b974f 100644
--- a/security/integrity/ima/ima_appraise.c
+++ b/security/integrity/ima/ima_appraise.c
@@ -378,7 +378,9 @@ static int xattr_verify(enum ima_hooks func, struct ima_iint_cache *iint,
 		}
 
 		rc = calc_file_id_hash(IMA_VERITY_DIGSIG, iint->ima_hash->algo,
-				       iint->ima_hash->digest, &hash.hdr);
+				       iint->ima_hash->digest,
+				       container_of(&hash.hdr,
+					       struct ima_digest_data, hdr));
 		if (rc) {
 			*cause = "sigv3-hashing-error";
 			*status = INTEGRITY_FAIL;
diff --git a/security/integrity/ima/ima_init.c b/security/integrity/ima/ima_init.c
index 393f5c7912d5..4e208239a40e 100644
--- a/security/integrity/ima/ima_init.c
+++ b/security/integrity/ima/ima_init.c
@@ -48,12 +48,14 @@ static int __init ima_add_boot_aggregate(void)
 	struct ima_event_data event_data = { .iint = iint,
 					     .filename = boot_aggregate_name };
 	struct ima_max_digest_data hash;
+	struct ima_digest_data *hash_hdr = container_of(&hash.hdr,
+						struct ima_digest_data, hdr);
 	int result = -ENOMEM;
 	int violation = 0;
 
 	memset(iint, 0, sizeof(*iint));
 	memset(&hash, 0, sizeof(hash));
-	iint->ima_hash = &hash.hdr;
+	iint->ima_hash = hash_hdr;
 	iint->ima_hash->algo = ima_hash_algo;
 	iint->ima_hash->length = hash_digest_size[ima_hash_algo];
 
@@ -70,7 +72,7 @@ static int __init ima_add_boot_aggregate(void)
 	 * is not found.
 	 */
 	if (ima_tpm_chip) {
-		result = ima_calc_boot_aggregate(&hash.hdr);
+		result = ima_calc_boot_aggregate(hash_hdr);
 		if (result < 0) {
 			audit_cause = "hashing_error";
 			goto err_out;
diff --git a/security/integrity/ima/ima_main.c b/security/integrity/ima/ima_main.c
index c84e8c55333d..0d3a7c864fd4 100644
--- a/security/integrity/ima/ima_main.c
+++ b/security/integrity/ima/ima_main.c
@@ -941,6 +941,8 @@ int process_buffer_measurement(struct mnt_idmap *idmap,
 					    .buf_len = size};
 	struct ima_template_desc *template;
 	struct ima_max_digest_data hash;
+	struct ima_digest_data *hash_hdr = container_of(&hash.hdr,
+						struct ima_digest_data, hdr);
 	char digest_hash[IMA_MAX_DIGEST_SIZE];
 	int digest_hash_len = hash_digest_size[ima_hash_algo];
 	int violation = 0;
@@ -979,7 +981,7 @@ int process_buffer_measurement(struct mnt_idmap *idmap,
 	if (!pcr)
 		pcr = CONFIG_IMA_MEASURE_PCR_IDX;
 
-	iint.ima_hash = &hash.hdr;
+	iint.ima_hash = hash_hdr;
 	iint.ima_hash->algo = ima_hash_algo;
 	iint.ima_hash->length = hash_digest_size[ima_hash_algo];
 
@@ -990,7 +992,7 @@ int process_buffer_measurement(struct mnt_idmap *idmap,
 	}
 
 	if (buf_hash) {
-		memcpy(digest_hash, hash.hdr.digest, digest_hash_len);
+		memcpy(digest_hash, hash_hdr->digest, digest_hash_len);
 
 		ret = ima_calc_buffer_hash(digest_hash, digest_hash_len,
 					   iint.ima_hash);
diff --git a/security/integrity/ima/ima_template_lib.c b/security/integrity/ima/ima_template_lib.c
index 6cd0add524cd..74198d7619da 100644
--- a/security/integrity/ima/ima_template_lib.c
+++ b/security/integrity/ima/ima_template_lib.c
@@ -339,6 +339,8 @@ int ima_eventdigest_init(struct ima_event_data *event_data,
 			 struct ima_field_data *field_data)
 {
 	struct ima_max_digest_data hash;
+	struct ima_digest_data *hash_hdr = container_of(&hash.hdr,
+						struct ima_digest_data, hdr);
 	u8 *cur_digest = NULL;
 	u32 cur_digestsize = 0;
 	struct inode *inode;
@@ -358,7 +360,7 @@ int ima_eventdigest_init(struct ima_event_data *event_data,
 	if ((const char *)event_data->filename == boot_aggregate_name) {
 		if (ima_tpm_chip) {
 			hash.hdr.algo = HASH_ALGO_SHA1;
-			result = ima_calc_boot_aggregate(&hash.hdr);
+			result = ima_calc_boot_aggregate(hash_hdr);
 
 			/* algo can change depending on available PCR banks */
 			if (!result && hash.hdr.algo != HASH_ALGO_SHA1)
@@ -368,7 +370,7 @@ int ima_eventdigest_init(struct ima_event_data *event_data,
 				memset(&hash, 0, sizeof(hash));
 		}
 
-		cur_digest = hash.hdr.digest;
+		cur_digest = hash_hdr->digest;
 		cur_digestsize = hash_digest_size[HASH_ALGO_SHA1];
 		goto out;
 	}
@@ -379,14 +381,14 @@ int ima_eventdigest_init(struct ima_event_data *event_data,
 	inode = file_inode(event_data->file);
 	hash.hdr.algo = ima_template_hash_algo_allowed(ima_hash_algo) ?
 	    ima_hash_algo : HASH_ALGO_SHA1;
-	result = ima_calc_file_hash(event_data->file, &hash.hdr);
+	result = ima_calc_file_hash(event_data->file, hash_hdr);
 	if (result) {
 		integrity_audit_msg(AUDIT_INTEGRITY_DATA, inode,
 				    event_data->filename, "collect_data",
 				    "failed", result, 0);
 		return result;
 	}
-	cur_digest = hash.hdr.digest;
+	cur_digest = hash_hdr->digest;
 	cur_digestsize = hash.hdr.length;
 out:
 	return ima_eventdigest_init_common(cur_digest, cur_digestsize,
diff --git a/security/integrity/integrity.h b/security/integrity/integrity.h
index 50d6f798e613..fc1952da02ea 100644
--- a/security/integrity/integrity.h
+++ b/security/integrity/integrity.h
@@ -44,6 +44,7 @@ struct evm_xattr {
 #define IMA_MAX_DIGEST_SIZE	HASH_MAX_DIGESTSIZE
 
 struct ima_digest_data {
+	struct_group_tagged(ima_digest_data_hdr, hdr,
 	u8 algo;
 	u8 length;
 	union {
@@ -57,6 +58,7 @@ struct ima_digest_data {
 		} ng;
 		u8 data[2];
 	} xattr;
+	);
 	u8 digest[];
 } __packed;
 
@@ -65,7 +67,7 @@ struct ima_digest_data {
  * with the maximum hash size, define ima_max_digest_data struct.
  */
 struct ima_max_digest_data {
-	struct ima_digest_data hdr;
+	struct ima_digest_data_hdr hdr;
 	u8 digest[HASH_MAX_DIGESTSIZE];
 } __packed;
 
-- 
2.34.1


^ permalink raw reply related	[flat|nested] 4+ messages in thread

* Re: [PATCH][next] integrity: Avoid -Wflex-array-member-not-at-end warnings
  2024-03-04 17:52 [PATCH][next] integrity: Avoid -Wflex-array-member-not-at-end warnings Gustavo A. R. Silva
@ 2024-03-19 18:44 ` Gustavo A. R. Silva
  2024-03-21  1:19 ` Mimi Zohar
  1 sibling, 0 replies; 4+ messages in thread
From: Gustavo A. R. Silva @ 2024-03-19 18:44 UTC (permalink / raw)
  To: Gustavo A. R. Silva, Mimi Zohar, Roberto Sassu, Dmitry Kasatkin,
	Eric Snowberg, Paul Moore, James Morris, Serge E. Hallyn
  Cc: linux-integrity, linux-security-module, linux-kernel,
	linux-hardening, Kees Cook

Hi all,

Friendly ping: who can take this, please? :)

Thanks!
--
Gustavo

On 3/4/24 11:52, Gustavo A. R. Silva wrote:
> -Wflex-array-member-not-at-end is coming in GCC-14, and we are getting
> ready to enable it globally.
> 
> There is currently an object (`hdr)` in `struct ima_max_digest_data`
> that contains a flexible structure (`struct ima_digest_data`):
> 
>   struct ima_max_digest_data {
> 	struct ima_digest_data hdr;
>          u8 digest[HASH_MAX_DIGESTSIZE];
>   } __packed;
> 
> So, in order to avoid ending up with a flexible-array member in the
> middle of another struct, we use the `struct_group_tagged()` helper to
> separate the flexible array from the rest of the members in the flexible
> structure:
> 
> struct ima_digest_data {
>          struct_group_tagged(ima_digest_data_hdr, hdr,
> 
> 	... the rest of the members
> 
>          );
>          u8 digest[];
> } __packed;
> 
> With the change described above, we can now declare an object of the
> type of the tagged struct, without embedding the flexible array in the
> middle of another struct:
> 
>   struct ima_max_digest_data {
>          struct ima_digest_data_hdr hdr;
>          u8 digest[HASH_MAX_DIGESTSIZE];
>   } __packed;
> 
> We also use `container_of()` whenever we need to retrieve a pointer to
> the flexible structure.
> 
> So, with these changes, fix the following warnings:
> 
> security/integrity/evm/evm.h:45:32: warning: structure containing a flexible array member is not at the end of another structure [-Wflex-array-member-not-at-end]
> security/integrity/evm/evm.h:45:32: warning: structure containing a flexible array member is not at the end of another structure [-Wflex-array-member-not-at-end]
> security/integrity/evm/evm.h:45:32: warning: structure containing a flexible array member is not at the end of another structure [-Wflex-array-member-not-at-end]
> 
> Signed-off-by: Gustavo A. R. Silva <gustavoars@kernel.org>
> ---
>   security/integrity/ima/ima_api.c          |  6 ++++--
>   security/integrity/ima/ima_appraise.c     |  4 +++-
>   security/integrity/ima/ima_init.c         |  6 ++++--
>   security/integrity/ima/ima_main.c         |  6 ++++--
>   security/integrity/ima/ima_template_lib.c | 10 ++++++----
>   security/integrity/integrity.h            |  4 +++-
>   6 files changed, 24 insertions(+), 12 deletions(-)
> 
> diff --git a/security/integrity/ima/ima_api.c b/security/integrity/ima/ima_api.c
> index b37d043d5748..c7c8d1bffb17 100644
> --- a/security/integrity/ima/ima_api.c
> +++ b/security/integrity/ima/ima_api.c
> @@ -247,6 +247,8 @@ int ima_collect_measurement(struct ima_iint_cache *iint, struct file *file,
>   	struct inode *real_inode = d_real_inode(file_dentry(file));
>   	const char *filename = file->f_path.dentry->d_name.name;
>   	struct ima_max_digest_data hash;
> +	struct ima_digest_data *hash_hdr = container_of(&hash.hdr,
> +						struct ima_digest_data, hdr);
>   	struct kstat stat;
>   	int result = 0;
>   	int length;
> @@ -286,9 +288,9 @@ int ima_collect_measurement(struct ima_iint_cache *iint, struct file *file,
>   			result = -ENODATA;
>   		}
>   	} else if (buf) {
> -		result = ima_calc_buffer_hash(buf, size, &hash.hdr);
> +		result = ima_calc_buffer_hash(buf, size, hash_hdr);
>   	} else {
> -		result = ima_calc_file_hash(file, &hash.hdr);
> +		result = ima_calc_file_hash(file, hash_hdr);
>   	}
>   
>   	if (result && result != -EBADF && result != -EINVAL)
> diff --git a/security/integrity/ima/ima_appraise.c b/security/integrity/ima/ima_appraise.c
> index 3497741caea9..656c709b974f 100644
> --- a/security/integrity/ima/ima_appraise.c
> +++ b/security/integrity/ima/ima_appraise.c
> @@ -378,7 +378,9 @@ static int xattr_verify(enum ima_hooks func, struct ima_iint_cache *iint,
>   		}
>   
>   		rc = calc_file_id_hash(IMA_VERITY_DIGSIG, iint->ima_hash->algo,
> -				       iint->ima_hash->digest, &hash.hdr);
> +				       iint->ima_hash->digest,
> +				       container_of(&hash.hdr,
> +					       struct ima_digest_data, hdr));
>   		if (rc) {
>   			*cause = "sigv3-hashing-error";
>   			*status = INTEGRITY_FAIL;
> diff --git a/security/integrity/ima/ima_init.c b/security/integrity/ima/ima_init.c
> index 393f5c7912d5..4e208239a40e 100644
> --- a/security/integrity/ima/ima_init.c
> +++ b/security/integrity/ima/ima_init.c
> @@ -48,12 +48,14 @@ static int __init ima_add_boot_aggregate(void)
>   	struct ima_event_data event_data = { .iint = iint,
>   					     .filename = boot_aggregate_name };
>   	struct ima_max_digest_data hash;
> +	struct ima_digest_data *hash_hdr = container_of(&hash.hdr,
> +						struct ima_digest_data, hdr);
>   	int result = -ENOMEM;
>   	int violation = 0;
>   
>   	memset(iint, 0, sizeof(*iint));
>   	memset(&hash, 0, sizeof(hash));
> -	iint->ima_hash = &hash.hdr;
> +	iint->ima_hash = hash_hdr;
>   	iint->ima_hash->algo = ima_hash_algo;
>   	iint->ima_hash->length = hash_digest_size[ima_hash_algo];
>   
> @@ -70,7 +72,7 @@ static int __init ima_add_boot_aggregate(void)
>   	 * is not found.
>   	 */
>   	if (ima_tpm_chip) {
> -		result = ima_calc_boot_aggregate(&hash.hdr);
> +		result = ima_calc_boot_aggregate(hash_hdr);
>   		if (result < 0) {
>   			audit_cause = "hashing_error";
>   			goto err_out;
> diff --git a/security/integrity/ima/ima_main.c b/security/integrity/ima/ima_main.c
> index c84e8c55333d..0d3a7c864fd4 100644
> --- a/security/integrity/ima/ima_main.c
> +++ b/security/integrity/ima/ima_main.c
> @@ -941,6 +941,8 @@ int process_buffer_measurement(struct mnt_idmap *idmap,
>   					    .buf_len = size};
>   	struct ima_template_desc *template;
>   	struct ima_max_digest_data hash;
> +	struct ima_digest_data *hash_hdr = container_of(&hash.hdr,
> +						struct ima_digest_data, hdr);
>   	char digest_hash[IMA_MAX_DIGEST_SIZE];
>   	int digest_hash_len = hash_digest_size[ima_hash_algo];
>   	int violation = 0;
> @@ -979,7 +981,7 @@ int process_buffer_measurement(struct mnt_idmap *idmap,
>   	if (!pcr)
>   		pcr = CONFIG_IMA_MEASURE_PCR_IDX;
>   
> -	iint.ima_hash = &hash.hdr;
> +	iint.ima_hash = hash_hdr;
>   	iint.ima_hash->algo = ima_hash_algo;
>   	iint.ima_hash->length = hash_digest_size[ima_hash_algo];
>   
> @@ -990,7 +992,7 @@ int process_buffer_measurement(struct mnt_idmap *idmap,
>   	}
>   
>   	if (buf_hash) {
> -		memcpy(digest_hash, hash.hdr.digest, digest_hash_len);
> +		memcpy(digest_hash, hash_hdr->digest, digest_hash_len);
>   
>   		ret = ima_calc_buffer_hash(digest_hash, digest_hash_len,
>   					   iint.ima_hash);
> diff --git a/security/integrity/ima/ima_template_lib.c b/security/integrity/ima/ima_template_lib.c
> index 6cd0add524cd..74198d7619da 100644
> --- a/security/integrity/ima/ima_template_lib.c
> +++ b/security/integrity/ima/ima_template_lib.c
> @@ -339,6 +339,8 @@ int ima_eventdigest_init(struct ima_event_data *event_data,
>   			 struct ima_field_data *field_data)
>   {
>   	struct ima_max_digest_data hash;
> +	struct ima_digest_data *hash_hdr = container_of(&hash.hdr,
> +						struct ima_digest_data, hdr);
>   	u8 *cur_digest = NULL;
>   	u32 cur_digestsize = 0;
>   	struct inode *inode;
> @@ -358,7 +360,7 @@ int ima_eventdigest_init(struct ima_event_data *event_data,
>   	if ((const char *)event_data->filename == boot_aggregate_name) {
>   		if (ima_tpm_chip) {
>   			hash.hdr.algo = HASH_ALGO_SHA1;
> -			result = ima_calc_boot_aggregate(&hash.hdr);
> +			result = ima_calc_boot_aggregate(hash_hdr);
>   
>   			/* algo can change depending on available PCR banks */
>   			if (!result && hash.hdr.algo != HASH_ALGO_SHA1)
> @@ -368,7 +370,7 @@ int ima_eventdigest_init(struct ima_event_data *event_data,
>   				memset(&hash, 0, sizeof(hash));
>   		}
>   
> -		cur_digest = hash.hdr.digest;
> +		cur_digest = hash_hdr->digest;
>   		cur_digestsize = hash_digest_size[HASH_ALGO_SHA1];
>   		goto out;
>   	}
> @@ -379,14 +381,14 @@ int ima_eventdigest_init(struct ima_event_data *event_data,
>   	inode = file_inode(event_data->file);
>   	hash.hdr.algo = ima_template_hash_algo_allowed(ima_hash_algo) ?
>   	    ima_hash_algo : HASH_ALGO_SHA1;
> -	result = ima_calc_file_hash(event_data->file, &hash.hdr);
> +	result = ima_calc_file_hash(event_data->file, hash_hdr);
>   	if (result) {
>   		integrity_audit_msg(AUDIT_INTEGRITY_DATA, inode,
>   				    event_data->filename, "collect_data",
>   				    "failed", result, 0);
>   		return result;
>   	}
> -	cur_digest = hash.hdr.digest;
> +	cur_digest = hash_hdr->digest;
>   	cur_digestsize = hash.hdr.length;
>   out:
>   	return ima_eventdigest_init_common(cur_digest, cur_digestsize,
> diff --git a/security/integrity/integrity.h b/security/integrity/integrity.h
> index 50d6f798e613..fc1952da02ea 100644
> --- a/security/integrity/integrity.h
> +++ b/security/integrity/integrity.h
> @@ -44,6 +44,7 @@ struct evm_xattr {
>   #define IMA_MAX_DIGEST_SIZE	HASH_MAX_DIGESTSIZE
>   
>   struct ima_digest_data {
> +	struct_group_tagged(ima_digest_data_hdr, hdr,
>   	u8 algo;
>   	u8 length;
>   	union {
> @@ -57,6 +58,7 @@ struct ima_digest_data {
>   		} ng;
>   		u8 data[2];
>   	} xattr;
> +	);
>   	u8 digest[];
>   } __packed;
>   
> @@ -65,7 +67,7 @@ struct ima_digest_data {
>    * with the maximum hash size, define ima_max_digest_data struct.
>    */
>   struct ima_max_digest_data {
> -	struct ima_digest_data hdr;
> +	struct ima_digest_data_hdr hdr;
>   	u8 digest[HASH_MAX_DIGESTSIZE];
>   } __packed;
>   

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH][next] integrity: Avoid -Wflex-array-member-not-at-end warnings
  2024-03-04 17:52 [PATCH][next] integrity: Avoid -Wflex-array-member-not-at-end warnings Gustavo A. R. Silva
  2024-03-19 18:44 ` Gustavo A. R. Silva
@ 2024-03-21  1:19 ` Mimi Zohar
  2024-03-21  3:39   ` Gustavo A. R. Silva
  1 sibling, 1 reply; 4+ messages in thread
From: Mimi Zohar @ 2024-03-21  1:19 UTC (permalink / raw)
  To: Gustavo A. R. Silva, Roberto Sassu, Dmitry Kasatkin,
	Eric Snowberg, Paul Moore, James Morris, Serge E. Hallyn
  Cc: linux-integrity, linux-security-module, linux-kernel,
	linux-hardening, Kees Cook

Hi Gustavo,

Sorry for the delay...

On Mon, 2024-03-04 at 11:52 -0600, Gustavo A. R. Silva wrote:
> -Wflex-array-member-not-at-end is coming in GCC-14, and we are getting
> ready to enable it globally.
> 
> There is currently an object (`hdr)` in `struct ima_max_digest_data`
> that contains a flexible structure (`struct ima_digest_data`):
> 
>  struct ima_max_digest_data {
> 	struct ima_digest_data hdr;
>         u8 digest[HASH_MAX_DIGESTSIZE];
>  } __packed;
> 
> So, in order to avoid ending up with a flexible-array member in the
> middle of another struct, we use the `struct_group_tagged()` helper to
> separate the flexible array from the rest of the members in the flexible
> structure:
> 
> struct ima_digest_data {
>         struct_group_tagged(ima_digest_data_hdr, hdr,
> 
> 	... the rest of the members
> 
>         );
>         u8 digest[];
> } __packed;
> 
> With the change described above, we can now declare an object of the
> type of the tagged struct, without embedding the flexible array in the
> middle of another struct:
> 
>  struct ima_max_digest_data {
>         struct ima_digest_data_hdr hdr;
>         u8 digest[HASH_MAX_DIGESTSIZE];
>  } __packed;
> 
> We also use `container_of()` whenever we need to retrieve a pointer to
> the flexible structure.

Nice!

> 
> So, with these changes, fix the following warnings:
> 
> security/integrity/evm/evm.h:45:32: warning: structure containing a flexible
> array member is not at the end of another structure [-Wflex-array-member-not-
> at-end]
> security/integrity/evm/evm.h:45:32: warning: structure containing a flexible
> array member is not at the end of another structure [-Wflex-array-member-not-
> at-end]
> security/integrity/evm/evm.h:45:32: warning: structure containing a flexible
> array member is not at the end of another structure [-Wflex-array-member-not-
> at-end]

A similar change would need to be made to struct evm_digest:

struct evm_digest {
        struct ima_digest_data hdr;
        char digest[IMA_MAX_DIGEST_SIZE];
} __packed;

Is there are another patch?

Mimi

> 
> Signed-off-by: Gustavo A. R. Silva <gustavoars@kernel.org>
> 


^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH][next] integrity: Avoid -Wflex-array-member-not-at-end warnings
  2024-03-21  1:19 ` Mimi Zohar
@ 2024-03-21  3:39   ` Gustavo A. R. Silva
  0 siblings, 0 replies; 4+ messages in thread
From: Gustavo A. R. Silva @ 2024-03-21  3:39 UTC (permalink / raw)
  To: Mimi Zohar, Gustavo A. R. Silva, Roberto Sassu, Dmitry Kasatkin,
	Eric Snowberg, Paul Moore, James Morris, Serge E. Hallyn
  Cc: linux-integrity, linux-security-module, linux-kernel,
	linux-hardening, Kees Cook



On 20/03/24 19:19, Mimi Zohar wrote:
> Hi Gustavo,
> 
> Sorry for the delay...

No worries. :)

> 
> On Mon, 2024-03-04 at 11:52 -0600, Gustavo A. R. Silva wrote:
>> -Wflex-array-member-not-at-end is coming in GCC-14, and we are getting
>> ready to enable it globally.
>>
>> There is currently an object (`hdr)` in `struct ima_max_digest_data`
>> that contains a flexible structure (`struct ima_digest_data`):
>>
>>   struct ima_max_digest_data {
>> 	struct ima_digest_data hdr;
>>          u8 digest[HASH_MAX_DIGESTSIZE];
>>   } __packed;
>>
>> So, in order to avoid ending up with a flexible-array member in the
>> middle of another struct, we use the `struct_group_tagged()` helper to
>> separate the flexible array from the rest of the members in the flexible
>> structure:
>>
>> struct ima_digest_data {
>>          struct_group_tagged(ima_digest_data_hdr, hdr,
>>
>> 	... the rest of the members
>>
>>          );
>>          u8 digest[];
>> } __packed;
>>
>> With the change described above, we can now declare an object of the
>> type of the tagged struct, without embedding the flexible array in the
>> middle of another struct:
>>
>>   struct ima_max_digest_data {
>>          struct ima_digest_data_hdr hdr;
>>          u8 digest[HASH_MAX_DIGESTSIZE];
>>   } __packed;
>>
>> We also use `container_of()` whenever we need to retrieve a pointer to
>> the flexible structure.
> 
> Nice!
> 
>>
>> So, with these changes, fix the following warnings:
>>
>> security/integrity/evm/evm.h:45:32: warning: structure containing a flexible
>> array member is not at the end of another structure [-Wflex-array-member-not-
>> at-end]
>> security/integrity/evm/evm.h:45:32: warning: structure containing a flexible
>> array member is not at the end of another structure [-Wflex-array-member-not-
>> at-end]
>> security/integrity/evm/evm.h:45:32: warning: structure containing a flexible
>> array member is not at the end of another structure [-Wflex-array-member-not-
>> at-end]
> 
> A similar change would need to be made to struct evm_digest:
> 
> struct evm_digest {
>          struct ima_digest_data hdr;
>          char digest[IMA_MAX_DIGEST_SIZE];
> } __packed;
> 
> Is there are another patch?

Oh, I missed that one. I'll include it and send v2, shortly.

Thanks!
--
Gustavo

^ permalink raw reply	[flat|nested] 4+ messages in thread

end of thread, other threads:[~2024-03-21  3:39 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2024-03-04 17:52 [PATCH][next] integrity: Avoid -Wflex-array-member-not-at-end warnings Gustavo A. R. Silva
2024-03-19 18:44 ` Gustavo A. R. Silva
2024-03-21  1:19 ` Mimi Zohar
2024-03-21  3:39   ` Gustavo A. R. Silva

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.