crypto: omap-sham - hmac calculation bug fix for sha1 base hash

This patch fixes 2 hmac inter-dependent bugs. 1. "omap-sham: hash-in-progress is stored in hw format" commit introduced optimization where temporary hash had been stored in OMAP specific format (big endian). For SHA1 it is different to real hash format, which is little endian. Final HMAC value was calculated using incorrect hash. Because CONFIG_CRYPTO_MANAGER_TESTS was disabled this error remained unnoticed. After enabling this option, bug has been found. 2. HMAC was calculated using temporrary hash value. For a single-request updates, temporary hash was the final one and HMAC result was correct. But in fact only the final hash had to be used. All crypto tests for HMAC produces only single request and could not catch the problem. This problem is fixed here. Signed-off-by: Dmitry Kasatkin <dmitry.kasatkin@nokia.com> Signed-off-by: Herbert Xu <herbert@gondor.apana.org.au>

crypto: omap-sham - hmac calculation bug fix for sha1 base hash
This patch fixes 2 hmac inter-dependent bugs. 1. "omap-sham: hash-in-progress is stored in hw format" commit introduced optimization where temporary hash had been stored in OMAP specific format (big endian). For SHA1 it is different to real hash format, which is little endian. Final HMAC value was calculated using incorrect hash. Because CONFIG_CRYPTO_MANAGER_TESTS was disabled this error remained unnoticed. After enabling this option, bug has been found. 2. HMAC was calculated using temporrary hash value. For a single-request updates, temporary hash was the final one and HMAC result was correct. But in fact only the final hash had to be used. All crypto tests for HMAC produces only single request and could not catch the problem. This problem is fixed here. Signed-off-by: Dmitry Kasatkin <dmitry.kasatkin@nokia.com> Signed-off-by: Herbert Xu <herbert@gondor.apana.org.au>
Dmitry Kasatkin · Herbert Xu
1 parent 528d26f57a
Showing 1 changed file with 31 additions and 40 deletions Side-by-side Diff
drivers/crypto/omap-sham.c
@@ -78,7 +78,6 @@
 #define FLAGS_SHA1		0x0010
 #define FLAGS_DMA_ACTIVE	0x0020
 #define FLAGS_OUTPUT_READY	0x0040
-#define FLAGS_CLEAN		0x0080
 #define FLAGS_INIT		0x0100
 #define FLAGS_CPU		0x0200
 #define FLAGS_HMAC		0x0400
@@ -511,26 +510,6 @@
 	return 0;
 }
  
-static void omap_sham_cleanup(struct ahash_request *req)
-{
-	struct omap_sham_reqctx *ctx = ahash_request_ctx(req);
-	struct omap_sham_dev *dd = ctx->dd;
-	unsigned long flags;
-
-	spin_lock_irqsave(&dd->lock, flags);
-	if (ctx->flags & FLAGS_CLEAN) {
-		spin_unlock_irqrestore(&dd->lock, flags);
-		return;
-	}
-	ctx->flags |= FLAGS_CLEAN;
-	spin_unlock_irqrestore(&dd->lock, flags);
-
-	if (ctx->digcnt)
-		omap_sham_copy_ready_hash(req);
-
-	dev_dbg(dd->dev, "digcnt: %d, bufcnt: %d\n", ctx->digcnt, ctx->bufcnt);
-}
-
 static int omap_sham_init(struct ahash_request *req)
 {
 	struct crypto_ahash *tfm = crypto_ahash_reqtfm(req);
  
@@ -618,9 +597,8 @@
 	return err;
 }
  
-static int omap_sham_finish_req_hmac(struct ahash_request *req)
+static int omap_sham_finish_hmac(struct ahash_request *req)
 {
-	struct omap_sham_reqctx *ctx = ahash_request_ctx(req);
 	struct omap_sham_ctx *tctx = crypto_tfm_ctx(req->base.tfm);
 	struct omap_sham_hmac_ctx *bctx = tctx->base;
 	int bs = crypto_shash_blocksize(bctx->shash);
  
@@ -635,9 +613,26 @@
  
 	return crypto_shash_init(&desc.shash) ?:
 	       crypto_shash_update(&desc.shash, bctx->opad, bs) ?:
-	       crypto_shash_finup(&desc.shash, ctx->digest, ds, ctx->digest);
+	       crypto_shash_finup(&desc.shash, req->result, ds, req->result);
 }
  
