Re: [RFC PATCH 2/3] dm verity: add support for IMA target update event

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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;



[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
[Index of Archives]     [Linux Kernel]     [Linux Kernel Hardening]     [Linux NFS]     [Linux NILFS]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Yosemite News]     [Linux SCSI]

  Powered by Linux