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: <20220907133055.1.Ic8a1dafe960dc0f8302e189642bc88ebb785d274@changeid>
In-Reply-To: <20220907133055.1.Ic8a1dafe960dc0f8302e189642bc88ebb785d274@changeid>
From:   Sarthak Kukreti <sarthakkukreti@chromium.org>
Date:   Wed, 7 Sep 2022 13:45:24 -0700
Message-ID: <CAG9=OMPAP-BS=vqr3FmPn-antqJJKjkQc3Q0GGJRmbHkUaYmTw@mail.gmail.com>
Subject: Re: [PATCH] dm: verity-loadpin: Only trust verity targets with enforcement
To:     Matthias Kaehlcke <mka@chromium.org>
Cc:     Alasdair Kergon <agk@redhat.com>,
        Mike Snitzer <snitzer@kernel.org>,
        Kees Cook <keescook@chromium.org>,
        linux-security-module@vger.kernel.org, dm-devel@redhat.com,
        Douglas Anderson <dianders@chromium.org>,
        linux-kernel@vger.kernel.org
Content-Type: text/plain; charset="UTF-8"
Precedence: bulk

Reviewed-by: Sarthak Kukreti <sarthakkukreti@chromium.org>

On Wed, Sep 7, 2022 at 1:31 PM Matthias Kaehlcke <mka@chromium.org> wrote:
>
> Verity targets can be configured to ignore corrupted data blocks.
> LoadPin must only trust verity targets that are configured to
> perform some kind of enforcement when data corruption is detected,
> like returning an error, restarting the system or triggering a
> panic.
>
> Fixes: b6c1c5745ccc ("dm: Add verity helpers for LoadPin")
> Reported-by: Sarthak Kukreti <sarthakkukreti@chromium.org>
> Signed-off-by: Matthias Kaehlcke <mka@chromium.org>
> ---
>
>  drivers/md/dm-verity-loadpin.c |  8 ++++++++
>  drivers/md/dm-verity-target.c  | 16 ++++++++++++++++
>  drivers/md/dm-verity.h         |  1 +
>  3 files changed, 25 insertions(+)
>
> diff --git a/drivers/md/dm-verity-loadpin.c b/drivers/md/dm-verity-loadpin.c
> index 387ec43aef72..4f78cc55c251 100644
> --- a/drivers/md/dm-verity-loadpin.c
> +++ b/drivers/md/dm-verity-loadpin.c
> @@ -14,6 +14,7 @@ LIST_HEAD(dm_verity_loadpin_trusted_root_digests);
>
>  static bool is_trusted_verity_target(struct dm_target *ti)
>  {
> +       int verity_mode;
>         u8 *root_digest;
>         unsigned int digest_size;
>         struct dm_verity_loadpin_trusted_root_digest *trd;
> @@ -22,6 +23,13 @@ static bool is_trusted_verity_target(struct dm_target *ti)
>         if (!dm_is_verity_target(ti))
>                 return false;
>
> +       verity_mode = dm_verity_get_mode(ti);
> +
> +       if ((verity_mode != DM_VERITY_MODE_EIO) &&
> +           (verity_mode != DM_VERITY_MODE_RESTART) &&
> +           (verity_mode != DM_VERITY_MODE_PANIC))
> +               return false;
> +
>         if (dm_verity_get_root_digest(ti, &root_digest, &digest_size))
>                 return false;
>
> diff --git a/drivers/md/dm-verity-target.c b/drivers/md/dm-verity-target.c
> index 94b6cb599db4..8a00cc42e498 100644
> --- a/drivers/md/dm-verity-target.c
> +++ b/drivers/md/dm-verity-target.c
> @@ -1446,6 +1446,22 @@ bool dm_is_verity_target(struct dm_target *ti)
>         return ti->type->module == THIS_MODULE;
>  }
>
> +/*
> + * Get the verity mode (error behavior) of a verity target.
> + *
> + * Returns the verity mode of the target, or -EINVAL if 'ti' is not a verity
> + * target.
> + */
> +int dm_verity_get_mode(struct dm_target *ti)

nit: It might be cleaner to combine the mode check above into this
function; eg. dm_verity_is_enforcing_mode(struct dm_target *ti).

> +{
> +       struct dm_verity *v = ti->private;
> +
> +       if (!dm_is_verity_target(ti))
> +               return -EINVAL;
> +
> +       return v->mode;
> +}
> +
>  /*
>   * Get the root digest of a verity target.
>   *
> diff --git a/drivers/md/dm-verity.h b/drivers/md/dm-verity.h
> index 45455de1b4bc..98f306ec6a33 100644
> --- a/drivers/md/dm-verity.h
> +++ b/drivers/md/dm-verity.h
> @@ -134,6 +134,7 @@ extern int verity_hash_for_block(struct dm_verity *v, struct dm_verity_io *io,
>                                  sector_t block, u8 *digest, bool *is_zero);
>
>  extern bool dm_is_verity_target(struct dm_target *ti);
> +extern int dm_verity_get_mode(struct dm_target *ti);
>  extern int dm_verity_get_root_digest(struct dm_target *ti, u8 **root_digest,
>                                      unsigned int *digest_size);
>
> --
> 2.37.2.789.g6183377224-goog
>