+static int omap_sham_finish(struct ahash_request *req)
+{
+	struct omap_sham_reqctx *ctx = ahash_request_ctx(req);
+	struct omap_sham_dev *dd = ctx->dd;
+	int err = 0;
+
+	if (ctx->digcnt) {
+		omap_sham_copy_ready_hash(req);
+		if (ctx->flags & FLAGS_HMAC)
+			err = omap_sham_finish_hmac(req);
+	}
+
+	dev_dbg(dd->dev, "digcnt: %d, bufcnt: %d\n", ctx->digcnt, ctx->bufcnt);
+
+	return err;
+}
+
 static void omap_sham_finish_req(struct ahash_request *req, int err)
 {
 	struct omap_sham_reqctx *ctx = ahash_request_ctx(req);
  
@@ -645,15 +640,12 @@
  
 	if (!err) {
 		omap_sham_copy_hash(ctx->dd->req, 1);
-		if (ctx->flags & FLAGS_HMAC)
-			err = omap_sham_finish_req_hmac(req);
+		if (ctx->flags & FLAGS_FINAL)
+			err = omap_sham_finish(req);
 	} else {
 		ctx->flags |= FLAGS_ERROR;
 	}
  
-	if ((ctx->flags & FLAGS_FINAL) || err)
-		omap_sham_cleanup(req);
-
 	clk_disable(dd->iclk);
 	dd->flags &= ~FLAGS_BUSY;
  
  
  
  
@@ -809,22 +801,21 @@
 static int omap_sham_final(struct ahash_request *req)
 {
 	struct omap_sham_reqctx *ctx = ahash_request_ctx(req);
-	int err = 0;
  
 	ctx->flags |= FLAGS_FINUP;
  
-	if (!(ctx->flags & FLAGS_ERROR)) {
-		/* OMAP HW accel works only with buffers >= 9 */
-		/* HMAC is always >= 9 because of ipad */
-		if ((ctx->digcnt + ctx->bufcnt) < 9)
-			err = omap_sham_final_shash(req);
-		else if (ctx->bufcnt)
-			return omap_sham_enqueue(req, OP_FINAL);
-	}
+	if (ctx->flags & FLAGS_ERROR)
+		return 0; /* uncompleted hash is not needed */
  
-	omap_sham_cleanup(req);
+	/* OMAP HW accel works only with buffers >= 9 */
+	/* HMAC is always >= 9 because ipad == block size */
+	if ((ctx->digcnt + ctx->bufcnt) < 9)
+		return omap_sham_final_shash(req);
+	else if (ctx->bufcnt)
+		return omap_sham_enqueue(req, OP_FINAL);
  
-	return err;
+	/* copy ready hash (+ finalize hmac) */
+	return omap_sham_finish(req);
 }
  
 static int omap_sham_finup(struct ahash_request *req)
...	...	@@ -78,7 +78,6 @@
78	78	#define FLAGS_SHA1 0x0010
79	79	#define FLAGS_DMA_ACTIVE 0x0020
80	80	#define FLAGS_OUTPUT_READY 0x0040
81		-#define FLAGS_CLEAN 0x0080
82	81	#define FLAGS_INIT 0x0100
83	82	#define FLAGS_CPU 0x0200
84	83	#define FLAGS_HMAC 0x0400
...	...	@@ -511,26 +510,6 @@
511	510	return 0;
512	511	}
513	512
514		-static void omap_sham_cleanup(struct ahash_request *req)
515		-{
516		- struct omap_sham_reqctx *ctx = ahash_request_ctx(req);
517		- struct omap_sham_dev *dd = ctx->dd;
518		- unsigned long flags;
519		-
520		- spin_lock_irqsave(&dd->lock, flags);
521		- if (ctx->flags & FLAGS_CLEAN) {
522		- spin_unlock_irqrestore(&dd->lock, flags);
523		- return;
524		- }
525		- ctx->flags \|= FLAGS_CLEAN;
526		- spin_unlock_irqrestore(&dd->lock, flags);
527		-
528		- if (ctx->digcnt)
529		- omap_sham_copy_ready_hash(req);
530		-
531		- dev_dbg(dd->dev, "digcnt: %d, bufcnt: %d\n", ctx->digcnt, ctx->bufcnt);
532		-}
533		-
534	513	static int omap_sham_init(struct ahash_request *req)
535	514	{
536	515	struct crypto_ahash *tfm = crypto_ahash_reqtfm(req);
537	516
...	...	@@ -618,9 +597,8 @@
618	597	return err;
619	598	}
620	599
621		-static int omap_sham_finish_req_hmac(struct ahash_request *req)
	600	+static int omap_sham_finish_hmac(struct ahash_request *req)
