Commit 3ac2939b authored by Harald Freudenberger's avatar Harald Freudenberger Committed by Herbert Xu
Browse files

crypto: s390/phmac - Do not modify the req->nbytes value 



The phmac implementation used the req->nbytes field on combined
operations (finup, digest) to track the state:
with req->nbytes > 0 the update needs to be processed,
while req->nbytes == 0 means to do the final operation. For
this purpose the req->nbytes field was set to 0 after successful
update operation. However, aead uses the req->nbytes field after a
successful hash operation to determine the amount of data to
en/decrypt. So an implementation must not modify the nbytes field.

Fixed by a slight rework on the phmac implementation. There is
now a new field async_op in the request context which tracks
the (asynch) operation to process. So the 'state' via req->nbytes
is not needed any more and now this field is untouched and may
be evaluated even after a request is processed by the phmac
implementation.

Fixes: cbbc6755 ("crypto: s390 - New s390 specific protected key hash phmac")
Reported-by: default avatarIngo Franzki <ifranzki@linux.ibm.com>
Signed-off-by: default avatarHarald Freudenberger <freude@linux.ibm.com>
Tested-by: default avatarIngo Franzki <ifranzki@linux.ibm.com>
Reviewed-by: default avatarIngo Franzki <ifranzki@linux.ibm.com>
Reviewed-by: default avatarHolger Dengler <dengler@linux.ibm.com>
Signed-off-by: default avatarHerbert Xu <herbert@gondor.apana.org.au>
parent 3a866087

arch/s390/crypto/phmac_s390.c

+34 −18
Original line number Diff line number Diff line
@@ -169,11 +169,18 @@ struct kmac_sha2_ctx { 
	u64 buflen[2];

};



enum async_op {

	OP_NOP = 0,

	OP_UPDATE,

	OP_FINAL,

	OP_FINUP,

};



/* phmac request context */

struct phmac_req_ctx {

	struct hash_walk_helper hwh;

	struct kmac_sha2_ctx kmac_ctx;

	bool final;

	enum async_op async_op;

};



/*
@@ -610,6 +617,7 @@ static int phmac_update(struct ahash_request *req) 
	 * using engine to serialize requests.

	 */

	if (rc == 0 || rc == -EKEYEXPIRED) {

		req_ctx->async_op = OP_UPDATE;

		atomic_inc(&tfm_ctx->via_engine_ctr);

		rc = crypto_transfer_hash_request_to_engine(phmac_crypto_engine, req);

		if (rc != -EINPROGRESS)
@@ -647,8 +655,7 @@ static int phmac_final(struct ahash_request *req) 
	 * using engine to serialize requests.

	 */

	if (rc == 0 || rc == -EKEYEXPIRED) {

		req->nbytes = 0;

		req_ctx->final = true;

		req_ctx->async_op = OP_FINAL;

		atomic_inc(&tfm_ctx->via_engine_ctr);

		rc = crypto_transfer_hash_request_to_engine(phmac_crypto_engine, req);

		if (rc != -EINPROGRESS)
@@ -676,13 +683,16 @@ static int phmac_finup(struct ahash_request *req) 
	if (rc)

		goto out;



	req_ctx->async_op = OP_FINUP;



	/* Try synchronous operations if no active engine usage */

	if (!atomic_read(&tfm_ctx->via_engine_ctr)) {

		rc = phmac_kmac_update(req, false);

		if (rc == 0)

			req->nbytes = 0;

			req_ctx->async_op = OP_FINAL;

	}

	if (!rc && !req->nbytes && !atomic_read(&tfm_ctx->via_engine_ctr)) {

	if (!rc && req_ctx->async_op == OP_FINAL &&

	    !atomic_read(&tfm_ctx->via_engine_ctr)) {

		rc = phmac_kmac_final(req, false);

		if (rc == 0)

			goto out;
@@ -694,7 +704,7 @@ static int phmac_finup(struct ahash_request *req) 
	 * using engine to serialize requests.

	 */

	if (rc == 0 || rc == -EKEYEXPIRED) {

		req_ctx->final = true;

		/* req->async_op has been set to either OP_FINUP or OP_FINAL */

		atomic_inc(&tfm_ctx->via_engine_ctr);

		rc = crypto_transfer_hash_request_to_engine(phmac_crypto_engine, req);

		if (rc != -EINPROGRESS)
@@ -855,15 +865,16 @@ static int phmac_do_one_request(struct crypto_engine *engine, void *areq) 


	/*

	 * Three kinds of requests come in here:

	 * update when req->nbytes > 0 and req_ctx->final is false

	 * final when req->nbytes = 0 and req_ctx->final is true

	 * finup when req->nbytes > 0 and req_ctx->final is true

	 * For update and finup the hwh walk needs to be prepared and

	 * up to date but the actual nr of bytes in req->nbytes may be

	 * any non zero number. For final there is no hwh walk needed.

	 * 1. req->async_op == OP_UPDATE with req->nbytes > 0

	 * 2. req->async_op == OP_FINUP with req->nbytes > 0

	 * 3. req->async_op == OP_FINAL

	 * For update and finup the hwh walk has already been prepared

	 * by the caller. For final there is no hwh walk needed.

	 */



	if (req->nbytes) {

	switch (req_ctx->async_op) {

	case OP_UPDATE:

	case OP_FINUP:

		rc = phmac_kmac_update(req, true);

		if (rc == -EKEYEXPIRED) {

			/*
@@ -880,10 +891,11 @@ static int phmac_do_one_request(struct crypto_engine *engine, void *areq) 
			hwh_advance(hwh, rc);

			goto out;

		}

		req->nbytes = 0;

	}



	if (req_ctx->final) {

		if (req_ctx->async_op == OP_UPDATE)

			break;

		req_ctx->async_op = OP_FINAL;

		fallthrough;

	case OP_FINAL:

		rc = phmac_kmac_final(req, true);

		if (rc == -EKEYEXPIRED) {

			/*
@@ -897,10 +909,14 @@ static int phmac_do_one_request(struct crypto_engine *engine, void *areq) 
			cond_resched();

			return -ENOSPC;

		}

		break;

	default:

		/* unknown/unsupported/unimplemented asynch op */

		return -EOPNOTSUPP;

	}



out:

	if (rc || req_ctx->final)

	if (rc || req_ctx->async_op == OP_FINAL)

		memzero_explicit(kmac_ctx, sizeof(*kmac_ctx));

	pr_debug("request complete with rc=%d\n", rc);

	local_bh_disable();