linux-kernel - Re: [PATCH 3/3] dm-verity: Add try_verify_in

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite for Android: free password hash cracker in your pocket

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:   Mon, 25 Jul 2022 20:06:01 -0700
From:   Eric Biggers <ebiggers@...nel.org>
To:     Mike Snitzer <snitzer@...hat.com>
Cc:     Nathan Huckleberry <nhuck@...gle.com>,
        linux-kernel@...r.kernel.org, dm-devel@...hat.com,
        Alasdair Kergon <agk@...hat.com>,
        Mike Snitzer <snitzer@...nel.org>,
        Sami Tolvanen <samitolvanen@...gle.com>
Subject: Re: [PATCH 3/3] dm-verity: Add try_verify_in_tasklet

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.

> @@ -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;

- Eric