622	601	{
623		- struct omap_sham_reqctx *ctx = ahash_request_ctx(req);
624	602	struct omap_sham_ctx *tctx = crypto_tfm_ctx(req->base.tfm);
625	603	struct omap_sham_hmac_ctx *bctx = tctx->base;
626	604	int bs = crypto_shash_blocksize(bctx->shash);
627	605
...	...	@@ -635,9 +613,26 @@
635	613
636	614	return crypto_shash_init(&desc.shash) ?:
637	615	crypto_shash_update(&desc.shash, bctx->opad, bs) ?:
638		- crypto_shash_finup(&desc.shash, ctx->digest, ds, ctx->digest);
	616	+ crypto_shash_finup(&desc.shash, req->result, ds, req->result);
639	617	}
640	618
	619	+static int omap_sham_finish(struct ahash_request *req)
	620	+{
	621	+ struct omap_sham_reqctx *ctx = ahash_request_ctx(req);
	622	+ struct omap_sham_dev *dd = ctx->dd;
	623	+ int err = 0;
	624	+
	625	+ if (ctx->digcnt) {
	626	+ omap_sham_copy_ready_hash(req);
	627	+ if (ctx->flags & FLAGS_HMAC)
	628	+ err = omap_sham_finish_hmac(req);
	629	+ }
	630	+
	631	+ dev_dbg(dd->dev, "digcnt: %d, bufcnt: %d\n", ctx->digcnt, ctx->bufcnt);
	632	+
	633	+ return err;
	634	+}
	635	+
641	636	static void omap_sham_finish_req(struct ahash_request *req, int err)
642	637	{
643	638	struct omap_sham_reqctx *ctx = ahash_request_ctx(req);
644	639
...	...	@@ -645,15 +640,12 @@
645	640
646	641	if (!err) {
647	642	omap_sham_copy_hash(ctx->dd->req, 1);
648		- if (ctx->flags & FLAGS_HMAC)
649		- err = omap_sham_finish_req_hmac(req);
	643	+ if (ctx->flags & FLAGS_FINAL)
	644	+ err = omap_sham_finish(req);
650	645	} else {
651	646	ctx->flags \|= FLAGS_ERROR;
652	647	}
653	648
654		- if ((ctx->flags & FLAGS_FINAL) \|\| err)
655		- omap_sham_cleanup(req);
656		-
657	649	clk_disable(dd->iclk);
658	650	dd->flags &= ~FLAGS_BUSY;
659	651
660	652
661	653
662	654
...	...	@@ -809,22 +801,21 @@
809	801	static int omap_sham_final(struct ahash_request *req)
810	802	{
811	803	struct omap_sham_reqctx *ctx = ahash_request_ctx(req);
812		- int err = 0;
813	804
814	805	ctx->flags \|= FLAGS_FINUP;
815	806
816		- if (!(ctx->flags & FLAGS_ERROR)) {
817		- /* OMAP HW accel works only with buffers >= 9 */
818		- /* HMAC is always >= 9 because of ipad */
819		- if ((ctx->digcnt + ctx->bufcnt) < 9)
820		- err = omap_sham_final_shash(req);
821		- else if (ctx->bufcnt)
822		- return omap_sham_enqueue(req, OP_FINAL);
823		- }
	807	+ if (ctx->flags & FLAGS_ERROR)
	808	+ return 0; /* uncompleted hash is not needed */
824	809
825		- omap_sham_cleanup(req);
	810	+ /* OMAP HW accel works only with buffers >= 9 */
	811	+ /* HMAC is always >= 9 because ipad == block size */
	812	+ if ((ctx->digcnt + ctx->bufcnt) < 9)
	813	+ return omap_sham_final_shash(req);
	814	+ else if (ctx->bufcnt)
	815	+ return omap_sham_enqueue(req, OP_FINAL);
826	816
827		- return err;
	817	+ /* copy ready hash (+ finalize hmac) */
	818	+ return omap_sham_finish(req);
828	819	}
829	820
830	821	static int omap_sham_finup(struct ahash_request *req)