Hi Lakshmi,
On 06.05.22 22:35, Lakshmi Ramasubramanian wrote:
Hi Thore,
On 1/6/2022 12:34 PM, Thore Sommer wrote:
On first corruption the verity targets triggers a dm_target_update event.
This allows other systems to check using IMA if the state of the
device is
still trustworthy via remote attestation.
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.
The problem is that we never see a IMA entry when a target gets marked
as corrupted. The proposed change uses the new dm_target_update event to
remeasure the table when the target table entry changes from valid to
corrupted. I will add a more complete description to v2.
Signed-off-by: Thore Sommer <public@xxxxxxxx>
---
drivers/md/dm-verity-target.c | 6 ++++++
1 file changed, 6 insertions(+)
diff --git a/drivers/md/dm-verity-target.c
b/drivers/md/dm-verity-target.c
index 80133aae0db3..09696e25bf1c 100644
--- a/drivers/md/dm-verity-target.c
+++ b/drivers/md/dm-verity-target.c
@@ -16,6 +16,7 @@
#include "dm-verity.h"
#include "dm-verity-fec.h"
#include "dm-verity-verify-sig.h"
+#include "dm-ima.h"
#include <linux/module.h>
#include <linux/reboot.h>
#include <linux/scatterlist.h>
@@ -218,10 +219,15 @@ static int verity_handle_err(struct dm_verity
*v, enum verity_block_type type,
char *envp[] = { verity_env, NULL };
const char *type_str = "";
struct mapped_device *md = dm_table_get_md(v->ti->table);
+ int old_hash_failed = v->hash_failed;
/* Corruption should be visible in device status in all modes */
v->hash_failed = 1;
+ /* Only remeasure on first failure */
+ if (!old_hash_failed)
+ dm_ima_measure_on_target_update(v->ti);
It is not obvious to me why the call to measure on target update is not
done here immediately. Updating the comment to explain the logic would
help.
The change should be only measured once, because otherwise we would
create many IMA entries each for every corrupted block read if I
understand the verity code correctly. So we need to check if the state
before was valid, but we need to measure after the table was changed to
corrupted (v->hash_failed = 1).
Something like this might be a bit clearer and does not use a extra
variable:
+ if (!v->hash_failed)
+ v->hash_failed = 1;
+ dm_ima_measure_on_target_update(v->ti);
Regards,
Thore
thanks,
-lakshmi
+
if (v->corrupted_errs >= DM_VERITY_MAX_CORRUPTED_ERRS)
goto out;
