Fix bad free in skein code

Clang's static analyzer found a bad free caused by skein_mac_atomic().
It will allocate a context on the stack and then pass it to
skein_final(), which attempts to free it. Upon inspection,
skein_digest_atomic() also has the same problem.

These functions were created to match the OpenSolaris ICP API, so I was
curious how we avoided this in other providers and looked at the SHA2
code. It appears that SHA2 has a SHA2Final() helper function that is
called by the exported sha2_mac_final()/sha2_digest_final() as well as
the sha2_mac_atomic() and sha2_digest_atomic() functions. The real work
is done in SHA2Final() while some checks and the free are done in
sha2_mac_final()/sha2_digest_final().

We fix the use after free in the skein code by taking inspiration from
the SHA2 code. We introduce a skein_final_nofree() that does most of the
work, and make skein_final() into a function that calls it and then
frees the memory.

Reviewed-by: Brian Behlendorf <[email protected]>
Reviewed-by: Tony Hutter <[email protected]>
Signed-off-by: Richard Yao <[email protected]>
Closes #13954

module/icp/io/skein_mod.c

@@ -421,7 +421,7 @@ skein_update(crypto_ctx_t *ctx, crypto_data_t *data)
  * Supported output digest formats are raw, uio and mblk.
  */
 static int
-skein_final(crypto_ctx_t *ctx, crypto_data_t *digest)
+skein_final_nofree(crypto_ctx_t *ctx, crypto_data_t *digest)
 {
 	int error = CRYPTO_SUCCESS;
 
@@ -452,6 +452,17 @@ skein_final(crypto_ctx_t *ctx, crypto_data_t *digest)
 	else
 		digest->cd_length = 0;
 
+	return (error);
+}
+
+static int
+skein_final(crypto_ctx_t *ctx, crypto_data_t *digest)
+{
+	int error = skein_final_nofree(ctx, digest);
+
+	if (error == CRYPTO_BUFFER_TOO_SMALL)
+		return (error);
+
 	memset(SKEIN_CTX(ctx), 0, sizeof (*SKEIN_CTX(ctx)));
 	kmem_free(SKEIN_CTX(ctx), sizeof (*(SKEIN_CTX(ctx))));
 	SKEIN_CTX_LVALUE(ctx) = NULL;
@@ -485,7 +496,7 @@ skein_digest_atomic(crypto_mechanism_t *mechanism, crypto_data_t *data,
 
 	if ((error = skein_update(&ctx, data)) != CRYPTO_SUCCESS)
 		goto out;
-	if ((error = skein_final(&ctx, data)) != CRYPTO_SUCCESS)
+	if ((error = skein_final_nofree(&ctx, data)) != CRYPTO_SUCCESS)
 		goto out;
 
 out:
@@ -588,7 +599,7 @@ skein_mac_atomic(crypto_mechanism_t *mechanism,
 
 	if ((error = skein_update(&ctx, data)) != CRYPTO_SUCCESS)
 		goto errout;
-	if ((error = skein_final(&ctx, mac)) != CRYPTO_SUCCESS)
+	if ((error = skein_final_nofree(&ctx, mac)) != CRYPTO_SUCCESS)
 		goto errout;
 
 	return (CRYPTO_SUCCESS);