Linux-Block Archive mirror
 help / color / mirror / Atom feed
From: Mikulas Patocka <mpatocka@redhat.com>
To: Fan Wu <wufan@linux.microsoft.com>
Cc: Mike Snitzer <snitzer@kernel.org>,
	corbet@lwn.net, zohar@linux.ibm.com,  jmorris@namei.org,
	serge@hallyn.com, tytso@mit.edu, ebiggers@kernel.org,
	 axboe@kernel.dk, agk@redhat.com, eparis@redhat.com,
	paul@paul-moore.com,  linux-doc@vger.kernel.org,
	linux-integrity@vger.kernel.org,
	 linux-security-module@vger.kernel.org, fsverity@lists.linux.dev,
	 linux-block@vger.kernel.org, dm-devel@lists.linux.dev,
	 audit@vger.kernel.org, linux-kernel@vger.kernel.org
Subject: Re: [PATCH v18 12/21] dm: add finalize hook to target_type
Date: Mon, 20 May 2024 14:31:16 +0200 (CEST)	[thread overview]
Message-ID: <889a7880-8336-a44a-bea4-a4c81c5e5cce@redhat.com> (raw)
In-Reply-To: <234910c1-40c3-4489-94ab-6e9a5f00d93e@linux.microsoft.com>



On Fri, 17 May 2024, Fan Wu wrote:

> > So, it seems that the preresume callback provides the guarantee that you
> > looking for.
> > 
> >> -Fan
> > 
> > Mikulas
> 
> Thanks for the info. I have tested and verified that the preresume() hook can
> also work for our case.
> 
> From the source code
> https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/drivers/md/dm-ioctl.c#n1149,
> the whole resume process appears to be:
> 
> 1. Check if there is a new map for the device. If so, attempt to activate the
> new map using dm_swap_table() (where the finalize() callback occurs).
> 
> 2. Check if the device is suspended. If so, use dm_resume() (where the
> preresume() callback occurs) to resume the device.
> 
> 3. If a new map is activated, use dm_table_destroy() to destroy the old map.
> 
> For our case:
> 
> - Using the finalize() callback, the metadata of the dm-verity target inside
> the table is attached to the mapped device every time a new table is
> activated.
> - Using the preresume() callback, the same metadata is attached every time the
> device resumes from suspension.
> 
> If I understand the code correctly, resuming from suspension is a necessary
> step for loading a new mapping table. Thus, the preresume() callback covers
> all conditions where the finalize() callback would be triggered.

Yes.

> However, the preresume() callback can also be triggered when the device 
> resumes from suspension without loading a new table, in which case there 
> is no new metadata in the table to attach to the mapped device.

Yes.

> In the scenario where the finalize() callback succeeds but the preresume()
> callback fails, it seems the device will remain in a suspended state, the
> newly activated table will be kept, and the old table will be destroyed, so it
> seems there is no inconsistency using finalize() even preresume() potentially
> fails.

What does your security module do when the verification of the dm-verity 
hash fails? Does it halt the whole system? Does it destroy just the 
failing dm device? Or does it attempt to recover somehow from this 
situation?

> I believe both the finalize() callback proposed by Mike and the preresume()
> callback suggested by Mikulas can work for our case. I am fine with either
> approach, but I would like to know which one is preferred by the maintainers
> and would appreciate an ACK for the chosen approach.
> 
> -Fan

I would prefer preresume - we shouldn't add new callbacks unless it's 
necessary.

Mikulas


  reply	other threads:[~2024-05-20 12:31 UTC|newest]

