On Wed, 19 Jan 2011, Struk, Tadeusz wrote:

> Hi Jesper,

Hi Tadeusz,

> We have tested your changes and all is fine.
> Thank you for identifying this issue.

You are welcome. But I believe I made a mistake in that patch. I looked at 
it again and I'm convinced that the 'out' label should not exist. The 
jump to it will still leak 'ctr_tfm' - don't you agree?

I believe the patch should look like this instead:


Signed-off-by: Jesper Juhl <j...@chaosbits.net>
---
 aesni-intel_glue.c |   16 ++++++++--------
 1 file changed, 8 insertions(+), 8 deletions(-)

diff --git a/arch/x86/crypto/aesni-intel_glue.c 
b/arch/x86/crypto/aesni-intel_glue.c
index e1e60c7..4a8c015 100644
--- a/arch/x86/crypto/aesni-intel_glue.c
+++ b/arch/x86/crypto/aesni-intel_glue.c
@@ -873,21 +873,19 @@ rfc4106_set_hash_subkey(u8 *hash_subkey, const u8 *key, 
unsigned int key_len)
        crypto_ablkcipher_clear_flags(ctr_tfm, ~0);
 
        ret = crypto_ablkcipher_setkey(ctr_tfm, key, key_len);
-       if (ret) {
-               crypto_free_ablkcipher(ctr_tfm);
-               return ret;
-       }
+       if (ret)
+               goto out_free_ablkcipher;
 
        req = ablkcipher_request_alloc(ctr_tfm, GFP_KERNEL);
        if (!req) {
-               crypto_free_ablkcipher(ctr_tfm);
-               return -EINVAL;
+               ret = -EINVAL;
+               goto out_free_ablkcipher;
        }
 
        req_data = kmalloc(sizeof(*req_data), GFP_KERNEL);
        if (!req_data) {
-               crypto_free_ablkcipher(ctr_tfm);
-               return -ENOMEM;
+               ret = -ENOMEM;
+               goto out_free_request;
        }
        memset(req_data->iv, 0, sizeof(req_data->iv));
 
@@ -913,8 +911,10 @@ rfc4106_set_hash_subkey(u8 *hash_subkey, const u8 *key, 
unsigned int key_len)
                if (!ret)
                        ret = req_data->result.err;
        }
+out_free_request:
        ablkcipher_request_free(req);
        kfree(req_data);
+out_free_ablkcipher:
        crypto_free_ablkcipher(ctr_tfm);
        return ret;
 }


Herbert: If Tadeusz agrees, could you please replace the patch you merged 
with the one above?

-- 
Jesper Juhl <j...@chaosbits.net>            http://www.chaosbits.net/
Plain text mails only, please.
Don't top-post http://www.catb.org/~esr/jargon/html/T/top-post.html

--
To unsubscribe from this list: send the line "unsubscribe linux-crypto" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Reply via email to