[dm-devel] [RFC PATCH 1/3] dm ima: allow targets to remeasure their table entry

[Lakshmi Ramasubramanian]

Hi Thore,

On 1/6/2022 12:34 PM, Thore Sommer wrote:
> A new DM event dm_target_update is introduced for targets to remeasure
> their table entry. This is intended for targets that indicate security
> relevant events by updating one of their table entries (e.g. verity for
> corruption).
In the patch description please state the existing problem (or, 
limitation in the existing implementation) and then describe how the 
proposed change addresses the issue.

> 
> In the event the dm version, device metadata and target data gets
> measured.
> 
> This does not update the hash of the active table because it would require
> to rehash the whole table with all the other targets entries.
> 
> Signed-off-by: Thore Sommer <public at thson.de>
> ---
>   drivers/md/dm-ima.c | 76 +++++++++++++++++++++++++++++++++++++++++++++
>   drivers/md/dm-ima.h |  2 ++
>   2 files changed, 78 insertions(+)
> 
> diff --git a/drivers/md/dm-ima.c b/drivers/md/dm-ima.c
> index 957999998d70..3b1bb97263d9 100644
> --- a/drivers/md/dm-ima.c
> +++ b/drivers/md/dm-ima.c
> @@ -750,3 +750,79 @@ void dm_ima_measure_on_device_rename(struct mapped_device *md)
>   	kfree(new_dev_name);
>   	kfree(new_dev_uuid);
>   }
> +
> +/*
> + * Give the option for targets to remeasure on state change.
> + */
> +void dm_ima_measure_on_target_update(struct dm_target *ti)
> +{
> +	char *ima_buf = NULL, *target_metadata_buf = NULL, *target_data_buf = NULL;
> +	struct dm_target *ti2;
> +	size_t target_metadata_buf_len, target_data_buf_len;
> +	unsigned int num_targets, target_index;
> +	struct dm_table *table = ti->table;
> +	struct mapped_device *md = table->md;
> +	bool found = false;
> +	bool noio = true;
> +	int l = 0;
> +
Initializing "ima_buf" to NULL is not necessary since the statement 
below initializes it.

> +	ima_buf = dm_ima_alloc(DM_IMA_MEASUREMENT_BUF_LEN, GFP_KERNEL, noio);
> +	if (!ima_buf)
> +		return;
> +
> +	target_metadata_buf = dm_ima_alloc(DM_IMA_TARGET_METADATA_BUF_LEN, GFP_KERNEL, noio);
> +	if (!target_metadata_buf)
> +		goto exit;
> +
> +	target_data_buf = dm_ima_alloc(DM_IMA_TARGET_DATA_BUF_LEN, GFP_KERNEL, noio);
> +	if (!target_data_buf)
> +		goto exit;
> +
> +	/*
> +	 * Get the index of the target in the table.
> +	 */
> +	num_targets = dm_table_get_num_targets(table);
> +	for (target_index = 0; target_index < num_targets; target_index++) {
> +		ti2 = dm_table_get_target(table, target_index);
> +		if (!ti)
> +			goto exit;
This check for "ti" can be done on function entry to avoid memory 
allocations and calls to dm_table_get_num_targets(), 
dm_table_get_target() when "ti" is NULL.

> +		if (ti == ti2) {
> +			found = true;
> +			break;
> +		}
> +	}
> +	if (!found)
> +		goto exit;
> +
> +	scnprintf(target_metadata_buf, DM_IMA_TARGET_METADATA_BUF_LEN,
> +			  "target_index=%d,target_begin=%llu,target_len=%llu,",
> +			  target_index, ti->begin, ti->len);
Check return status of "scnprintf()" and handle any error.

thanks,
  -lakshmi

> +	target_metadata_buf_len = strlen(target_metadata_buf);
> +
> +	if (ti->type->status)
> +		ti->type->status(ti, STATUSTYPE_IMA, STATUSTYPE_IMA, target_data_buf,
> +					DM_IMA_TARGET_DATA_BUF_LEN);
> +	else
> +		target_data_buf[0] = '\0';
> +	target_data_buf_len = strlen(target_data_buf);
> +
> +	memcpy(ima_buf + l, DM_IMA_VERSION_STR, md->ima.dm_version_str_len);
> +	l += md->ima.dm_version_str_len;
> +
> +	memcpy(ima_buf + l, md->ima.active_table.device_metadata,
> +	       md->ima.active_table.device_metadata_len);
> +	l += md->ima.active_table.device_metadata_len;
> +
> +	memcpy(ima_buf + l, target_metadata_buf, target_metadata_buf_len);
> +	l += target_metadata_buf_len;
> +
> +	memcpy(ima_buf + l, target_data_buf, target_data_buf_len);
> +
> +	dm_ima_measure_data("dm_target_update", ima_buf, strlen(ima_buf), noio);
> +
> +exit:
> +	kfree(ima_buf);
> +	kfree(target_data_buf);
> +	kfree(target_metadata_buf);
> +}
> +EXPORT_SYMBOL_GPL(dm_ima_measure_on_target_update);
> diff --git a/drivers/md/dm-ima.h b/drivers/md/dm-ima.h
> index b8c3b614670b..281a8b65f8a9 100644
> --- a/drivers/md/dm-ima.h
> +++ b/drivers/md/dm-ima.h
> @@ -63,6 +63,7 @@ void dm_ima_measure_on_device_resume(struct mapped_device *md, bool swap);
>   void dm_ima_measure_on_device_remove(struct mapped_device *md, bool remove_all);
>   void dm_ima_measure_on_table_clear(struct mapped_device *md, bool new_map);
>   void dm_ima_measure_on_device_rename(struct mapped_device *md);
> +void dm_ima_measure_on_target_update(struct dm_target *ti);
>   
>   #else
>   
> @@ -72,6 +73,7 @@ static inline void dm_ima_measure_on_device_resume(struct mapped_device *md, boo
>   static inline void dm_ima_measure_on_device_remove(struct mapped_device *md, bool remove_all) {}
>   static inline void dm_ima_measure_on_table_clear(struct mapped_device *md, bool new_map) {}
>   static inline void dm_ima_measure_on_device_rename(struct mapped_device *md) {}
> +static inline void dm_ima_measure_on_target_update(struct dm_target *ti) {}
>   
>   #endif /* CONFIG_IMA */
>