lkml.org 
[lkml]   [2022]   [Jul]   [26]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
Patch in this message
/
Date
From
SubjectRe: [PATCH 3/3] dm-verity: Add try_verify_in_tasklet
On Mon, Jul 25 2022 at 11:06P -0400,
Eric Biggers <ebiggers@kernel.org> wrote:

> On Mon, Jul 25, 2022 at 09:58:39PM -0400, Mike Snitzer wrote:
> >
> > > @@ -1156,7 +1217,7 @@ static int verity_ctr(struct dm_target *ti, unsigned argc, char **argv)
> > > goto bad;
> > > }
> > >
> > > - v->tfm = crypto_alloc_ahash(v->alg_name, 0, 0);
> > > + v->tfm = crypto_alloc_ahash(v->alg_name, 0, CRYPTO_ALG_ASYNC);
> > > if (IS_ERR(v->tfm)) {
> > > ti->error = "Cannot initialize hash function";
> > > r = PTR_ERR(v->tfm);
> >
> > This hunk that adds the CRYPTO_ALG_ASYNC flag _seems_ unrelated.
>
> I believe it's needed to ensure that only a synchronous algorithm is allocated,
> so that verity_hash_update() doesn't have to sleep during the tasklet. It
> should be conditional on v->use_tasklet, though.

Ah yes, it is a mask, that makes sense.

I can now see why it was being set unconditionally given dm-verity's
optional ctr args aren't processed until after the crypto_alloc_ahash() call.
And of course verity_parse_opt_args() depends on non-optional args
related to the tfm.... gah!

Do you have a sense for what the implications are for always setting
CRYPTO_ALG_ASYNC like Nathan had? Will it disallow certain tfm that
may already be in use by some users?

> > @@ -321,14 +320,12 @@ static int verity_verify_level(struct dm_verity *v, struct dm_verity_io *io,
> > if (likely(memcmp(verity_io_real_digest(v, io), want_digest,
> > v->digest_size) == 0))
> > aux->hash_verified = 1;
> > - else if (io->in_tasklet) {
> > + else if (io->in_tasklet)
> > /*
> > * FEC code cannot be run in a tasklet since it may
> > - * sleep. We need to resume execution in a work-queue
> > - * to handle FEC.
> > + * sleep, so fallback to using a work-queue.
> > */
> > return -EAGAIN;
> > - }
>
>
> Doesn't this need to be:
>
> r = -EAGAIN;
> goto release_ret_r;

Yes, good catch.

Thanks,
Mike

\
 
 \ /
  Last update: 2022-07-26 06:14    [W:0.161 / U:0.036 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site