Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
MIME-Version: 1.0
References: <1687986571-16823-1-git-send-email-wufan@linux.microsoft.com>
 <1687986571-16823-12-git-send-email-wufan@linux.microsoft.com>
 <ZKgm+ffQbdDTxrg9@redhat.com> <20230712034319.GA17642@linuxonhyperv3.guj3yctzbm1etfxqx2vob5hsef.xx.internal.cloudapp.net>
In-Reply-To: <20230712034319.GA17642@linuxonhyperv3.guj3yctzbm1etfxqx2vob5hsef.xx.internal.cloudapp.net>
From:   Paul Moore <paul@paul-moore.com>
Date:   Tue, 25 Jul 2023 16:43:48 -0400
Message-ID: <CAHC9VhQFxqcfgR0acgdiXKP9LT1KLgGjZd-QHs6O1dEex31HEQ@mail.gmail.com>
Subject: Re: [RFC PATCH v10 11/17] dm-verity: consume root hash digest and
 signature data via LSM hook
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, linux-doc@vger.kernel.org,
        linux-integrity@vger.kernel.org,
        linux-security-module@vger.kernel.org,
        linux-fscrypt@vger.kernel.org, linux-block@vger.kernel.org,
        dm-devel@redhat.com, audit@vger.kernel.org,
        roberto.sassu@huawei.com, linux-kernel@vger.kernel.org,
        Deven Bowers <deven.desai@linux.microsoft.com>
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: quoted-printable
Precedence: bulk

On Tue, Jul 11, 2023 at 11:43=E2=80=AFPM Fan Wu <wufan@linux.microsoft.com>=
 wrote:
> On Fri, Jul 07, 2023 at 10:53:45AM -0400, Mike Snitzer wrote:

...

> > Both of your calls to security_bdev_setsecurity() to set your blobs in
> > the bdev are suspect because you're doing so from the verity_ctr().
> > The mapped_device has 2 dm_table slots (active and inactive).  The
> > verity_ctr() becomes part of the inactive slot, there is an extra step
> > to bind the inactive table to the active table.
> >
> > This leads to you changing the blobs in the global bdev _before_ the
> > table is actually active.  It is possible that the inactive table will
> > simply be removed and the DM verity device put back in service;
> > leaving your blob(s) in the bdev inconsistent.
> >
> > This issue has parallels to how we need to defer changing the global
> > queue_limits associated with a request_queue until _after_ all table
> > loading is settled and then the update is done just before resuming
> > the DM device (mapped_device) -- see dm_table_set_restrictions().
> >
> > Unfortunately, this feels like it may require a new hook in the
> > target_type struct (e.g. ->finalize())
>
> Thanks for pointing out this issue. We were calling security_bdev_setsecu=
rity()
> because the roothash signature data is only available in verity_ctr()
> and it is discarded after verity_ctr() finishes.
> After digging deeper into the table_load, I realized that we were indeed
> wrong here.
>
> Based on my understanding of your suggestion, it seems that the correct
> approach would be to save the roothash signature into the struct dm_targe=
t

Would you be doing this with a LSM hook, or would this live in the
device mapper layer?

> and then invoke security_bdev_setsecurity() before activating
> the inactive table in the __bind function (where dm_table_set_restriction=
s is called).
>
> To facilitate this process, it seems appropriate to introduce a new hook
> called finalize() within the struct target_type. This hook would enable
> targets to define tasks that need to be completed before activating
> a new table.
>
> In our specific case, we would add a finalize hook to the dm-verity modul=
e,
> allowing us to call security_bdev_setsecurity() and associate the roothas=
h
> information in the struct dm_target with the struct block_device of
> the struct mapped_device. Is this correct?

Where would the finalize() hook be called?

--=20
paul-moore.com