Re: [PATCH] crypto: arm64/sm4 - Fix possible crash in GCM cryption

[Herbert Xu <herbert@gondor.apana.org.au>]

On Mon, Jan 30, 2023 at 03:34:42PM +0800, Tianjia Zhang wrote:
>
> I printed the walk->nbytes of each iteration of the walker, it is not
> always multiples of chunksize except at the end when the algorithm test
> manager is turned on.

Sorry I was mistaken.  We only guarantee that a minimum of chunksize
bytes is given to you until the very end, not that it is exactly a
multiple of chunksize.

While you still need to compute tail, you could get rid of the else if
check as walk->nbytes - tail cannot be zero (we must provide you with
at least one chunk before the end):

		if (walk->nbytes == walk->total) {
			tail = 0;

			sm4_ce_pmull_gcm_crypt(ctx->key.rkey_enc, dst, src, iv,
					       walk->nbytes, ghash,
					       ctx->ghash_table,
					       (const u8 *)&lengths);
		} else {
			sm4_ce_pmull_gcm_crypt(ctx->key.rkey_enc, dst, src, iv,
					       walk->nbytes - tail, ghash,
					       ctx->ghash_table, NULL);
		}

In fact we could rewrite it like this:

		unsigned int tail = walk->nbytes % SM4_BLOCK_SIZE;
		unsigned int nbytes = walk->nbytes - tail;
		const u8 *src = walk->src.virt.addr;
		u8 *dst = walk->dst.virt.addr;
		u8 *lp = NULL;

		if (walk->nbytes == walk->total) {
			nbytes = walk->nbytes;
			tail = 0;
			lp = (u8 *)&lengths;
		}

		sm4_ce_pmull_gcm_crypt(ctx->key.rkey_enc, dst, src, iv,
				       nbytes, ghash, ctx->ghash_table, lp);

The second part of that loop could also be rewritten as:

		kernel_neon_end();

		err = skcipher_walk_done(walk, tail);
		if (!walk->nbytes)
			return err;

		kernel_neon_begin();
	} while (1);

Actually I think there is a serious bug here.  If you're doing an
empty message, you must not call skcipher_walk_done as that may
then free random uninitialised stack memory.

Did you copy this code from somewhere else? If so wherever you got
it from needs to be fixed too.  The loop should look like this:

	if (!walk->nbytes) {
		/* iv may be unaligned as the walker didn't run at all. */
		sm4_ce_pmull_gcm_crypt(ctx->key.rkey_enc, NULL, NULL, iv,
				       0, ghash, ctx->ghash_table,
				       (u8 *)&lengths);
		kernel_neon_end();
		return 0;
	}

	do {
		...
	}

Thanks,
-- 
Email: Herbert Xu <herbert@gondor.apana.org.au>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt