linux-kernel - Re: [PATCH v7 3/3] crypto: Add Inside Secure SafeXcel EIP-93 crypto engine support

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: Windows password security audit tool. GUI, reports in PDF.

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

Message-ID: <67582c1b.050a0220.83ef5.c8df@mx.google.com>
Date: Tue, 10 Dec 2024 12:55:02 +0100
From: Christian Marangi <ansuelsmth@...il.com>
To: Herbert Xu <herbert@...dor.apana.org.au>
Cc: "David S. Miller" <davem@...emloft.net>, Rob Herring <robh@...nel.org>,
	Krzysztof Kozlowski <krzk+dt@...nel.org>,
	Conor Dooley <conor+dt@...nel.org>,
	Antoine Tenart <atenart@...nel.org>,
	Peter Zijlstra <peterz@...radead.org>,
	Ingo Molnar <mingo@...hat.com>, Will Deacon <will@...nel.org>,
	Waiman Long <longman@...hat.com>, Boqun Feng <boqun.feng@...il.com>,
	Nathan Chancellor <nathan@...nel.org>,
	Nick Desaulniers <ndesaulniers@...gle.com>,
	Bill Wendling <morbo@...gle.com>,
	Justin Stitt <justinstitt@...gle.com>, linux-crypto@...r.kernel.org,
	devicetree@...r.kernel.org, linux-kernel@...r.kernel.org,
	llvm@...ts.linux.dev, upstream@...oha.com,
	Richard van Schagen <vschagen@...oud.com>
Subject: Re: [PATCH v7 3/3] crypto: Add Inside Secure SafeXcel EIP-93 crypto
 engine support

On Tue, Dec 10, 2024 at 11:23:24AM +0800, Herbert Xu wrote:
> On Tue, Nov 12, 2024 at 02:59:00AM +0100, Christian Marangi wrote:
> >
> > +static int eip93_hash_export(struct ahash_request *req, void *out)
> > +{
> > +	struct eip93_hash_reqctx *rctx = ahash_request_ctx(req);
> > +	struct eip93_hash_export_state *state = out;
> > +	DECLARE_CRYPTO_WAIT(wait);
> > +	int ret;
> > +
> > +	crypto_init_wait(&wait);
> > +	/* Set the req callback for hash_partial_final wait */
> > +	ahash_request_set_callback(req, CRYPTO_TFM_REQ_MAY_BACKLOG,
> > +				   crypto_req_done, &wait);
> > +
> > +	/* Set for partial hash generation */
> > +	rctx->partial_hash = true;
> > +	rctx->export_state = true;
> > +	rctx->state = state;
> > +
> > +	/* Save the first block in state data */
> > +	if (rctx->left_last || rctx->len) {
> > +		struct mkt_hash_block *block;
> > +
> > +		block = list_first_entry(&rctx->blocks,
> > +					 struct mkt_hash_block,
> > +					 list);
> > +
> > +		memcpy(state->data, block->data,
> > +		       SHA256_BLOCK_SIZE - rctx->left_last);
> > +	}
> > +
> > +	/* Call hash_partial_final.
> > +	 * This will send a dummpy 0 length packet. This is done to
> > +	 * wait for every descriptor to being handled and sync the sa_state
> > +	 * from the host. Partial hash and any other data will be copied in
> > +	 * eip93_hash_handle_result()
> > +	 */
> > +	ret = crypto_wait_req(eip93_hash_partial_final(req), &wait);
> 
> Sorry, you can't do that here.  Your hash state should have been
> exported when the request previously completed.  The export function
> must not sleep.
>

Ah that unfortunate but doesn't that goes against the async idea of ahash
OPs? Anyway is busy loop acceptable here?

The main problem here is that .update only enqueue packet to be
processed and we don't wait for it to finish as that would result in
really bad performance.

To export the state with the previous request (.update) we would have to
wait for each packet to complete or we would export the wrong partial
hash.

The process is fast enough so busy loop here should not be problematic
but waiting for every packet sent to .update on being processed might
introduce big performance regression.

Thanks for checking this hope you can help me understand this.

-- 
	Ansuel