[PATCH] crypto : prevent cryptoloop from oopsing on stupid ciphers

With the cryptoloop patch applied, it's possible to request
ECB mode encryption, which will result in a Oops because of
uninitialized function pointers. Initializing them to the
nocrypt_iv() function does not completely solve the problem
because cryptoloop does not check the return code, and kernel
memory will leak uninitialized through cryptoloop.

Proposed solution :
  Can we maybe define working but IV-ignoring functions for ECB (like I
  did), but use memory-clearing nocrypt*() for CFB and CTR (as long as
  these are not supported)?  Of course, all of these will return -ENOSYS.

Response from Herbert Xu :
  In cryptodev-2.6, with block ciphers you can no longer select CFB/CTR
  until someone writes support for them so this is no longer an issue.
  For 2.4, I don't really mind either way what nocrypt does.

Final solution :
  OK, I've merged Willy's suggestion for the memset()s into the patch
  that I had submitted previously.  The resulting patch is attached.

diff --git a/crypto/cipher.c b/crypto/cipher.c
index 6ab56eb..9b03eda 100644 (file)
--- a/crypto/cipher.c
+++ b/crypto/cipher.c
@@ -147,6 +147,15 @@ static int ecb_encrypt(struct crypto_tfm *tfm,
                     ecb_process, 1, NULL);
 }
 
+static int ecb_encrypt_iv(struct crypto_tfm *tfm,
+                         struct scatterlist *dst,
+                         struct scatterlist *src,
+                         unsigned int nbytes, u8 *iv)
+{
+       ecb_encrypt(tfm, dst, src, nbytes);
+       return -ENOSYS;
+}
+
 static int ecb_decrypt(struct crypto_tfm *tfm,
                        struct scatterlist *dst,
                        struct scatterlist *src,
@@ -157,6 +166,15 @@ static int ecb_decrypt(struct crypto_tfm *tfm,
                     ecb_process, 1, NULL);
 }
 
+static int ecb_decrypt_iv(struct crypto_tfm *tfm,
+                         struct scatterlist *dst,
+                         struct scatterlist *src,
+                         unsigned int nbytes, u8 *iv)
+{
+       ecb_decrypt(tfm, dst, src, nbytes);
+       return -ENOSYS;
+}
+
 static int cbc_encrypt(struct crypto_tfm *tfm,
                        struct scatterlist *dst,
                        struct scatterlist *src,
@@ -197,11 +215,20 @@ static int cbc_decrypt_iv(struct crypto_tfm *tfm,
                     cbc_process, 0, iv);
 }
 
+/*
+ * nocrypt*() zeroize the destination buffer to make sure we don't leak
+ * uninitialized memory contents if the caller ignores the return value.
+ * This is bad since the data in the source buffer is unused and may be
+ * lost, but an infoleak would be even worse.  The performance cost of
+ * memset() is irrelevant since a well-behaved caller would not bump into
+ * the error repeatedly.
+ */
 static int nocrypt(struct crypto_tfm *tfm,
                    struct scatterlist *dst,
                    struct scatterlist *src,
                   unsigned int nbytes)
 {
+       memset(dst, 0, nbytes);
        return -ENOSYS;
 }
 
@@ -210,6 +237,7 @@ static int nocrypt_iv(struct crypto_tfm *tfm,
                       struct scatterlist *src,
                       unsigned int nbytes, u8 *iv)
 {
+       memset(dst, 0, nbytes);
        return -ENOSYS;
 }
 
@@ -235,6 +263,11 @@ int crypto_init_cipher_ops(struct crypto_tfm *tfm)
        case CRYPTO_TFM_MODE_ECB:
                ops->cit_encrypt = ecb_encrypt;
                ops->cit_decrypt = ecb_decrypt;
+/* These should have been nocrypt_iv, but patch-cryptoloop-jari-2.4.22.0
+ * (and its other revisions) directly calls the *_iv() functions even in
+ * ECB mode and ignores their return value. */
+               ops->cit_encrypt_iv = ecb_encrypt_iv;
+               ops->cit_decrypt_iv = ecb_decrypt_iv;
                break;
                
        case CRYPTO_TFM_MODE_CBC: