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

On Thu, 15 Aug 2024, Paul Moore wrote:

quoted

On Thu, Aug 8, 2024 at 6:38 PM Fan Wu [off-list ref] wrote:

quoted

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@linux.microsoft.com (local)

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@redhat.com>

Thank you for reviewing the patch and for your suggestion.

quoted

quoted

quoted

+#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