Re: [PATCH v20 12/20] dm verity: expose root hash digest and signature data to LSMs

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

 





On 8/16/2024 6:35 AM, Mikulas Patocka wrote:


On Thu, 15 Aug 2024, Paul Moore wrote:

On Thu, Aug 8, 2024 at 6:38 PM Fan Wu <wufan@xxxxxxxxxxxxxxxxxxx> wrote:

Hi Mikulas,

I hope you’re doing well. I wanted to thank you again for your thorough
review for the last version. I’ve since made some minor updates for this
version, including adding more comments and refactoring the way the hash
algorithm name is obtained due to recent changes in dm-verity.

Would you mind if we keep the Review-by tag on the latest version since
the changes are minor? Your feedback is greatly valued, and I’d
appreciate it if you could take a quick look when you have a moment.

To add a bit more to this, this patchset now looks like it is in a
state where we would like to merge it into the LSM tree for the
upcoming merge window, but I would really like to make sure that the
device-mapper folks are okay with these changes; an
Acked-by/Reviewed-by on this patch would be appreciated, assuming you
are still okay with this patch.

For those who may be missing the context, the full patchset can be
found on lore at the link below:

https://lore.kernel.org/linux-security-module/1722665314-21156-1-git-send-email-wufan@xxxxxxxxxxxxxxxxxxx

Hi

I'm not an expert in Linux security subsystems. I skimmed through the
dm-verity patch, didn't find anything wrong with it, so you can add

Reviewed-by: Mikulas Patocka <mpatocka@xxxxxxxxxx>


Thank you for reviewing the patch and for your suggestion.


+#ifdef CONFIG_SECURITY
+     u8 *root_digest_sig;    /* signature of the root digest */
+#endif /* CONFIG_SECURITY */
       unsigned int salt_size;
       sector_t data_start;    /* data offset in 512-byte sectors */
       sector_t hash_start;    /* hash start in blocks */
@@ -58,6 +61,9 @@ struct dm_verity {
       bool hash_failed:1;     /* set if hash of any block failed */
       bool use_bh_wq:1;       /* try to verify in BH wq before normal work-queue */
       unsigned int digest_size;       /* digest size for the current hash algorithm */
+#ifdef CONFIG_SECURITY
+     unsigned int sig_size;  /* root digest signature size */
+#endif /* CONFIG_SECURITY */
       unsigned int hash_reqsize; /* the size of temporary space for crypto */
       enum verity_mode mode;  /* mode for handling verification errors */
       unsigned int corrupted_errs;/* Number of errors for corrupted blocks */

Just nit-picking: I would move "unsigned int sig_size" up, after "u8
*root_digest_sig" entry.

Mikulas

Sure, I can make these two fields together.

-Fan




[Index of Archives]     [DM Crypt]     [Fedora Desktop]     [ATA RAID]     [Fedora Marketing]     [Fedora Packaging]     [Fedora SELinux]     [Yosemite Discussion]     [KDE Users]     [Fedora Docs]

  Powered by Linux