Thread overview: 33+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2024-05-03 22:32 [PATCH v18 00/21] Integrity Policy Enforcement LSM (IPE) Fan Wu
2024-05-03 22:32 ` [PATCH v18 01/21] security: add ipe lsm Fan Wu
2024-05-03 22:32 ` [PATCH v18 02/21] ipe: add policy parser Fan Wu
2024-05-03 22:32 ` [PATCH v18 03/21] ipe: add evaluation loop Fan Wu
2024-05-03 22:32 ` [PATCH v18 04/21] ipe: add LSM hooks on execution and kernel read Fan Wu
2024-05-03 22:32 ` [PATCH v18 05/21] initramfs|security: Add a security hook to do_populate_rootfs() Fan Wu
2024-05-03 22:32 ` [PATCH v18 06/21] ipe: introduce 'boot_verified' as a trust provider Fan Wu
2024-05-03 22:32 ` [PATCH v18 07/21] security: add new securityfs delete function Fan Wu
2024-05-03 22:32 ` [PATCH v18 08/21] ipe: add userspace interface Fan Wu
2024-05-03 22:32 ` [PATCH v18 09/21] uapi|audit|ipe: add ipe auditing support Fan Wu
2024-05-03 22:32 ` [PATCH v18 10/21] ipe: add permissive toggle Fan Wu
2024-05-03 22:32 ` [PATCH v18 11/21] block,lsm: add LSM blob and new LSM hooks for block device Fan Wu
2024-05-03 22:32 ` [PATCH v18 12/21] dm: add finalize hook to target_type Fan Wu
2024-05-08 17:17   ` Mikulas Patocka
2024-05-08 22:30     ` Fan Wu
2024-05-09 17:07       ` Mikulas Patocka
2024-05-17 19:13         ` Fan Wu
2024-05-20 12:31           ` Mikulas Patocka [this message]
2024-05-21 21:42             ` Fan Wu
2024-05-03 22:32 ` [PATCH v18 13/21] dm verity: expose root hash digest and signature data to LSMs Fan Wu
2024-05-03 22:32 ` [PATCH v18 14/21] ipe: add support for dm-verity as a trust provider Fan Wu
2024-05-03 22:32 ` [PATCH v18 15/21] security: add security_inode_setintegrity() hook Fan Wu
2024-05-03 22:32 ` [PATCH v18 16/21] fsverity: expose verified fsverity built-in signatures to LSMs Fan Wu
2024-05-14 19:55   ` Paul Moore
2024-05-03 22:32 ` [PATCH v18 17/21] ipe: enable support for fs-verity as a trust provider Fan Wu
2024-05-03 22:32 ` [PATCH v18 18/21] scripts: add boot policy generation program Fan Wu
2024-05-03 22:32 ` [PATCH v18 19/21] ipe: kunit test for parser Fan Wu
2024-05-03 22:32 ` [PATCH v18 20/21] Documentation: add ipe documentation Fan Wu
2024-05-04  8:04   ` Bagas Sanjaya
2024-05-04 20:13     ` Fan Wu
2024-05-05  0:15       ` Bagas Sanjaya
2024-05-14 19:50       ` Paul Moore
2024-05-03 22:32 ` [PATCH v18 21/21] MAINTAINERS: ipe: add ipe maintainer information Fan Wu

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=889a7880-8336-a44a-bea4-a4c81c5e5cce@redhat.com \
    --to=mpatocka@redhat.com \
    --cc=agk@redhat.com \
    --cc=audit@vger.kernel.org \
    --cc=axboe@kernel.dk \
    --cc=corbet@lwn.net \
    --cc=dm-devel@lists.linux.dev \
    --cc=ebiggers@kernel.org \
    --cc=eparis@redhat.com \
    --cc=fsverity@lists.linux.dev \
    --cc=jmorris@namei.org \
    --cc=linux-block@vger.kernel.org \
    --cc=linux-doc@vger.kernel.org \
    --cc=linux-integrity@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-security-module@vger.kernel.org \
    --cc=paul@paul-moore.com \
    --cc=serge@hallyn.com \
    --cc=snitzer@kernel.org \
    --cc=tytso@mit.edu \
    --cc=wufan@linux.microsoft.com \
    --cc=zohar@linux.ibm.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).