qemu-devel.nongnu.org archive mirror
 help / color / mirror / Atom feed
* [PATCH 00/17] crypto/cipher: Class hierarchy cleanups
@ 2020-08-13  3:25 Richard Henderson
  2020-08-13  3:25 ` [PATCH 01/17] crypto: Move QCryptoCipher typedef to qemu/typedefs.h Richard Henderson
                   ` (20 more replies)
  0 siblings, 21 replies; 52+ messages in thread
From: Richard Henderson @ 2020-08-13  3:25 UTC (permalink / raw)
  To: qemu-devel; +Cc: berrange

Mostly this is intended to cleanup the class hierarchy
used for the ciphers.  We currently have multiple levels
of dispatch, and multiple separate allocations.  The final
patches rearrange this to one level of indirect call, and
all memory allocated contiguously.

But on the way there are a number of other misc cleanups.

I know those final patches are somewhat big, but I don't
immediately see how to split them apart.

I noticed this while profiling patches to make ARM PAUTH
use the crypto subsystem.  The qcrypto_cipher_* dispatch
routines were consuming a noticeable portion of the runtime,
and with these changes they were down below 1% where they
ought to be.

While I did not continue with PAUTH using AES, I still think
these are good cleanups.


r~


Richard Henderson (17):
  crypto: Move QCryptoCipher typedef to qemu/typedefs.h
  crypto: Move QCryptoCipherDriver typedef to qemu/typedefs.h
  crypto: Assume blocksize is a power of 2
  crypto: Rename cipher include files to .inc.c
  crypto: Remove redundant includes
  crypto/nettle: Fix xts_encrypt arguments
  crypto: Use the correct const type for driver
  crypto: Allocate QCryptoCipher with the subclass
  crypto: Move cipher->driver init to qcrypto_*_cipher_ctx_new
  crypto: Constify cipher data tables
  crypto/builtin: Remove odd-sized AES block handling
  crypto/builtin: Merge qcrypto_cipher_aes_{ecb,xts}_{en,de}crypt
  crypto/builtin: Move AES_cbc_encrypt into cipher-builtin.inc.c
  crypto/builtin: Split and simplify AES_encrypt_cbc
  crypto/builtin: Split QCryptoCipherBuiltin into subclasses
  crypto/nettle: Split QCryptoCipherNettle into subclasses
  crypto/gcrypt: Split QCryptoCipherGcrypt into subclasses

 crypto/afalgpriv.h                            |   3 +
 crypto/cipherpriv.h                           |   6 +-
 include/crypto/aes.h                          |   4 -
 include/crypto/cipher.h                       |   5 +-
 include/qemu/typedefs.h                       |   2 +
 crypto/aes.c                                  |  51 --
 crypto/cipher-afalg.c                         |  25 +-
 crypto/cipher-builtin.c                       | 532 ------------
 crypto/cipher-builtin.inc.c                   | 425 ++++++++++
 .../{cipher-gcrypt.c => cipher-gcrypt.inc.c}  | 522 ++++++------
 crypto/cipher-nettle.c                        | 733 -----------------
 crypto/cipher-nettle.inc.c                    | 756 ++++++++++++++++++
 crypto/cipher.c                               |  44 +-
 13 files changed, 1477 insertions(+), 1631 deletions(-)
 delete mode 100644 crypto/cipher-builtin.c
 create mode 100644 crypto/cipher-builtin.inc.c
 rename crypto/{cipher-gcrypt.c => cipher-gcrypt.inc.c} (51%)
 delete mode 100644 crypto/cipher-nettle.c
 create mode 100644 crypto/cipher-nettle.inc.c

-- 
2.25.1



^ permalink raw reply	[flat|nested] 52+ messages in thread

* [PATCH 01/17] crypto: Move QCryptoCipher typedef to qemu/typedefs.h
  2020-08-13  3:25 [PATCH 00/17] crypto/cipher: Class hierarchy cleanups Richard Henderson
@ 2020-08-13  3:25 ` Richard Henderson
  2020-08-13  7:59   ` Philippe Mathieu-Daudé
  2020-08-17 16:48   ` Daniel P. Berrangé
  2020-08-13  3:25 ` [PATCH 02/17] crypto: Move QCryptoCipherDriver " Richard Henderson
                   ` (19 subsequent siblings)
  20 siblings, 2 replies; 52+ messages in thread
From: Richard Henderson @ 2020-08-13  3:25 UTC (permalink / raw)
  To: qemu-devel; +Cc: berrange

This allows header files to declare pointers without pulling
in the entire crypto subsystem.

Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
---
 include/crypto/cipher.h | 2 --
 include/qemu/typedefs.h | 1 +
 2 files changed, 1 insertion(+), 2 deletions(-)

diff --git a/include/crypto/cipher.h b/include/crypto/cipher.h
index 5928e5ecc7..95a0412911 100644
--- a/include/crypto/cipher.h
+++ b/include/crypto/cipher.h
@@ -23,8 +23,6 @@
 
 #include "qapi/qapi-types-crypto.h"
 
-typedef struct QCryptoCipher QCryptoCipher;
-
 /* See also "QCryptoCipherAlgorithm" and "QCryptoCipherMode"
  * enums defined in qapi/crypto.json */
 
diff --git a/include/qemu/typedefs.h b/include/qemu/typedefs.h
index 427027a970..45b9c57717 100644
--- a/include/qemu/typedefs.h
+++ b/include/qemu/typedefs.h
@@ -95,6 +95,7 @@ typedef struct PostcopyDiscardState PostcopyDiscardState;
 typedef struct Property Property;
 typedef struct PropertyInfo PropertyInfo;
 typedef struct QBool QBool;
+typedef struct QCryptoCipher QCryptoCipher;
 typedef struct QDict QDict;
 typedef struct QEMUBH QEMUBH;
 typedef struct QemuConsole QemuConsole;
-- 
2.25.1



^ permalink raw reply related	[flat|nested] 52+ messages in thread

* [PATCH 02/17] crypto: Move QCryptoCipherDriver typedef to qemu/typedefs.h
  2020-08-13  3:25 [PATCH 00/17] crypto/cipher: Class hierarchy cleanups Richard Henderson
  2020-08-13  3:25 ` [PATCH 01/17] crypto: Move QCryptoCipher typedef to qemu/typedefs.h Richard Henderson
@ 2020-08-13  3:25 ` Richard Henderson
  2020-08-13  7:59   ` Philippe Mathieu-Daudé
  2020-08-13  3:25 ` [PATCH 03/17] crypto: Assume blocksize is a power of 2 Richard Henderson
                   ` (18 subsequent siblings)
  20 siblings, 1 reply; 52+ messages in thread
From: Richard Henderson @ 2020-08-13  3:25 UTC (permalink / raw)
  To: qemu-devel; +Cc: berrange

This will allow the pointer to be used in crypto/cipher.h,
and not just in code using cipherpriv.h.

Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
---
 crypto/cipherpriv.h     | 2 --
 include/qemu/typedefs.h | 1 +
 2 files changed, 1 insertion(+), 2 deletions(-)

diff --git a/crypto/cipherpriv.h b/crypto/cipherpriv.h
index 0823239f41..9228c9fc3a 100644
--- a/crypto/cipherpriv.h
+++ b/crypto/cipherpriv.h
@@ -17,8 +17,6 @@
 
 #include "qapi/qapi-types-crypto.h"
 
-typedef struct QCryptoCipherDriver QCryptoCipherDriver;
-
 struct QCryptoCipherDriver {
     int (*cipher_encrypt)(QCryptoCipher *cipher,
                           const void *in,
diff --git a/include/qemu/typedefs.h b/include/qemu/typedefs.h
index 45b9c57717..d4ca469b6b 100644
--- a/include/qemu/typedefs.h
+++ b/include/qemu/typedefs.h
@@ -96,6 +96,7 @@ typedef struct Property Property;
 typedef struct PropertyInfo PropertyInfo;
 typedef struct QBool QBool;
 typedef struct QCryptoCipher QCryptoCipher;
+typedef struct QCryptoCipherDriver QCryptoCipherDriver;
 typedef struct QDict QDict;
 typedef struct QEMUBH QEMUBH;
 typedef struct QemuConsole QemuConsole;
-- 
2.25.1



^ permalink raw reply related	[flat|nested] 52+ messages in thread

* [PATCH 03/17] crypto: Assume blocksize is a power of 2
  2020-08-13  3:25 [PATCH 00/17] crypto/cipher: Class hierarchy cleanups Richard Henderson
  2020-08-13  3:25 ` [PATCH 01/17] crypto: Move QCryptoCipher typedef to qemu/typedefs.h Richard Henderson
  2020-08-13  3:25 ` [PATCH 02/17] crypto: Move QCryptoCipherDriver " Richard Henderson
@ 2020-08-13  3:25 ` Richard Henderson
  2020-08-17 16:51   ` Daniel P. Berrangé
  2020-08-13  3:25 ` [PATCH 04/17] crypto: Rename cipher include files to .inc.c Richard Henderson
                   ` (17 subsequent siblings)
  20 siblings, 1 reply; 52+ messages in thread
From: Richard Henderson @ 2020-08-13  3:25 UTC (permalink / raw)
  To: qemu-devel; +Cc: berrange

The check in the encode/decode path using full division has a
noticeable amount of overhead.  By asserting the blocksize is
a power of 2, we can reduce this check to a mask.

Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
---
 crypto/cipher-builtin.c | 4 ++--
 crypto/cipher-gcrypt.c  | 5 +++--
 crypto/cipher-nettle.c  | 5 +++--
 crypto/cipher.c         | 1 +
 4 files changed, 9 insertions(+), 6 deletions(-)

diff --git a/crypto/cipher-builtin.c b/crypto/cipher-builtin.c
index 35cf7820d9..6eafd39da0 100644
--- a/crypto/cipher-builtin.c
+++ b/crypto/cipher-builtin.c
@@ -484,7 +484,7 @@ qcrypto_builtin_cipher_encrypt(QCryptoCipher *cipher,
 {
     QCryptoCipherBuiltin *ctxt = cipher->opaque;
 
-    if (len % ctxt->blocksize) {
+    if (len & (ctxt->blocksize - 1)) {
         error_setg(errp, "Length %zu must be a multiple of block size %zu",
                    len, ctxt->blocksize);
         return -1;
@@ -503,7 +503,7 @@ qcrypto_builtin_cipher_decrypt(QCryptoCipher *cipher,
 {
     QCryptoCipherBuiltin *ctxt = cipher->opaque;
 
-    if (len % ctxt->blocksize) {
+    if (len & (ctxt->blocksize - 1)) {
         error_setg(errp, "Length %zu must be a multiple of block size %zu",
                    len, ctxt->blocksize);
         return -1;
diff --git a/crypto/cipher-gcrypt.c b/crypto/cipher-gcrypt.c
index 2864099527..81e4745bff 100644
--- a/crypto/cipher-gcrypt.c
+++ b/crypto/cipher-gcrypt.c
@@ -245,6 +245,7 @@ static QCryptoCipherGcrypt *qcrypto_cipher_ctx_new(QCryptoCipherAlgorithm alg,
             g_assert_not_reached();
         }
     }
+    g_assert(is_power_of_2(ctx->blocksize));
 
 #ifdef CONFIG_QEMU_PRIVATE_XTS
     if (mode == QCRYPTO_CIPHER_MODE_XTS) {
@@ -305,7 +306,7 @@ qcrypto_gcrypt_cipher_encrypt(QCryptoCipher *cipher,
     QCryptoCipherGcrypt *ctx = cipher->opaque;
     gcry_error_t err;
 
-    if (len % ctx->blocksize) {
+    if (len & (ctx->blocksize - 1)) {
         error_setg(errp, "Length %zu must be a multiple of block size %zu",
                    len, ctx->blocksize);
         return -1;
@@ -344,7 +345,7 @@ qcrypto_gcrypt_cipher_decrypt(QCryptoCipher *cipher,
     QCryptoCipherGcrypt *ctx = cipher->opaque;
     gcry_error_t err;
 
-    if (len % ctx->blocksize) {
+    if (len & (ctx->blocksize - 1)) {
         error_setg(errp, "Length %zu must be a multiple of block size %zu",
                    len, ctx->blocksize);
         return -1;
diff --git a/crypto/cipher-nettle.c b/crypto/cipher-nettle.c
index 7e9a4cc199..0677fdfd33 100644
--- a/crypto/cipher-nettle.c
+++ b/crypto/cipher-nettle.c
@@ -576,6 +576,7 @@ static QCryptoCipherNettle *qcrypto_cipher_ctx_new(QCryptoCipherAlgorithm alg,
                    QCryptoCipherAlgorithm_str(alg));
         goto error;
     }
+    g_assert(is_power_of_2(ctx->blocksize));
 
     if (mode == QCRYPTO_CIPHER_MODE_XTS &&
         ctx->blocksize != XTS_BLOCK_SIZE) {
@@ -613,7 +614,7 @@ qcrypto_nettle_cipher_encrypt(QCryptoCipher *cipher,
 {
     QCryptoCipherNettle *ctx = cipher->opaque;
 
-    if (len % ctx->blocksize) {
+    if (len & (ctx->blocksize - 1)) {
         error_setg(errp, "Length %zu must be a multiple of block size %zu",
                    len, ctx->blocksize);
         return -1;
@@ -666,7 +667,7 @@ qcrypto_nettle_cipher_decrypt(QCryptoCipher *cipher,
 {
     QCryptoCipherNettle *ctx = cipher->opaque;
 
-    if (len % ctx->blocksize) {
+    if (len & (ctx->blocksize - 1)) {
         error_setg(errp, "Length %zu must be a multiple of block size %zu",
                    len, ctx->blocksize);
         return -1;
diff --git a/crypto/cipher.c b/crypto/cipher.c
index e5adb56271..2722dc7d87 100644
--- a/crypto/cipher.c
+++ b/crypto/cipher.c
@@ -19,6 +19,7 @@
  */
 
 #include "qemu/osdep.h"
+#include "qemu/host-utils.h"
 #include "qapi/error.h"
 #include "crypto/cipher.h"
 #include "cipherpriv.h"
-- 
2.25.1



^ permalink raw reply related	[flat|nested] 52+ messages in thread

* [PATCH 04/17] crypto: Rename cipher include files to .inc.c
  2020-08-13  3:25 [PATCH 00/17] crypto/cipher: Class hierarchy cleanups Richard Henderson
                   ` (2 preceding siblings ...)
  2020-08-13  3:25 ` [PATCH 03/17] crypto: Assume blocksize is a power of 2 Richard Henderson
@ 2020-08-13  3:25 ` Richard Henderson
  2020-08-13  8:00   ` Philippe Mathieu-Daudé
  2020-08-13  3:25 ` [PATCH 05/17] crypto: Remove redundant includes Richard Henderson
                   ` (16 subsequent siblings)
  20 siblings, 1 reply; 52+ messages in thread
From: Richard Henderson @ 2020-08-13  3:25 UTC (permalink / raw)
  To: qemu-devel; +Cc: berrange

QEMU standard procedure for included c files is to use *.inc.c.
E.g. there are a different set of checks that are applied.

Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
---
 crypto/{cipher-builtin.c => cipher-builtin.inc.c} | 0
 crypto/{cipher-gcrypt.c => cipher-gcrypt.inc.c}   | 0
 crypto/{cipher-nettle.c => cipher-nettle.inc.c}   | 0
 crypto/cipher.c                                   | 6 +++---
 4 files changed, 3 insertions(+), 3 deletions(-)
 rename crypto/{cipher-builtin.c => cipher-builtin.inc.c} (100%)
 rename crypto/{cipher-gcrypt.c => cipher-gcrypt.inc.c} (100%)
 rename crypto/{cipher-nettle.c => cipher-nettle.inc.c} (100%)

diff --git a/crypto/cipher-builtin.c b/crypto/cipher-builtin.inc.c
similarity index 100%
rename from crypto/cipher-builtin.c
rename to crypto/cipher-builtin.inc.c
diff --git a/crypto/cipher-gcrypt.c b/crypto/cipher-gcrypt.inc.c
similarity index 100%
rename from crypto/cipher-gcrypt.c
rename to crypto/cipher-gcrypt.inc.c
diff --git a/crypto/cipher-nettle.c b/crypto/cipher-nettle.inc.c
similarity index 100%
rename from crypto/cipher-nettle.c
rename to crypto/cipher-nettle.inc.c
diff --git a/crypto/cipher.c b/crypto/cipher.c
index 2722dc7d87..deae82c264 100644
--- a/crypto/cipher.c
+++ b/crypto/cipher.c
@@ -151,11 +151,11 @@ qcrypto_cipher_munge_des_rfb_key(const uint8_t *key,
 #endif /* CONFIG_GCRYPT || CONFIG_NETTLE */
 
 #ifdef CONFIG_GCRYPT
-#include "cipher-gcrypt.c"
+#include "cipher-gcrypt.inc.c"
 #elif defined CONFIG_NETTLE
-#include "cipher-nettle.c"
+#include "cipher-nettle.inc.c"
 #else
-#include "cipher-builtin.c"
+#include "cipher-builtin.inc.c"
 #endif
 
 QCryptoCipher *qcrypto_cipher_new(QCryptoCipherAlgorithm alg,
-- 
2.25.1



^ permalink raw reply related	[flat|nested] 52+ messages in thread

* [PATCH 05/17] crypto: Remove redundant includes
  2020-08-13  3:25 [PATCH 00/17] crypto/cipher: Class hierarchy cleanups Richard Henderson
                   ` (3 preceding siblings ...)
  2020-08-13  3:25 ` [PATCH 04/17] crypto: Rename cipher include files to .inc.c Richard Henderson
@ 2020-08-13  3:25 ` Richard Henderson
  2020-08-13  8:01   ` Philippe Mathieu-Daudé
  2020-08-17 16:50   ` Daniel P. Berrangé
  2020-08-13  3:25 ` [PATCH 06/17] crypto/nettle: Fix xts_encrypt arguments Richard Henderson
                   ` (15 subsequent siblings)
  20 siblings, 2 replies; 52+ messages in thread
From: Richard Henderson @ 2020-08-13  3:25 UTC (permalink / raw)
  To: qemu-devel; +Cc: berrange

Both qemu/osdep.h and cipherpriv.h have already been
included by the parent cipher.c.

Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
---
 crypto/cipher-builtin.inc.c | 2 --
 crypto/cipher-gcrypt.inc.c  | 2 --
 crypto/cipher-nettle.inc.c  | 2 --
 3 files changed, 6 deletions(-)

diff --git a/crypto/cipher-builtin.inc.c b/crypto/cipher-builtin.inc.c
index 6eafd39da0..56d45b0227 100644
--- a/crypto/cipher-builtin.inc.c
+++ b/crypto/cipher-builtin.inc.c
@@ -18,11 +18,9 @@
  *
  */
 
-#include "qemu/osdep.h"
 #include "crypto/aes.h"
 #include "crypto/desrfb.h"
 #include "crypto/xts.h"
-#include "cipherpriv.h"
 
 typedef struct QCryptoCipherBuiltinAESContext QCryptoCipherBuiltinAESContext;
 struct QCryptoCipherBuiltinAESContext {
diff --git a/crypto/cipher-gcrypt.inc.c b/crypto/cipher-gcrypt.inc.c
index 81e4745bff..a62839914b 100644
--- a/crypto/cipher-gcrypt.inc.c
+++ b/crypto/cipher-gcrypt.inc.c
@@ -18,11 +18,9 @@
  *
  */
 
-#include "qemu/osdep.h"
 #ifdef CONFIG_QEMU_PRIVATE_XTS
 #include "crypto/xts.h"
 #endif
-#include "cipherpriv.h"
 
 #include <gcrypt.h>
 
diff --git a/crypto/cipher-nettle.inc.c b/crypto/cipher-nettle.inc.c
index 0677fdfd33..256931a823 100644
--- a/crypto/cipher-nettle.inc.c
+++ b/crypto/cipher-nettle.inc.c
@@ -18,11 +18,9 @@
  *
  */
 
-#include "qemu/osdep.h"
 #ifdef CONFIG_QEMU_PRIVATE_XTS
 #include "crypto/xts.h"
 #endif
-#include "cipherpriv.h"
 
 #include <nettle/nettle-types.h>
 #include <nettle/aes.h>
-- 
2.25.1



^ permalink raw reply related	[flat|nested] 52+ messages in thread

* [PATCH 06/17] crypto/nettle: Fix xts_encrypt arguments
  2020-08-13  3:25 [PATCH 00/17] crypto/cipher: Class hierarchy cleanups Richard Henderson
                   ` (4 preceding siblings ...)
  2020-08-13  3:25 ` [PATCH 05/17] crypto: Remove redundant includes Richard Henderson
@ 2020-08-13  3:25 ` Richard Henderson
  2020-08-17 16:53   ` Daniel P. Berrangé
  2020-08-13  3:25 ` [PATCH 07/17] crypto: Use the correct const type for driver Richard Henderson
                   ` (14 subsequent siblings)
  20 siblings, 1 reply; 52+ messages in thread
From: Richard Henderson @ 2020-08-13  3:25 UTC (permalink / raw)
  To: qemu-devel; +Cc: berrange

The fourth argument to xts_encrypt should be the decrypt
callback; we were accidentally passing encrypt twice.

Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
---
 crypto/cipher-nettle.inc.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/crypto/cipher-nettle.inc.c b/crypto/cipher-nettle.inc.c
index 256931a823..0404cfc6da 100644
--- a/crypto/cipher-nettle.inc.c
+++ b/crypto/cipher-nettle.inc.c
@@ -632,7 +632,7 @@ qcrypto_nettle_cipher_encrypt(QCryptoCipher *cipher,
     case QCRYPTO_CIPHER_MODE_XTS:
 #ifdef CONFIG_QEMU_PRIVATE_XTS
         xts_encrypt(ctx->ctx, ctx->ctx_tweak,
-                    ctx->alg_encrypt_wrapper, ctx->alg_encrypt_wrapper,
+                    ctx->alg_encrypt_wrapper, ctx->alg_decrypt_wrapper,
                     ctx->iv, len, out, in);
 #else
         xts_encrypt_message(ctx->ctx, ctx->ctx_tweak,
-- 
2.25.1



^ permalink raw reply related	[flat|nested] 52+ messages in thread

* [PATCH 07/17] crypto: Use the correct const type for driver
  2020-08-13  3:25 [PATCH 00/17] crypto/cipher: Class hierarchy cleanups Richard Henderson
                   ` (5 preceding siblings ...)
  2020-08-13  3:25 ` [PATCH 06/17] crypto/nettle: Fix xts_encrypt arguments Richard Henderson
@ 2020-08-13  3:25 ` Richard Henderson
  2020-08-13  8:02   ` Philippe Mathieu-Daudé
  2020-08-17 16:53   ` Daniel P. Berrangé
  2020-08-13  3:25 ` [PATCH 08/17] crypto: Allocate QCryptoCipher with the subclass Richard Henderson
                   ` (13 subsequent siblings)
  20 siblings, 2 replies; 52+ messages in thread
From: Richard Henderson @ 2020-08-13  3:25 UTC (permalink / raw)
  To: qemu-devel; +Cc: berrange

This allows the in memory structures to be read-only.

Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
---
 crypto/cipherpriv.h         |  2 +-
 include/crypto/cipher.h     |  2 +-
 crypto/cipher-afalg.c       |  2 +-
 crypto/cipher-builtin.inc.c |  2 +-
 crypto/cipher-gcrypt.inc.c  |  2 +-
 crypto/cipher-nettle.inc.c  |  2 +-
 crypto/cipher.c             | 12 ++++++------
 7 files changed, 12 insertions(+), 12 deletions(-)

diff --git a/crypto/cipherpriv.h b/crypto/cipherpriv.h
index 9228c9fc3a..b73be33bd2 100644
--- a/crypto/cipherpriv.h
+++ b/crypto/cipherpriv.h
@@ -47,7 +47,7 @@ qcrypto_afalg_cipher_ctx_new(QCryptoCipherAlgorithm alg,
                              const uint8_t *key,
                              size_t nkey, Error **errp);
 
-extern struct QCryptoCipherDriver qcrypto_cipher_afalg_driver;
+extern const struct QCryptoCipherDriver qcrypto_cipher_afalg_driver;
 
 #endif
 
diff --git a/include/crypto/cipher.h b/include/crypto/cipher.h
index 95a0412911..022a8d1157 100644
--- a/include/crypto/cipher.h
+++ b/include/crypto/cipher.h
@@ -78,7 +78,7 @@ struct QCryptoCipher {
     QCryptoCipherAlgorithm alg;
     QCryptoCipherMode mode;
     void *opaque;
-    void *driver;
+    const QCryptoCipherDriver *driver;
 };
 
 /**
diff --git a/crypto/cipher-afalg.c b/crypto/cipher-afalg.c
index cd72284690..5c7c44761b 100644
--- a/crypto/cipher-afalg.c
+++ b/crypto/cipher-afalg.c
@@ -218,7 +218,7 @@ static void qcrypto_afalg_comm_ctx_free(QCryptoCipher *cipher)
     qcrypto_afalg_comm_free(cipher->opaque);
 }
 
-struct QCryptoCipherDriver qcrypto_cipher_afalg_driver = {
+const struct QCryptoCipherDriver qcrypto_cipher_afalg_driver = {
     .cipher_encrypt = qcrypto_afalg_cipher_encrypt,
     .cipher_decrypt = qcrypto_afalg_cipher_decrypt,
     .cipher_setiv = qcrypto_afalg_cipher_setiv,
diff --git a/crypto/cipher-builtin.inc.c b/crypto/cipher-builtin.inc.c
index 56d45b0227..156f32f1c7 100644
--- a/crypto/cipher-builtin.inc.c
+++ b/crypto/cipher-builtin.inc.c
@@ -522,7 +522,7 @@ qcrypto_builtin_cipher_setiv(QCryptoCipher *cipher,
 }
 
 
-static struct QCryptoCipherDriver qcrypto_cipher_lib_driver = {
+static const struct QCryptoCipherDriver qcrypto_cipher_lib_driver = {
     .cipher_encrypt = qcrypto_builtin_cipher_encrypt,
     .cipher_decrypt = qcrypto_builtin_cipher_decrypt,
     .cipher_setiv = qcrypto_builtin_cipher_setiv,
diff --git a/crypto/cipher-gcrypt.inc.c b/crypto/cipher-gcrypt.inc.c
index a62839914b..18850fadb9 100644
--- a/crypto/cipher-gcrypt.inc.c
+++ b/crypto/cipher-gcrypt.inc.c
@@ -413,7 +413,7 @@ qcrypto_gcrypt_cipher_setiv(QCryptoCipher *cipher,
 }
 
 
-static struct QCryptoCipherDriver qcrypto_cipher_lib_driver = {
+static const struct QCryptoCipherDriver qcrypto_cipher_lib_driver = {
     .cipher_encrypt = qcrypto_gcrypt_cipher_encrypt,
     .cipher_decrypt = qcrypto_gcrypt_cipher_decrypt,
     .cipher_setiv = qcrypto_gcrypt_cipher_setiv,
diff --git a/crypto/cipher-nettle.inc.c b/crypto/cipher-nettle.inc.c
index 0404cfc6da..6ecce5e8ea 100644
--- a/crypto/cipher-nettle.inc.c
+++ b/crypto/cipher-nettle.inc.c
@@ -724,7 +724,7 @@ qcrypto_nettle_cipher_setiv(QCryptoCipher *cipher,
 }
 
 
-static struct QCryptoCipherDriver qcrypto_cipher_lib_driver = {
+static const struct QCryptoCipherDriver qcrypto_cipher_lib_driver = {
     .cipher_encrypt = qcrypto_nettle_cipher_encrypt,
     .cipher_decrypt = qcrypto_nettle_cipher_decrypt,
     .cipher_setiv = qcrypto_nettle_cipher_setiv,
diff --git a/crypto/cipher.c b/crypto/cipher.c
index deae82c264..d3ef856009 100644
--- a/crypto/cipher.c
+++ b/crypto/cipher.c
@@ -165,7 +165,7 @@ QCryptoCipher *qcrypto_cipher_new(QCryptoCipherAlgorithm alg,
 {
     QCryptoCipher *cipher;
     void *ctx = NULL;
-    QCryptoCipherDriver *drv = NULL;
+    const QCryptoCipherDriver *drv = NULL;
 
 #ifdef CONFIG_AF_ALG
     ctx = qcrypto_afalg_cipher_ctx_new(alg, mode, key, nkey, NULL);
@@ -187,7 +187,7 @@ QCryptoCipher *qcrypto_cipher_new(QCryptoCipherAlgorithm alg,
     cipher->alg = alg;
     cipher->mode = mode;
     cipher->opaque = ctx;
-    cipher->driver = (void *)drv;
+    cipher->driver = drv;
 
     return cipher;
 }
@@ -199,7 +199,7 @@ int qcrypto_cipher_encrypt(QCryptoCipher *cipher,
                            size_t len,
                            Error **errp)
 {
-    QCryptoCipherDriver *drv = cipher->driver;
+    const QCryptoCipherDriver *drv = cipher->driver;
     return drv->cipher_encrypt(cipher, in, out, len, errp);
 }
 
@@ -210,7 +210,7 @@ int qcrypto_cipher_decrypt(QCryptoCipher *cipher,
                            size_t len,
                            Error **errp)
 {
-    QCryptoCipherDriver *drv = cipher->driver;
+    const QCryptoCipherDriver *drv = cipher->driver;
     return drv->cipher_decrypt(cipher, in, out, len, errp);
 }
 
@@ -219,14 +219,14 @@ int qcrypto_cipher_setiv(QCryptoCipher *cipher,
                          const uint8_t *iv, size_t niv,
                          Error **errp)
 {
-    QCryptoCipherDriver *drv = cipher->driver;
+    const QCryptoCipherDriver *drv = cipher->driver;
     return drv->cipher_setiv(cipher, iv, niv, errp);
 }
 
 
 void qcrypto_cipher_free(QCryptoCipher *cipher)
 {
-    QCryptoCipherDriver *drv;
+    const QCryptoCipherDriver *drv;
     if (cipher) {
         drv = cipher->driver;
         drv->cipher_free(cipher);
-- 
2.25.1



^ permalink raw reply related	[flat|nested] 52+ messages in thread

* [PATCH 08/17] crypto: Allocate QCryptoCipher with the subclass
  2020-08-13  3:25 [PATCH 00/17] crypto/cipher: Class hierarchy cleanups Richard Henderson
                   ` (6 preceding siblings ...)
  2020-08-13  3:25 ` [PATCH 07/17] crypto: Use the correct const type for driver Richard Henderson
@ 2020-08-13  3:25 ` Richard Henderson
  2020-08-17 16:55   ` Daniel P. Berrangé
  2020-08-13  3:25 ` [PATCH 09/17] crypto: Move cipher->driver init to qcrypto_*_cipher_ctx_new Richard Henderson
                   ` (12 subsequent siblings)
  20 siblings, 1 reply; 52+ messages in thread
From: Richard Henderson @ 2020-08-13  3:25 UTC (permalink / raw)
  To: qemu-devel; +Cc: berrange

Merge the allocation of "opaque" into the allocation of "cipher".
This is step one in reducing the indirection in these classes.

Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
---
 crypto/afalgpriv.h          |  3 ++
 crypto/cipherpriv.h         |  2 +-
 include/crypto/cipher.h     |  1 -
 crypto/cipher-afalg.c       | 20 ++++++-----
 crypto/cipher-builtin.inc.c | 68 +++++++++++++++++++------------------
 crypto/cipher-gcrypt.inc.c  | 23 +++++++------
 crypto/cipher-nettle.inc.c  | 24 +++++++------
 crypto/cipher.c             | 20 ++++-------
 8 files changed, 84 insertions(+), 77 deletions(-)

diff --git a/crypto/afalgpriv.h b/crypto/afalgpriv.h
index f6550b5c51..5a2393f1b7 100644
--- a/crypto/afalgpriv.h
+++ b/crypto/afalgpriv.h
@@ -15,6 +15,7 @@
 #define QCRYPTO_AFALGPRIV_H
 
 #include <linux/if_alg.h>
+#include "crypto/cipher.h"
 
 #define SALG_TYPE_LEN_MAX 14
 #define SALG_NAME_LEN_MAX 64
@@ -32,6 +33,8 @@
 typedef struct QCryptoAFAlg QCryptoAFAlg;
 
 struct QCryptoAFAlg {
+    QCryptoCipher base;
+
     int tfmfd;
     int opfd;
     struct msghdr *msg;
diff --git a/crypto/cipherpriv.h b/crypto/cipherpriv.h
index b73be33bd2..437b109b5e 100644
--- a/crypto/cipherpriv.h
+++ b/crypto/cipherpriv.h
@@ -41,7 +41,7 @@ struct QCryptoCipherDriver {
 
 #include "afalgpriv.h"
 
-extern QCryptoAFAlg *
+extern QCryptoCipher *
 qcrypto_afalg_cipher_ctx_new(QCryptoCipherAlgorithm alg,
                              QCryptoCipherMode mode,
                              const uint8_t *key,
diff --git a/include/crypto/cipher.h b/include/crypto/cipher.h
index 022a8d1157..56377c80fc 100644
--- a/include/crypto/cipher.h
+++ b/include/crypto/cipher.h
@@ -77,7 +77,6 @@
 struct QCryptoCipher {
     QCryptoCipherAlgorithm alg;
     QCryptoCipherMode mode;
-    void *opaque;
     const QCryptoCipherDriver *driver;
 };
 
diff --git a/crypto/cipher-afalg.c b/crypto/cipher-afalg.c
index 5c7c44761b..86e5249bd6 100644
--- a/crypto/cipher-afalg.c
+++ b/crypto/cipher-afalg.c
@@ -58,7 +58,7 @@ qcrypto_afalg_cipher_format_name(QCryptoCipherAlgorithm alg,
     return name;
 }
 
-QCryptoAFAlg *
+QCryptoCipher *
 qcrypto_afalg_cipher_ctx_new(QCryptoCipherAlgorithm alg,
                              QCryptoCipherMode mode,
                              const uint8_t *key,
@@ -109,7 +109,7 @@ qcrypto_afalg_cipher_ctx_new(QCryptoCipherAlgorithm alg,
     }
     afalg->cmsg = CMSG_FIRSTHDR(afalg->msg);
 
-    return afalg;
+    return &afalg->base;
 }
 
 static int
@@ -117,9 +117,9 @@ qcrypto_afalg_cipher_setiv(QCryptoCipher *cipher,
                            const uint8_t *iv,
                            size_t niv, Error **errp)
 {
+    QCryptoAFAlg *afalg = container_of(cipher, QCryptoAFAlg, base);
     struct af_alg_iv *alg_iv;
     size_t expect_niv;
-    QCryptoAFAlg *afalg = cipher->opaque;
 
     expect_niv = qcrypto_cipher_get_iv_len(cipher->alg, cipher->mode);
     if (niv != expect_niv) {
@@ -200,8 +200,9 @@ qcrypto_afalg_cipher_encrypt(QCryptoCipher *cipher,
                              const void *in, void *out,
                              size_t len, Error **errp)
 {
-    return qcrypto_afalg_cipher_op(cipher->opaque, in, out,
-                                   len, true, errp);
+    QCryptoAFAlg *afalg = container_of(cipher, QCryptoAFAlg, base);
+
+    return qcrypto_afalg_cipher_op(afalg, in, out, len, true, errp);
 }
 
 static int
@@ -209,13 +210,16 @@ qcrypto_afalg_cipher_decrypt(QCryptoCipher *cipher,
                              const void *in, void *out,
                              size_t len, Error **errp)
 {
-    return qcrypto_afalg_cipher_op(cipher->opaque, in, out,
-                                   len, false, errp);
+    QCryptoAFAlg *afalg = container_of(cipher, QCryptoAFAlg, base);
+
+    return qcrypto_afalg_cipher_op(afalg, in, out, len, false, errp);
 }
 
 static void qcrypto_afalg_comm_ctx_free(QCryptoCipher *cipher)
 {
-    qcrypto_afalg_comm_free(cipher->opaque);
+    QCryptoAFAlg *afalg = container_of(cipher, QCryptoAFAlg, base);
+
+    qcrypto_afalg_comm_free(afalg);
 }
 
 const struct QCryptoCipherDriver qcrypto_cipher_afalg_driver = {
diff --git a/crypto/cipher-builtin.inc.c b/crypto/cipher-builtin.inc.c
index 156f32f1c7..6a03e23040 100644
--- a/crypto/cipher-builtin.inc.c
+++ b/crypto/cipher-builtin.inc.c
@@ -41,6 +41,8 @@ struct QCryptoCipherBuiltinDESRFB {
 
 typedef struct QCryptoCipherBuiltin QCryptoCipherBuiltin;
 struct QCryptoCipherBuiltin {
+    QCryptoCipher base;
+
     union {
         QCryptoCipherBuiltinAES aes;
         QCryptoCipherBuiltinDESRFB desrfb;
@@ -65,10 +67,7 @@ struct QCryptoCipherBuiltin {
 
 static void qcrypto_cipher_free_aes(QCryptoCipher *cipher)
 {
-    QCryptoCipherBuiltin *ctxt = cipher->opaque;
-
-    g_free(ctxt);
-    cipher->opaque = NULL;
+    g_free(cipher);
 }
 
 
@@ -152,7 +151,8 @@ static int qcrypto_cipher_encrypt_aes(QCryptoCipher *cipher,
                                       size_t len,
                                       Error **errp)
 {
-    QCryptoCipherBuiltin *ctxt = cipher->opaque;
+    QCryptoCipherBuiltin *ctxt
+        = container_of(cipher, QCryptoCipherBuiltin, base);
 
     switch (cipher->mode) {
     case QCRYPTO_CIPHER_MODE_ECB:
@@ -186,7 +186,8 @@ static int qcrypto_cipher_decrypt_aes(QCryptoCipher *cipher,
                                       size_t len,
                                       Error **errp)
 {
-    QCryptoCipherBuiltin *ctxt = cipher->opaque;
+    QCryptoCipherBuiltin *ctxt
+        = container_of(cipher, QCryptoCipherBuiltin, base);
 
     switch (cipher->mode) {
     case QCRYPTO_CIPHER_MODE_ECB:
@@ -217,7 +218,9 @@ static int qcrypto_cipher_setiv_aes(QCryptoCipher *cipher,
                                      const uint8_t *iv, size_t niv,
                                      Error **errp)
 {
-    QCryptoCipherBuiltin *ctxt = cipher->opaque;
+    QCryptoCipherBuiltin *ctxt
+        = container_of(cipher, QCryptoCipherBuiltin, base);
+
     if (niv != AES_BLOCK_SIZE) {
         error_setg(errp, "IV must be %d bytes not %zu",
                    AES_BLOCK_SIZE, niv);
@@ -232,7 +235,7 @@ static int qcrypto_cipher_setiv_aes(QCryptoCipher *cipher,
 
 
 
-static QCryptoCipherBuiltin *
+static QCryptoCipher *
 qcrypto_cipher_init_aes(QCryptoCipherMode mode,
                         const uint8_t *key, size_t nkey,
                         Error **errp)
@@ -289,7 +292,7 @@ qcrypto_cipher_init_aes(QCryptoCipherMode mode,
     ctxt->encrypt = qcrypto_cipher_encrypt_aes;
     ctxt->decrypt = qcrypto_cipher_decrypt_aes;
 
-    return ctxt;
+    return &ctxt->base;
 
  error:
     g_free(ctxt);
@@ -299,11 +302,11 @@ qcrypto_cipher_init_aes(QCryptoCipherMode mode,
 
 static void qcrypto_cipher_free_des_rfb(QCryptoCipher *cipher)
 {
-    QCryptoCipherBuiltin *ctxt = cipher->opaque;
+    QCryptoCipherBuiltin *ctxt
+        = container_of(cipher, QCryptoCipherBuiltin, base);
 
     g_free(ctxt->state.desrfb.key);
     g_free(ctxt);
-    cipher->opaque = NULL;
 }
 
 
@@ -313,7 +316,8 @@ static int qcrypto_cipher_encrypt_des_rfb(QCryptoCipher *cipher,
                                           size_t len,
                                           Error **errp)
 {
-    QCryptoCipherBuiltin *ctxt = cipher->opaque;
+    QCryptoCipherBuiltin *ctxt
+        = container_of(cipher, QCryptoCipherBuiltin, base);
     size_t i;
 
     if (len % 8) {
@@ -338,7 +342,8 @@ static int qcrypto_cipher_decrypt_des_rfb(QCryptoCipher *cipher,
                                           size_t len,
                                           Error **errp)
 {
-    QCryptoCipherBuiltin *ctxt = cipher->opaque;
+    QCryptoCipherBuiltin *ctxt
+        = container_of(cipher, QCryptoCipherBuiltin, base);
     size_t i;
 
     if (len % 8) {
@@ -366,7 +371,7 @@ static int qcrypto_cipher_setiv_des_rfb(QCryptoCipher *cipher,
 }
 
 
-static QCryptoCipherBuiltin *
+static QCryptoCipher *
 qcrypto_cipher_init_des_rfb(QCryptoCipherMode mode,
                             const uint8_t *key, size_t nkey,
                             Error **errp)
@@ -391,7 +396,7 @@ qcrypto_cipher_init_des_rfb(QCryptoCipherMode mode,
     ctxt->encrypt = qcrypto_cipher_encrypt_des_rfb;
     ctxt->decrypt = qcrypto_cipher_decrypt_des_rfb;
 
-    return ctxt;
+    return &ctxt->base;
 }
 
 
@@ -421,14 +426,12 @@ bool qcrypto_cipher_supports(QCryptoCipherAlgorithm alg,
 }
 
 
-static QCryptoCipherBuiltin *qcrypto_cipher_ctx_new(QCryptoCipherAlgorithm alg,
-                                                    QCryptoCipherMode mode,
-                                                    const uint8_t *key,
-                                                    size_t nkey,
-                                                    Error **errp)
+static QCryptoCipher *qcrypto_cipher_ctx_new(QCryptoCipherAlgorithm alg,
+                                             QCryptoCipherMode mode,
+                                             const uint8_t *key,
+                                             size_t nkey,
+                                             Error **errp)
 {
-    QCryptoCipherBuiltin *ctxt;
-
     switch (mode) {
     case QCRYPTO_CIPHER_MODE_ECB:
     case QCRYPTO_CIPHER_MODE_CBC:
@@ -446,29 +449,25 @@ static QCryptoCipherBuiltin *qcrypto_cipher_ctx_new(QCryptoCipherAlgorithm alg,
 
     switch (alg) {
     case QCRYPTO_CIPHER_ALG_DES_RFB:
-        ctxt = qcrypto_cipher_init_des_rfb(mode, key, nkey, errp);
-        break;
+        return qcrypto_cipher_init_des_rfb(mode, key, nkey, errp);
     case QCRYPTO_CIPHER_ALG_AES_128:
     case QCRYPTO_CIPHER_ALG_AES_192:
     case QCRYPTO_CIPHER_ALG_AES_256:
-        ctxt = qcrypto_cipher_init_aes(mode, key, nkey, errp);
-        break;
+        return qcrypto_cipher_init_aes(mode, key, nkey, errp);
     default:
         error_setg(errp,
                    "Unsupported cipher algorithm %s",
                    QCryptoCipherAlgorithm_str(alg));
         return NULL;
     }
-
-    return ctxt;
 }
 
 static void
 qcrypto_builtin_cipher_ctx_free(QCryptoCipher *cipher)
 {
-    QCryptoCipherBuiltin *ctxt;
+    QCryptoCipherBuiltin *ctxt
+        = container_of(cipher, QCryptoCipherBuiltin, base);
 
-    ctxt = cipher->opaque;
     ctxt->free(cipher);
 }
 
@@ -480,7 +479,8 @@ qcrypto_builtin_cipher_encrypt(QCryptoCipher *cipher,
                                size_t len,
                                Error **errp)
 {
-    QCryptoCipherBuiltin *ctxt = cipher->opaque;
+    QCryptoCipherBuiltin *ctxt
+        = container_of(cipher, QCryptoCipherBuiltin, base);
 
     if (len & (ctxt->blocksize - 1)) {
         error_setg(errp, "Length %zu must be a multiple of block size %zu",
@@ -499,7 +499,8 @@ qcrypto_builtin_cipher_decrypt(QCryptoCipher *cipher,
                                size_t len,
                                Error **errp)
 {
-    QCryptoCipherBuiltin *ctxt = cipher->opaque;
+    QCryptoCipherBuiltin *ctxt
+        = container_of(cipher, QCryptoCipherBuiltin, base);
 
     if (len & (ctxt->blocksize - 1)) {
         error_setg(errp, "Length %zu must be a multiple of block size %zu",
@@ -516,7 +517,8 @@ qcrypto_builtin_cipher_setiv(QCryptoCipher *cipher,
                              const uint8_t *iv, size_t niv,
                              Error **errp)
 {
-    QCryptoCipherBuiltin *ctxt = cipher->opaque;
+    QCryptoCipherBuiltin *ctxt
+        = container_of(cipher, QCryptoCipherBuiltin, base);
 
     return ctxt->setiv(cipher, iv, niv, errp);
 }
diff --git a/crypto/cipher-gcrypt.inc.c b/crypto/cipher-gcrypt.inc.c
index 18850fadb9..3b3c85e265 100644
--- a/crypto/cipher-gcrypt.inc.c
+++ b/crypto/cipher-gcrypt.inc.c
@@ -58,6 +58,7 @@ bool qcrypto_cipher_supports(QCryptoCipherAlgorithm alg,
 
 typedef struct QCryptoCipherGcrypt QCryptoCipherGcrypt;
 struct QCryptoCipherGcrypt {
+    QCryptoCipher base;
     gcry_cipher_hd_t handle;
     size_t blocksize;
 #ifdef CONFIG_QEMU_PRIVATE_XTS
@@ -86,11 +87,11 @@ qcrypto_gcrypt_cipher_free_ctx(QCryptoCipherGcrypt *ctx,
 }
 
 
-static QCryptoCipherGcrypt *qcrypto_cipher_ctx_new(QCryptoCipherAlgorithm alg,
-                                                   QCryptoCipherMode mode,
-                                                   const uint8_t *key,
-                                                   size_t nkey,
-                                                   Error **errp)
+static QCryptoCipher *qcrypto_cipher_ctx_new(QCryptoCipherAlgorithm alg,
+                                             QCryptoCipherMode mode,
+                                             const uint8_t *key,
+                                             size_t nkey,
+                                             Error **errp)
 {
     QCryptoCipherGcrypt *ctx;
     gcry_error_t err;
@@ -257,7 +258,7 @@ static QCryptoCipherGcrypt *qcrypto_cipher_ctx_new(QCryptoCipherAlgorithm alg,
     }
 #endif
 
-    return ctx;
+    return &ctx->base;
 
  error:
     qcrypto_gcrypt_cipher_free_ctx(ctx, mode);
@@ -268,7 +269,9 @@ static QCryptoCipherGcrypt *qcrypto_cipher_ctx_new(QCryptoCipherAlgorithm alg,
 static void
 qcrypto_gcrypt_cipher_ctx_free(QCryptoCipher *cipher)
 {
-    qcrypto_gcrypt_cipher_free_ctx(cipher->opaque, cipher->mode);
+    QCryptoCipherGcrypt *ctx = container_of(cipher, QCryptoCipherGcrypt, base);
+
+    qcrypto_gcrypt_cipher_free_ctx(ctx, cipher->mode);
 }
 
 
@@ -301,7 +304,7 @@ qcrypto_gcrypt_cipher_encrypt(QCryptoCipher *cipher,
                               size_t len,
                               Error **errp)
 {
-    QCryptoCipherGcrypt *ctx = cipher->opaque;
+    QCryptoCipherGcrypt *ctx = container_of(cipher, QCryptoCipherGcrypt, base);
     gcry_error_t err;
 
     if (len & (ctx->blocksize - 1)) {
@@ -340,7 +343,7 @@ qcrypto_gcrypt_cipher_decrypt(QCryptoCipher *cipher,
                               size_t len,
                               Error **errp)
 {
-    QCryptoCipherGcrypt *ctx = cipher->opaque;
+    QCryptoCipherGcrypt *ctx = container_of(cipher, QCryptoCipherGcrypt, base);
     gcry_error_t err;
 
     if (len & (ctx->blocksize - 1)) {
@@ -376,7 +379,7 @@ qcrypto_gcrypt_cipher_setiv(QCryptoCipher *cipher,
                             const uint8_t *iv, size_t niv,
                             Error **errp)
 {
-    QCryptoCipherGcrypt *ctx = cipher->opaque;
+    QCryptoCipherGcrypt *ctx = container_of(cipher, QCryptoCipherGcrypt, base);
     gcry_error_t err;
 
     if (niv != ctx->blocksize) {
diff --git a/crypto/cipher-nettle.inc.c b/crypto/cipher-nettle.inc.c
index 6ecce5e8ea..d8371d1f37 100644
--- a/crypto/cipher-nettle.inc.c
+++ b/crypto/cipher-nettle.inc.c
@@ -294,6 +294,8 @@ static void twofish_decrypt_wrapper(const void *ctx, size_t length,
 
 typedef struct QCryptoCipherNettle QCryptoCipherNettle;
 struct QCryptoCipherNettle {
+    QCryptoCipher base;
+
     /* Primary cipher context for all modes */
     void *ctx;
     /* Second cipher context for XTS mode only */
@@ -355,11 +357,11 @@ qcrypto_nettle_cipher_free_ctx(QCryptoCipherNettle *ctx)
 }
 
 
-static QCryptoCipherNettle *qcrypto_cipher_ctx_new(QCryptoCipherAlgorithm alg,
-                                                   QCryptoCipherMode mode,
-                                                   const uint8_t *key,
-                                                   size_t nkey,
-                                                   Error **errp)
+static QCryptoCipher *qcrypto_cipher_ctx_new(QCryptoCipherAlgorithm alg,
+                                             QCryptoCipherMode mode,
+                                             const uint8_t *key,
+                                             size_t nkey,
+                                             Error **errp)
 {
     QCryptoCipherNettle *ctx;
     uint8_t *rfbkey;
@@ -585,7 +587,7 @@ static QCryptoCipherNettle *qcrypto_cipher_ctx_new(QCryptoCipherAlgorithm alg,
 
     ctx->iv = g_new0(uint8_t, ctx->blocksize);
 
-    return ctx;
+    return &ctx->base;
 
  error:
     qcrypto_nettle_cipher_free_ctx(ctx);
@@ -596,9 +598,8 @@ static QCryptoCipherNettle *qcrypto_cipher_ctx_new(QCryptoCipherAlgorithm alg,
 static void
 qcrypto_nettle_cipher_ctx_free(QCryptoCipher *cipher)
 {
-    QCryptoCipherNettle *ctx;
+    QCryptoCipherNettle *ctx = container_of(cipher, QCryptoCipherNettle, base);
 
-    ctx = cipher->opaque;
     qcrypto_nettle_cipher_free_ctx(ctx);
 }
 
@@ -610,7 +611,7 @@ qcrypto_nettle_cipher_encrypt(QCryptoCipher *cipher,
                               size_t len,
                               Error **errp)
 {
-    QCryptoCipherNettle *ctx = cipher->opaque;
+    QCryptoCipherNettle *ctx = container_of(cipher, QCryptoCipherNettle, base);
 
     if (len & (ctx->blocksize - 1)) {
         error_setg(errp, "Length %zu must be a multiple of block size %zu",
@@ -663,7 +664,7 @@ qcrypto_nettle_cipher_decrypt(QCryptoCipher *cipher,
                               size_t len,
                               Error **errp)
 {
-    QCryptoCipherNettle *ctx = cipher->opaque;
+    QCryptoCipherNettle *ctx = container_of(cipher, QCryptoCipherNettle, base);
 
     if (len & (ctx->blocksize - 1)) {
         error_setg(errp, "Length %zu must be a multiple of block size %zu",
@@ -713,7 +714,8 @@ qcrypto_nettle_cipher_setiv(QCryptoCipher *cipher,
                             const uint8_t *iv, size_t niv,
                             Error **errp)
 {
-    QCryptoCipherNettle *ctx = cipher->opaque;
+    QCryptoCipherNettle *ctx = container_of(cipher, QCryptoCipherNettle, base);
+
     if (niv != ctx->blocksize) {
         error_setg(errp, "Expected IV size %zu not %zu",
                    ctx->blocksize, niv);
diff --git a/crypto/cipher.c b/crypto/cipher.c
index d3ef856009..6ea75bb764 100644
--- a/crypto/cipher.c
+++ b/crypto/cipher.c
@@ -163,30 +163,27 @@ QCryptoCipher *qcrypto_cipher_new(QCryptoCipherAlgorithm alg,
                                   const uint8_t *key, size_t nkey,
                                   Error **errp)
 {
-    QCryptoCipher *cipher;
-    void *ctx = NULL;
+    QCryptoCipher *cipher = NULL;
     const QCryptoCipherDriver *drv = NULL;
 
 #ifdef CONFIG_AF_ALG
-    ctx = qcrypto_afalg_cipher_ctx_new(alg, mode, key, nkey, NULL);
-    if (ctx) {
+    cipher = qcrypto_afalg_cipher_ctx_new(alg, mode, key, nkey, NULL);
+    if (cipher) {
         drv = &qcrypto_cipher_afalg_driver;
     }
 #endif
 
-    if (!ctx) {
-        ctx = qcrypto_cipher_ctx_new(alg, mode, key, nkey, errp);
-        if (!ctx) {
+    if (!cipher) {
+        cipher = qcrypto_cipher_ctx_new(alg, mode, key, nkey, errp);
+        if (!cipher) {
             return NULL;
         }
 
         drv = &qcrypto_cipher_lib_driver;
     }
 
-    cipher = g_new0(QCryptoCipher, 1);
     cipher->alg = alg;
     cipher->mode = mode;
-    cipher->opaque = ctx;
     cipher->driver = drv;
 
     return cipher;
@@ -226,10 +223,7 @@ int qcrypto_cipher_setiv(QCryptoCipher *cipher,
 
 void qcrypto_cipher_free(QCryptoCipher *cipher)
 {
-    const QCryptoCipherDriver *drv;
     if (cipher) {
-        drv = cipher->driver;
-        drv->cipher_free(cipher);
-        g_free(cipher);
+        cipher->driver->cipher_free(cipher);
     }
 }
-- 
2.25.1



^ permalink raw reply related	[flat|nested] 52+ messages in thread

* [PATCH 09/17] crypto: Move cipher->driver init to qcrypto_*_cipher_ctx_new
  2020-08-13  3:25 [PATCH 00/17] crypto/cipher: Class hierarchy cleanups Richard Henderson
                   ` (7 preceding siblings ...)
  2020-08-13  3:25 ` [PATCH 08/17] crypto: Allocate QCryptoCipher with the subclass Richard Henderson
@ 2020-08-13  3:25 ` Richard Henderson
  2020-08-17 16:56   ` Daniel P. Berrangé
  2020-08-13  3:25 ` [PATCH 10/17] crypto: Constify cipher data tables Richard Henderson
                   ` (11 subsequent siblings)
  20 siblings, 1 reply; 52+ messages in thread
From: Richard Henderson @ 2020-08-13  3:25 UTC (permalink / raw)
  To: qemu-devel; +Cc: berrange

The class vtable should be set by the class initializer.
This will also allow additional subclassing, reducing the
amount of indirection in the hierarchy.

Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
---
 crypto/cipherpriv.h         | 2 --
 crypto/cipher-afalg.c       | 5 ++++-
 crypto/cipher-builtin.inc.c | 4 ++++
 crypto/cipher-gcrypt.inc.c  | 2 ++
 crypto/cipher-nettle.inc.c  | 3 +++
 crypto/cipher.c             | 7 -------
 6 files changed, 13 insertions(+), 10 deletions(-)

diff --git a/crypto/cipherpriv.h b/crypto/cipherpriv.h
index 437b109b5e..396527857d 100644
--- a/crypto/cipherpriv.h
+++ b/crypto/cipherpriv.h
@@ -47,8 +47,6 @@ qcrypto_afalg_cipher_ctx_new(QCryptoCipherAlgorithm alg,
                              const uint8_t *key,
                              size_t nkey, Error **errp);
 
-extern const struct QCryptoCipherDriver qcrypto_cipher_afalg_driver;
-
 #endif
 
 #endif
diff --git a/crypto/cipher-afalg.c b/crypto/cipher-afalg.c
index 86e5249bd6..052355a8a9 100644
--- a/crypto/cipher-afalg.c
+++ b/crypto/cipher-afalg.c
@@ -58,6 +58,8 @@ qcrypto_afalg_cipher_format_name(QCryptoCipherAlgorithm alg,
     return name;
 }
 
+static const struct QCryptoCipherDriver qcrypto_cipher_afalg_driver;
+
 QCryptoCipher *
 qcrypto_afalg_cipher_ctx_new(QCryptoCipherAlgorithm alg,
                              QCryptoCipherMode mode,
@@ -109,6 +111,7 @@ qcrypto_afalg_cipher_ctx_new(QCryptoCipherAlgorithm alg,
     }
     afalg->cmsg = CMSG_FIRSTHDR(afalg->msg);
 
+    afalg->base.driver = &qcrypto_cipher_afalg_driver;
     return &afalg->base;
 }
 
@@ -222,7 +225,7 @@ static void qcrypto_afalg_comm_ctx_free(QCryptoCipher *cipher)
     qcrypto_afalg_comm_free(afalg);
 }
 
-const struct QCryptoCipherDriver qcrypto_cipher_afalg_driver = {
+static const struct QCryptoCipherDriver qcrypto_cipher_afalg_driver = {
     .cipher_encrypt = qcrypto_afalg_cipher_encrypt,
     .cipher_decrypt = qcrypto_afalg_cipher_decrypt,
     .cipher_setiv = qcrypto_afalg_cipher_setiv,
diff --git a/crypto/cipher-builtin.inc.c b/crypto/cipher-builtin.inc.c
index 6a03e23040..1444139f36 100644
--- a/crypto/cipher-builtin.inc.c
+++ b/crypto/cipher-builtin.inc.c
@@ -22,6 +22,8 @@
 #include "crypto/desrfb.h"
 #include "crypto/xts.h"
 
+static const struct QCryptoCipherDriver qcrypto_cipher_lib_driver;
+
 typedef struct QCryptoCipherBuiltinAESContext QCryptoCipherBuiltinAESContext;
 struct QCryptoCipherBuiltinAESContext {
     AES_KEY enc;
@@ -292,6 +294,7 @@ qcrypto_cipher_init_aes(QCryptoCipherMode mode,
     ctxt->encrypt = qcrypto_cipher_encrypt_aes;
     ctxt->decrypt = qcrypto_cipher_decrypt_aes;
 
+    ctxt->base.driver = &qcrypto_cipher_lib_driver;
     return &ctxt->base;
 
  error:
@@ -396,6 +399,7 @@ qcrypto_cipher_init_des_rfb(QCryptoCipherMode mode,
     ctxt->encrypt = qcrypto_cipher_encrypt_des_rfb;
     ctxt->decrypt = qcrypto_cipher_decrypt_des_rfb;
 
+    ctxt->base.driver = &qcrypto_cipher_lib_driver;
     return &ctxt->base;
 }
 
diff --git a/crypto/cipher-gcrypt.inc.c b/crypto/cipher-gcrypt.inc.c
index 3b3c85e265..7a1fbc9745 100644
--- a/crypto/cipher-gcrypt.inc.c
+++ b/crypto/cipher-gcrypt.inc.c
@@ -24,6 +24,7 @@
 
 #include <gcrypt.h>
 
+static const struct QCryptoCipherDriver qcrypto_cipher_lib_driver;
 
 bool qcrypto_cipher_supports(QCryptoCipherAlgorithm alg,
                              QCryptoCipherMode mode)
@@ -258,6 +259,7 @@ static QCryptoCipher *qcrypto_cipher_ctx_new(QCryptoCipherAlgorithm alg,
     }
 #endif
 
+    ctx->base.driver = &qcrypto_cipher_lib_driver;
     return &ctx->base;
 
  error:
diff --git a/crypto/cipher-nettle.inc.c b/crypto/cipher-nettle.inc.c
index d8371d1f37..36d57ef430 100644
--- a/crypto/cipher-nettle.inc.c
+++ b/crypto/cipher-nettle.inc.c
@@ -34,6 +34,8 @@
 #include <nettle/xts.h>
 #endif
 
+static const struct QCryptoCipherDriver qcrypto_cipher_lib_driver;
+
 typedef void (*QCryptoCipherNettleFuncWrapper)(const void *ctx,
                                                size_t length,
                                                uint8_t *dst,
@@ -587,6 +589,7 @@ static QCryptoCipher *qcrypto_cipher_ctx_new(QCryptoCipherAlgorithm alg,
 
     ctx->iv = g_new0(uint8_t, ctx->blocksize);
 
+    ctx->base.driver = &qcrypto_cipher_lib_driver;
     return &ctx->base;
 
  error:
diff --git a/crypto/cipher.c b/crypto/cipher.c
index 6ea75bb764..6e25f68f5c 100644
--- a/crypto/cipher.c
+++ b/crypto/cipher.c
@@ -164,13 +164,9 @@ QCryptoCipher *qcrypto_cipher_new(QCryptoCipherAlgorithm alg,
                                   Error **errp)
 {
     QCryptoCipher *cipher = NULL;
-    const QCryptoCipherDriver *drv = NULL;
 
 #ifdef CONFIG_AF_ALG
     cipher = qcrypto_afalg_cipher_ctx_new(alg, mode, key, nkey, NULL);
-    if (cipher) {
-        drv = &qcrypto_cipher_afalg_driver;
-    }
 #endif
 
     if (!cipher) {
@@ -178,13 +174,10 @@ QCryptoCipher *qcrypto_cipher_new(QCryptoCipherAlgorithm alg,
         if (!cipher) {
             return NULL;
         }
-
-        drv = &qcrypto_cipher_lib_driver;
     }
 
     cipher->alg = alg;
     cipher->mode = mode;
-    cipher->driver = drv;
 
     return cipher;
 }
-- 
2.25.1



^ permalink raw reply related	[flat|nested] 52+ messages in thread

* [PATCH 10/17] crypto: Constify cipher data tables
  2020-08-13  3:25 [PATCH 00/17] crypto/cipher: Class hierarchy cleanups Richard Henderson
                   ` (8 preceding siblings ...)
  2020-08-13  3:25 ` [PATCH 09/17] crypto: Move cipher->driver init to qcrypto_*_cipher_ctx_new Richard Henderson
@ 2020-08-13  3:25 ` Richard Henderson
  2020-08-13  8:03   ` Philippe Mathieu-Daudé
  2020-08-17 16:56   ` Daniel P. Berrangé
  2020-08-13  3:25 ` [PATCH 11/17] crypto/builtin: Remove odd-sized AES block handling Richard Henderson
                   ` (10 subsequent siblings)
  20 siblings, 2 replies; 52+ messages in thread
From: Richard Henderson @ 2020-08-13  3:25 UTC (permalink / raw)
  To: qemu-devel; +Cc: berrange

Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
---
 crypto/cipher.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/crypto/cipher.c b/crypto/cipher.c
index 6e25f68f5c..2fe12a4fdc 100644
--- a/crypto/cipher.c
+++ b/crypto/cipher.c
@@ -25,7 +25,7 @@
 #include "cipherpriv.h"
 
 
-static size_t alg_key_len[QCRYPTO_CIPHER_ALG__MAX] = {
+static const size_t alg_key_len[QCRYPTO_CIPHER_ALG__MAX] = {
     [QCRYPTO_CIPHER_ALG_AES_128] = 16,
     [QCRYPTO_CIPHER_ALG_AES_192] = 24,
     [QCRYPTO_CIPHER_ALG_AES_256] = 32,
@@ -40,7 +40,7 @@ static size_t alg_key_len[QCRYPTO_CIPHER_ALG__MAX] = {
     [QCRYPTO_CIPHER_ALG_TWOFISH_256] = 32,
 };
 
-static size_t alg_block_len[QCRYPTO_CIPHER_ALG__MAX] = {
+static const size_t alg_block_len[QCRYPTO_CIPHER_ALG__MAX] = {
     [QCRYPTO_CIPHER_ALG_AES_128] = 16,
     [QCRYPTO_CIPHER_ALG_AES_192] = 16,
     [QCRYPTO_CIPHER_ALG_AES_256] = 16,
@@ -55,7 +55,7 @@ static size_t alg_block_len[QCRYPTO_CIPHER_ALG__MAX] = {
     [QCRYPTO_CIPHER_ALG_TWOFISH_256] = 16,
 };
 
-static bool mode_need_iv[QCRYPTO_CIPHER_MODE__MAX] = {
+static const bool mode_need_iv[QCRYPTO_CIPHER_MODE__MAX] = {
     [QCRYPTO_CIPHER_MODE_ECB] = false,
     [QCRYPTO_CIPHER_MODE_CBC] = true,
     [QCRYPTO_CIPHER_MODE_XTS] = true,
-- 
2.25.1



^ permalink raw reply related	[flat|nested] 52+ messages in thread

* [PATCH 11/17] crypto/builtin: Remove odd-sized AES block handling
  2020-08-13  3:25 [PATCH 00/17] crypto/cipher: Class hierarchy cleanups Richard Henderson
                   ` (9 preceding siblings ...)
  2020-08-13  3:25 ` [PATCH 10/17] crypto: Constify cipher data tables Richard Henderson
@ 2020-08-13  3:25 ` Richard Henderson
  2020-08-17 16:57   ` Daniel P. Berrangé
  2020-08-13  3:25 ` [PATCH 12/17] crypto/builtin: Merge qcrypto_cipher_aes_{ecb, xts}_{en, de}crypt Richard Henderson
                   ` (9 subsequent siblings)
  20 siblings, 1 reply; 52+ messages in thread
From: Richard Henderson @ 2020-08-13  3:25 UTC (permalink / raw)
  To: qemu-devel; +Cc: berrange

We verified that the data block is properly sized modulo
AES_BLOCK_SIZE within qcrypto_builtin_cipher_{en,de}crypt.
Therefore we will never have to handle odd sized blocks.

Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
---
 crypto/cipher-builtin.inc.c | 40 +++++++++++--------------------------
 1 file changed, 12 insertions(+), 28 deletions(-)

diff --git a/crypto/cipher-builtin.inc.c b/crypto/cipher-builtin.inc.c
index 1444139f36..e2ae5d090c 100644
--- a/crypto/cipher-builtin.inc.c
+++ b/crypto/cipher-builtin.inc.c
@@ -80,21 +80,13 @@ static void qcrypto_cipher_aes_ecb_encrypt(const AES_KEY *key,
 {
     const uint8_t *inptr = in;
     uint8_t *outptr = out;
+
+    /* We have already verified that len % AES_BLOCK_SIZE == 0. */
     while (len) {
-        if (len > AES_BLOCK_SIZE) {
-            AES_encrypt(inptr, outptr, key);
-            inptr += AES_BLOCK_SIZE;
-            outptr += AES_BLOCK_SIZE;
-            len -= AES_BLOCK_SIZE;
-        } else {
-            uint8_t tmp1[AES_BLOCK_SIZE], tmp2[AES_BLOCK_SIZE];
-            memcpy(tmp1, inptr, len);
-            /* Fill with 0 to avoid valgrind uninitialized reads */
-            memset(tmp1 + len, 0, sizeof(tmp1) - len);
-            AES_encrypt(tmp1, tmp2, key);
-            memcpy(outptr, tmp2, len);
-            len = 0;
-        }
+        AES_encrypt(inptr, outptr, key);
+        inptr += AES_BLOCK_SIZE;
+        outptr += AES_BLOCK_SIZE;
+        len -= AES_BLOCK_SIZE;
     }
 }
 
@@ -106,21 +98,13 @@ static void qcrypto_cipher_aes_ecb_decrypt(const AES_KEY *key,
 {
     const uint8_t *inptr = in;
     uint8_t *outptr = out;
+
+    /* We have already verified that len % AES_BLOCK_SIZE == 0. */
     while (len) {
-        if (len > AES_BLOCK_SIZE) {
-            AES_decrypt(inptr, outptr, key);
-            inptr += AES_BLOCK_SIZE;
-            outptr += AES_BLOCK_SIZE;
-            len -= AES_BLOCK_SIZE;
-        } else {
-            uint8_t tmp1[AES_BLOCK_SIZE], tmp2[AES_BLOCK_SIZE];
-            memcpy(tmp1, inptr, len);
-            /* Fill with 0 to avoid valgrind uninitialized reads */
-            memset(tmp1 + len, 0, sizeof(tmp1) - len);
-            AES_decrypt(tmp1, tmp2, key);
-            memcpy(outptr, tmp2, len);
-            len = 0;
-        }
+        AES_decrypt(inptr, outptr, key);
+        inptr += AES_BLOCK_SIZE;
+        outptr += AES_BLOCK_SIZE;
+        len -= AES_BLOCK_SIZE;
     }
 }
 
-- 
2.25.1



^ permalink raw reply related	[flat|nested] 52+ messages in thread

* [PATCH 12/17] crypto/builtin: Merge qcrypto_cipher_aes_{ecb, xts}_{en, de}crypt
  2020-08-13  3:25 [PATCH 00/17] crypto/cipher: Class hierarchy cleanups Richard Henderson
                   ` (10 preceding siblings ...)
  2020-08-13  3:25 ` [PATCH 11/17] crypto/builtin: Remove odd-sized AES block handling Richard Henderson
@ 2020-08-13  3:25 ` Richard Henderson
  2020-08-13  8:05   ` Philippe Mathieu-Daudé
  2020-08-17 17:00   ` [PATCH 12/17] crypto/builtin: Merge qcrypto_cipher_aes_{ecb,xts}_{en,de}crypt Daniel P. Berrangé
  2020-08-13  3:25 ` [PATCH 13/17] crypto/builtin: Move AES_cbc_encrypt into cipher-builtin.inc.c Richard Henderson
                   ` (8 subsequent siblings)
  20 siblings, 2 replies; 52+ messages in thread
From: Richard Henderson @ 2020-08-13  3:25 UTC (permalink / raw)
  To: qemu-devel; +Cc: berrange

There's no real reason we need two separate helper functions here.
Standardize on the function signature required for xts_encrypt.
Rename to do_aes_{en,de}crypt_ecb, since the helper does not
itself do anything with respect to xts.

Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
---
 crypto/cipher-builtin.inc.c | 69 ++++++++++---------------------------
 1 file changed, 18 insertions(+), 51 deletions(-)

diff --git a/crypto/cipher-builtin.inc.c b/crypto/cipher-builtin.inc.c
index e2ae5d090c..4d971a2b82 100644
--- a/crypto/cipher-builtin.inc.c
+++ b/crypto/cipher-builtin.inc.c
@@ -72,65 +72,34 @@ static void qcrypto_cipher_free_aes(QCryptoCipher *cipher)
     g_free(cipher);
 }
 
-
-static void qcrypto_cipher_aes_ecb_encrypt(const AES_KEY *key,
-                                           const void *in,
-                                           void *out,
-                                           size_t len)
+static void do_aes_encrypt_ecb(const void *vctx, size_t len,
+                               uint8_t *out, const uint8_t *in)
 {
-    const uint8_t *inptr = in;
-    uint8_t *outptr = out;
+    const QCryptoCipherBuiltinAESContext *ctx = vctx;
 
     /* We have already verified that len % AES_BLOCK_SIZE == 0. */
     while (len) {
-        AES_encrypt(inptr, outptr, key);
-        inptr += AES_BLOCK_SIZE;
-        outptr += AES_BLOCK_SIZE;
+        AES_encrypt(in, out, &ctx->enc);
+        in += AES_BLOCK_SIZE;
+        out += AES_BLOCK_SIZE;
         len -= AES_BLOCK_SIZE;
     }
 }
 
-
-static void qcrypto_cipher_aes_ecb_decrypt(const AES_KEY *key,
-                                           const void *in,
-                                           void *out,
-                                           size_t len)
+static void do_aes_decrypt_ecb(const void *vctx, size_t len,
+                               uint8_t *out, const uint8_t *in)
 {
-    const uint8_t *inptr = in;
-    uint8_t *outptr = out;
+    const QCryptoCipherBuiltinAESContext *ctx = vctx;
 
     /* We have already verified that len % AES_BLOCK_SIZE == 0. */
     while (len) {
-        AES_decrypt(inptr, outptr, key);
-        inptr += AES_BLOCK_SIZE;
-        outptr += AES_BLOCK_SIZE;
+        AES_decrypt(in, out, &ctx->dec);
+        in += AES_BLOCK_SIZE;
+        out += AES_BLOCK_SIZE;
         len -= AES_BLOCK_SIZE;
     }
 }
 
-
-static void qcrypto_cipher_aes_xts_encrypt(const void *ctx,
-                                           size_t length,
-                                           uint8_t *dst,
-                                           const uint8_t *src)
-{
-    const QCryptoCipherBuiltinAESContext *aesctx = ctx;
-
-    qcrypto_cipher_aes_ecb_encrypt(&aesctx->enc, src, dst, length);
-}
-
-
-static void qcrypto_cipher_aes_xts_decrypt(const void *ctx,
-                                           size_t length,
-                                           uint8_t *dst,
-                                           const uint8_t *src)
-{
-    const QCryptoCipherBuiltinAESContext *aesctx = ctx;
-
-    qcrypto_cipher_aes_ecb_decrypt(&aesctx->dec, src, dst, length);
-}
-
-
 static int qcrypto_cipher_encrypt_aes(QCryptoCipher *cipher,
                                       const void *in,
                                       void *out,
@@ -142,8 +111,7 @@ static int qcrypto_cipher_encrypt_aes(QCryptoCipher *cipher,
 
     switch (cipher->mode) {
     case QCRYPTO_CIPHER_MODE_ECB:
-        qcrypto_cipher_aes_ecb_encrypt(&ctxt->state.aes.key.enc,
-                                       in, out, len);
+        do_aes_encrypt_ecb(&ctxt->state.aes.key, len, out, in);
         break;
     case QCRYPTO_CIPHER_MODE_CBC:
         AES_cbc_encrypt(in, out, len,
@@ -153,8 +121,8 @@ static int qcrypto_cipher_encrypt_aes(QCryptoCipher *cipher,
     case QCRYPTO_CIPHER_MODE_XTS:
         xts_encrypt(&ctxt->state.aes.key,
                     &ctxt->state.aes.key_tweak,
-                    qcrypto_cipher_aes_xts_encrypt,
-                    qcrypto_cipher_aes_xts_decrypt,
+                    do_aes_encrypt_ecb,
+                    do_aes_decrypt_ecb,
                     ctxt->state.aes.iv,
                     len, out, in);
         break;
@@ -177,8 +145,7 @@ static int qcrypto_cipher_decrypt_aes(QCryptoCipher *cipher,
 
     switch (cipher->mode) {
     case QCRYPTO_CIPHER_MODE_ECB:
-        qcrypto_cipher_aes_ecb_decrypt(&ctxt->state.aes.key.dec,
-                                       in, out, len);
+        do_aes_decrypt_ecb(&ctxt->state.aes.key, len, out, in);
         break;
     case QCRYPTO_CIPHER_MODE_CBC:
         AES_cbc_encrypt(in, out, len,
@@ -188,8 +155,8 @@ static int qcrypto_cipher_decrypt_aes(QCryptoCipher *cipher,
     case QCRYPTO_CIPHER_MODE_XTS:
         xts_decrypt(&ctxt->state.aes.key,
                     &ctxt->state.aes.key_tweak,
-                    qcrypto_cipher_aes_xts_encrypt,
-                    qcrypto_cipher_aes_xts_decrypt,
+                    do_aes_encrypt_ecb,
+                    do_aes_decrypt_ecb,
                     ctxt->state.aes.iv,
                     len, out, in);
         break;
-- 
2.25.1



^ permalink raw reply related	[flat|nested] 52+ messages in thread

* [PATCH 13/17] crypto/builtin: Move AES_cbc_encrypt into cipher-builtin.inc.c
  2020-08-13  3:25 [PATCH 00/17] crypto/cipher: Class hierarchy cleanups Richard Henderson
                   ` (11 preceding siblings ...)
  2020-08-13  3:25 ` [PATCH 12/17] crypto/builtin: Merge qcrypto_cipher_aes_{ecb, xts}_{en, de}crypt Richard Henderson
@ 2020-08-13  3:25 ` Richard Henderson
  2020-08-13  8:11   ` Philippe Mathieu-Daudé
  2020-08-17 17:01   ` Daniel P. Berrangé
  2020-08-13  3:25 ` [PATCH 14/17] crypto/builtin: Split and simplify AES_encrypt_cbc Richard Henderson
                   ` (7 subsequent siblings)
  20 siblings, 2 replies; 52+ messages in thread
From: Richard Henderson @ 2020-08-13  3:25 UTC (permalink / raw)
  To: qemu-devel; +Cc: berrange

By making the function private, we will be able to make further
simplifications.  Re-indent the migrated code and fix the missing
braces for CODING_STYLE.

Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
---
 include/crypto/aes.h        |  4 ---
 crypto/aes.c                | 51 ---------------------------------
 crypto/cipher-builtin.inc.c | 56 +++++++++++++++++++++++++++++++++++++
 3 files changed, 56 insertions(+), 55 deletions(-)

diff --git a/include/crypto/aes.h b/include/crypto/aes.h
index 12fb321b89..ba297d6a73 100644
--- a/include/crypto/aes.h
+++ b/include/crypto/aes.h
@@ -16,7 +16,6 @@ typedef struct aes_key_st AES_KEY;
 #define AES_set_decrypt_key QEMU_AES_set_decrypt_key
 #define AES_encrypt QEMU_AES_encrypt
 #define AES_decrypt QEMU_AES_decrypt
-#define AES_cbc_encrypt QEMU_AES_cbc_encrypt
 
 int AES_set_encrypt_key(const unsigned char *userKey, const int bits,
 	AES_KEY *key);
@@ -27,9 +26,6 @@ void AES_encrypt(const unsigned char *in, unsigned char *out,
 	const AES_KEY *key);
 void AES_decrypt(const unsigned char *in, unsigned char *out,
 	const AES_KEY *key);
-void AES_cbc_encrypt(const unsigned char *in, unsigned char *out,
-		     const unsigned long length, const AES_KEY *key,
-		     unsigned char *ivec, const int enc);
 
 extern const uint8_t AES_sbox[256];
 extern const uint8_t AES_isbox[256];
diff --git a/crypto/aes.c b/crypto/aes.c
index 0f6a195af8..159800df65 100644
--- a/crypto/aes.c
+++ b/crypto/aes.c
@@ -1599,54 +1599,3 @@ void AES_decrypt(const unsigned char *in, unsigned char *out,
 }
 
 #endif /* AES_ASM */
-
-void AES_cbc_encrypt(const unsigned char *in, unsigned char *out,
-                     const unsigned long length, const AES_KEY *key,
-                     unsigned char *ivec, const int enc)
-{
-
-        unsigned long n;
-        unsigned long len = length;
-        unsigned char tmp[AES_BLOCK_SIZE];
-
-        assert(in && out && key && ivec);
-
-        if (enc) {
-                while (len >= AES_BLOCK_SIZE) {
-                        for(n=0; n < AES_BLOCK_SIZE; ++n)
-                                tmp[n] = in[n] ^ ivec[n];
-                        AES_encrypt(tmp, out, key);
-                        memcpy(ivec, out, AES_BLOCK_SIZE);
-                        len -= AES_BLOCK_SIZE;
-                        in += AES_BLOCK_SIZE;
-                        out += AES_BLOCK_SIZE;
-                }
-                if (len) {
-                        for(n=0; n < len; ++n)
-                                tmp[n] = in[n] ^ ivec[n];
-                        for(n=len; n < AES_BLOCK_SIZE; ++n)
-                                tmp[n] = ivec[n];
-                        AES_encrypt(tmp, tmp, key);
-                        memcpy(out, tmp, AES_BLOCK_SIZE);
-                        memcpy(ivec, tmp, AES_BLOCK_SIZE);
-                }
-        } else {
-                while (len >= AES_BLOCK_SIZE) {
-                        memcpy(tmp, in, AES_BLOCK_SIZE);
-                        AES_decrypt(in, out, key);
-                        for(n=0; n < AES_BLOCK_SIZE; ++n)
-                                out[n] ^= ivec[n];
-                        memcpy(ivec, tmp, AES_BLOCK_SIZE);
-                        len -= AES_BLOCK_SIZE;
-                        in += AES_BLOCK_SIZE;
-                        out += AES_BLOCK_SIZE;
-                }
-                if (len) {
-                        memcpy(tmp, in, AES_BLOCK_SIZE);
-                        AES_decrypt(tmp, tmp, key);
-                        for(n=0; n < len; ++n)
-                                out[n] = tmp[n] ^ ivec[n];
-                        memcpy(ivec, tmp, AES_BLOCK_SIZE);
-                }
-        }
-}
diff --git a/crypto/cipher-builtin.inc.c b/crypto/cipher-builtin.inc.c
index 4d971a2b82..416d44b38e 100644
--- a/crypto/cipher-builtin.inc.c
+++ b/crypto/cipher-builtin.inc.c
@@ -100,6 +100,62 @@ static void do_aes_decrypt_ecb(const void *vctx, size_t len,
     }
 }
 
+static void AES_cbc_encrypt(const unsigned char *in, unsigned char *out,
+                            const unsigned long length, const AES_KEY *key,
+                            unsigned char *ivec, const int enc)
+{
+    unsigned long n;
+    unsigned long len = length;
+    unsigned char tmp[AES_BLOCK_SIZE];
+
+    assert(in && out && key && ivec);
+
+    if (enc) {
+        while (len >= AES_BLOCK_SIZE) {
+            for (n = 0; n < AES_BLOCK_SIZE; ++n) {
+                tmp[n] = in[n] ^ ivec[n];
+            }
+            AES_encrypt(tmp, out, key);
+            memcpy(ivec, out, AES_BLOCK_SIZE);
+            len -= AES_BLOCK_SIZE;
+            in += AES_BLOCK_SIZE;
+            out += AES_BLOCK_SIZE;
+        }
+        if (len) {
+            for (n = 0; n < len; ++n) {
+                tmp[n] = in[n] ^ ivec[n];
+            }
+            for (n = len; n < AES_BLOCK_SIZE; ++n) {
+                tmp[n] = ivec[n];
+            }
+            AES_encrypt(tmp, tmp, key);
+            memcpy(out, tmp, AES_BLOCK_SIZE);
+            memcpy(ivec, tmp, AES_BLOCK_SIZE);
+        }
+    } else {
+        while (len >= AES_BLOCK_SIZE) {
+            memcpy(tmp, in, AES_BLOCK_SIZE);
+            AES_decrypt(in, out, key);
+            for (n = 0; n < AES_BLOCK_SIZE; ++n) {
+                out[n] ^= ivec[n];
+            }
+            memcpy(ivec, tmp, AES_BLOCK_SIZE);
+            len -= AES_BLOCK_SIZE;
+            in += AES_BLOCK_SIZE;
+            out += AES_BLOCK_SIZE;
+        }
+        if (len) {
+            memcpy(tmp, in, AES_BLOCK_SIZE);
+            AES_decrypt(tmp, tmp, key);
+            for (n = 0; n < len; ++n) {
+                out[n] = tmp[n] ^ ivec[n];
+            }
+            memcpy(ivec, tmp, AES_BLOCK_SIZE);
+        }
+    }
+}
+
+
 static int qcrypto_cipher_encrypt_aes(QCryptoCipher *cipher,
                                       const void *in,
                                       void *out,
-- 
2.25.1



^ permalink raw reply related	[flat|nested] 52+ messages in thread

* [PATCH 14/17] crypto/builtin: Split and simplify AES_encrypt_cbc
  2020-08-13  3:25 [PATCH 00/17] crypto/cipher: Class hierarchy cleanups Richard Henderson
                   ` (12 preceding siblings ...)
  2020-08-13  3:25 ` [PATCH 13/17] crypto/builtin: Move AES_cbc_encrypt into cipher-builtin.inc.c Richard Henderson
@ 2020-08-13  3:25 ` Richard Henderson
  2020-08-17 17:02   ` Daniel P. Berrangé
  2020-08-13  3:25 ` [PATCH 15/17] crypto/builtin: Split QCryptoCipherBuiltin into subclasses Richard Henderson
                   ` (6 subsequent siblings)
  20 siblings, 1 reply; 52+ messages in thread
From: Richard Henderson @ 2020-08-13  3:25 UTC (permalink / raw)
  To: qemu-devel; +Cc: berrange

Split into encrypt/decrypt functions, dropping the "enc" argument.
Now that the function is private to this file, we know that "len"
is a multiple of AES_BLOCK_SIZE.  So drop the odd block size code.

Name the functions do_aes_*crypt_cbc to match the *_ecb functions.
Reorder and re-type the arguments to match as well.

Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
---
 crypto/cipher-builtin.inc.c | 91 +++++++++++++++----------------------
 1 file changed, 36 insertions(+), 55 deletions(-)

diff --git a/crypto/cipher-builtin.inc.c b/crypto/cipher-builtin.inc.c
index 416d44b38e..4295f93af5 100644
--- a/crypto/cipher-builtin.inc.c
+++ b/crypto/cipher-builtin.inc.c
@@ -100,61 +100,44 @@ static void do_aes_decrypt_ecb(const void *vctx, size_t len,
     }
 }
 
-static void AES_cbc_encrypt(const unsigned char *in, unsigned char *out,
-                            const unsigned long length, const AES_KEY *key,
-                            unsigned char *ivec, const int enc)
+static void do_aes_encrypt_cbc(const AES_KEY *key, size_t len, uint8_t *out,
+                               const uint8_t *in, uint8_t *ivec)
 {
-    unsigned long n;
-    unsigned long len = length;
-    unsigned char tmp[AES_BLOCK_SIZE];
+    uint8_t tmp[AES_BLOCK_SIZE];
+    size_t n;
 
-    assert(in && out && key && ivec);
-
-    if (enc) {
-        while (len >= AES_BLOCK_SIZE) {
-            for (n = 0; n < AES_BLOCK_SIZE; ++n) {
-                tmp[n] = in[n] ^ ivec[n];
-            }
-            AES_encrypt(tmp, out, key);
-            memcpy(ivec, out, AES_BLOCK_SIZE);
-            len -= AES_BLOCK_SIZE;
-            in += AES_BLOCK_SIZE;
-            out += AES_BLOCK_SIZE;
-        }
-        if (len) {
-            for (n = 0; n < len; ++n) {
-                tmp[n] = in[n] ^ ivec[n];
-            }
-            for (n = len; n < AES_BLOCK_SIZE; ++n) {
-                tmp[n] = ivec[n];
-            }
-            AES_encrypt(tmp, tmp, key);
-            memcpy(out, tmp, AES_BLOCK_SIZE);
-            memcpy(ivec, tmp, AES_BLOCK_SIZE);
-        }
-    } else {
-        while (len >= AES_BLOCK_SIZE) {
-            memcpy(tmp, in, AES_BLOCK_SIZE);
-            AES_decrypt(in, out, key);
-            for (n = 0; n < AES_BLOCK_SIZE; ++n) {
-                out[n] ^= ivec[n];
-            }
-            memcpy(ivec, tmp, AES_BLOCK_SIZE);
-            len -= AES_BLOCK_SIZE;
-            in += AES_BLOCK_SIZE;
-            out += AES_BLOCK_SIZE;
-        }
-        if (len) {
-            memcpy(tmp, in, AES_BLOCK_SIZE);
-            AES_decrypt(tmp, tmp, key);
-            for (n = 0; n < len; ++n) {
-                out[n] = tmp[n] ^ ivec[n];
-            }
-            memcpy(ivec, tmp, AES_BLOCK_SIZE);
+    /* We have already verified that len % AES_BLOCK_SIZE == 0. */
+    while (len) {
+        for (n = 0; n < AES_BLOCK_SIZE; ++n) {
+            tmp[n] = in[n] ^ ivec[n];
         }
+        AES_encrypt(tmp, out, key);
+        memcpy(ivec, out, AES_BLOCK_SIZE);
+        len -= AES_BLOCK_SIZE;
+        in += AES_BLOCK_SIZE;
+        out += AES_BLOCK_SIZE;
     }
 }
 
+static void do_aes_decrypt_cbc(const AES_KEY *key, size_t len, uint8_t *out,
+                               const uint8_t *in, uint8_t *ivec)
+{
+    uint8_t tmp[AES_BLOCK_SIZE];
+    size_t n;
+
+    /* We have already verified that len % AES_BLOCK_SIZE == 0. */
+    while (len) {
+        memcpy(tmp, in, AES_BLOCK_SIZE);
+        AES_decrypt(in, out, key);
+        for (n = 0; n < AES_BLOCK_SIZE; ++n) {
+            out[n] ^= ivec[n];
+        }
+        memcpy(ivec, tmp, AES_BLOCK_SIZE);
+        len -= AES_BLOCK_SIZE;
+        in += AES_BLOCK_SIZE;
+        out += AES_BLOCK_SIZE;
+    }
+}
 
 static int qcrypto_cipher_encrypt_aes(QCryptoCipher *cipher,
                                       const void *in,
@@ -170,9 +153,8 @@ static int qcrypto_cipher_encrypt_aes(QCryptoCipher *cipher,
         do_aes_encrypt_ecb(&ctxt->state.aes.key, len, out, in);
         break;
     case QCRYPTO_CIPHER_MODE_CBC:
-        AES_cbc_encrypt(in, out, len,
-                        &ctxt->state.aes.key.enc,
-                        ctxt->state.aes.iv, 1);
+        do_aes_encrypt_cbc(&ctxt->state.aes.key.enc, len, out, in,
+                           ctxt->state.aes.iv);
         break;
     case QCRYPTO_CIPHER_MODE_XTS:
         xts_encrypt(&ctxt->state.aes.key,
@@ -204,9 +186,8 @@ static int qcrypto_cipher_decrypt_aes(QCryptoCipher *cipher,
         do_aes_decrypt_ecb(&ctxt->state.aes.key, len, out, in);
         break;
     case QCRYPTO_CIPHER_MODE_CBC:
-        AES_cbc_encrypt(in, out, len,
-                        &ctxt->state.aes.key.dec,
-                        ctxt->state.aes.iv, 0);
+        do_aes_decrypt_cbc(&ctxt->state.aes.key.dec, len, out, in,
+                           ctxt->state.aes.iv);
         break;
     case QCRYPTO_CIPHER_MODE_XTS:
         xts_decrypt(&ctxt->state.aes.key,
-- 
2.25.1



^ permalink raw reply related	[flat|nested] 52+ messages in thread

* [PATCH 15/17] crypto/builtin: Split QCryptoCipherBuiltin into subclasses
  2020-08-13  3:25 [PATCH 00/17] crypto/cipher: Class hierarchy cleanups Richard Henderson
                   ` (13 preceding siblings ...)
  2020-08-13  3:25 ` [PATCH 14/17] crypto/builtin: Split and simplify AES_encrypt_cbc Richard Henderson
@ 2020-08-13  3:25 ` Richard Henderson
  2020-08-28 13:44   ` Daniel P. Berrangé
  2020-08-13  3:25 ` [PATCH 16/17] crypto/nettle: Split QCryptoCipherNettle " Richard Henderson
                   ` (5 subsequent siblings)
  20 siblings, 1 reply; 52+ messages in thread
From: Richard Henderson @ 2020-08-13  3:25 UTC (permalink / raw)
  To: qemu-devel; +Cc: berrange

We had a second set of function pointers in QCryptoCipherBuiltin,
which are redundant with QCryptoCipherDriver.  Split the AES and
DES implementations to avoid one level of indirection.

Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
---
 crypto/cipher-builtin.inc.c | 553 +++++++++++++++---------------------
 1 file changed, 227 insertions(+), 326 deletions(-)

diff --git a/crypto/cipher-builtin.inc.c b/crypto/cipher-builtin.inc.c
index 4295f93af5..0da22784ec 100644
--- a/crypto/cipher-builtin.inc.c
+++ b/crypto/cipher-builtin.inc.c
@@ -22,56 +22,45 @@
 #include "crypto/desrfb.h"
 #include "crypto/xts.h"
 
-static const struct QCryptoCipherDriver qcrypto_cipher_lib_driver;
-
 typedef struct QCryptoCipherBuiltinAESContext QCryptoCipherBuiltinAESContext;
 struct QCryptoCipherBuiltinAESContext {
     AES_KEY enc;
     AES_KEY dec;
 };
+
 typedef struct QCryptoCipherBuiltinAES QCryptoCipherBuiltinAES;
 struct QCryptoCipherBuiltinAES {
+    QCryptoCipher base;
     QCryptoCipherBuiltinAESContext key;
     QCryptoCipherBuiltinAESContext key_tweak;
     uint8_t iv[AES_BLOCK_SIZE];
 };
-typedef struct QCryptoCipherBuiltinDESRFB QCryptoCipherBuiltinDESRFB;
-struct QCryptoCipherBuiltinDESRFB {
-    uint8_t *key;
-    size_t nkey;
-};
-
-typedef struct QCryptoCipherBuiltin QCryptoCipherBuiltin;
-struct QCryptoCipherBuiltin {
-    QCryptoCipher base;
-
-    union {
-        QCryptoCipherBuiltinAES aes;
-        QCryptoCipherBuiltinDESRFB desrfb;
-    } state;
-    size_t blocksize;
-    void (*free)(QCryptoCipher *cipher);
-    int (*setiv)(QCryptoCipher *cipher,
-                 const uint8_t *iv, size_t niv,
-                 Error **errp);
-    int (*encrypt)(QCryptoCipher *cipher,
-                   const void *in,
-                   void *out,
-                   size_t len,
-                   Error **errp);
-    int (*decrypt)(QCryptoCipher *cipher,
-                   const void *in,
-                   void *out,
-                   size_t len,
-                   Error **errp);
-};
 
 
-static void qcrypto_cipher_free_aes(QCryptoCipher *cipher)
+static inline bool qcrypto_length_check(size_t len, size_t blocksize,
+                                        Error **errp)
+{
+    if (unlikely(len & (blocksize - 1))) {
+        error_setg(errp, "Length %zu must be a multiple of block size %zu",
+                   len, blocksize);
+        return false;
+    }
+    return true;
+}
+
+static void qcrypto_cipher_ctx_free(QCryptoCipher *cipher)
 {
     g_free(cipher);
 }
 
+static int qcrypto_cipher_no_setiv(QCryptoCipher *cipher,
+                                   const uint8_t *iv, size_t niv,
+                                   Error **errp)
+{
+    error_setg(errp, "Setting IV is not supported");
+    return -1;
+}
+
 static void do_aes_encrypt_ecb(const void *vctx, size_t len,
                                uint8_t *out, const uint8_t *in)
 {
@@ -139,77 +128,100 @@ static void do_aes_decrypt_cbc(const AES_KEY *key, size_t len, uint8_t *out,
     }
 }
 
-static int qcrypto_cipher_encrypt_aes(QCryptoCipher *cipher,
-                                      const void *in,
-                                      void *out,
-                                      size_t len,
-                                      Error **errp)
+static int qcrypto_cipher_aes_encrypt_ecb(QCryptoCipher *cipher,
+                                          const void *in, void *out,
+                                          size_t len, Error **errp)
 {
-    QCryptoCipherBuiltin *ctxt
-        = container_of(cipher, QCryptoCipherBuiltin, base);
+    QCryptoCipherBuiltinAES *ctx
+        = container_of(cipher, QCryptoCipherBuiltinAES, base);
 
-    switch (cipher->mode) {
-    case QCRYPTO_CIPHER_MODE_ECB:
-        do_aes_encrypt_ecb(&ctxt->state.aes.key, len, out, in);
-        break;
-    case QCRYPTO_CIPHER_MODE_CBC:
-        do_aes_encrypt_cbc(&ctxt->state.aes.key.enc, len, out, in,
-                           ctxt->state.aes.iv);
-        break;
-    case QCRYPTO_CIPHER_MODE_XTS:
-        xts_encrypt(&ctxt->state.aes.key,
-                    &ctxt->state.aes.key_tweak,
-                    do_aes_encrypt_ecb,
-                    do_aes_decrypt_ecb,
-                    ctxt->state.aes.iv,
-                    len, out, in);
-        break;
-    default:
-        g_assert_not_reached();
+    if (!qcrypto_length_check(len, AES_BLOCK_SIZE, errp)) {
+        return -1;
     }
+    do_aes_encrypt_ecb(&ctx->key, len, out, in);
+    return 0;
+}
 
+static int qcrypto_cipher_aes_decrypt_ecb(QCryptoCipher *cipher,
+                                          const void *in, void *out,
+                                          size_t len, Error **errp)
+{
+    QCryptoCipherBuiltinAES *ctx
+        = container_of(cipher, QCryptoCipherBuiltinAES, base);
+
+    if (!qcrypto_length_check(len, AES_BLOCK_SIZE, errp)) {
+        return -1;
+    }
+    do_aes_decrypt_ecb(&ctx->key, len, out, in);
+    return 0;
+}
+
+static int qcrypto_cipher_aes_encrypt_cbc(QCryptoCipher *cipher,
+                                          const void *in, void *out,
+                                          size_t len, Error **errp)
+{
+    QCryptoCipherBuiltinAES *ctx
+        = container_of(cipher, QCryptoCipherBuiltinAES, base);
+
+    if (!qcrypto_length_check(len, AES_BLOCK_SIZE, errp)) {
+        return -1;
+    }
+    do_aes_encrypt_cbc(&ctx->key.enc, len, out, in, ctx->iv);
+    return 0;
+}
+
+static int qcrypto_cipher_aes_decrypt_cbc(QCryptoCipher *cipher,
+                                          const void *in, void *out,
+                                          size_t len, Error **errp)
+{
+    QCryptoCipherBuiltinAES *ctx
+        = container_of(cipher, QCryptoCipherBuiltinAES, base);
+
+    if (!qcrypto_length_check(len, AES_BLOCK_SIZE, errp)) {
+        return -1;
+    }
+    do_aes_decrypt_cbc(&ctx->key.dec, len, out, in, ctx->iv);
+    return 0;
+}
+
+static int qcrypto_cipher_aes_encrypt_xts(QCryptoCipher *cipher,
+                                          const void *in, void *out,
+                                          size_t len, Error **errp)
+{
+    QCryptoCipherBuiltinAES *ctx
+        = container_of(cipher, QCryptoCipherBuiltinAES, base);
+
+    if (!qcrypto_length_check(len, AES_BLOCK_SIZE, errp)) {
+        return -1;
+    }
+    xts_encrypt(&ctx->key, &ctx->key_tweak,
+                do_aes_encrypt_ecb, do_aes_decrypt_ecb,
+                ctx->iv, len, out, in);
+    return 0;
+}
+
+static int qcrypto_cipher_aes_decrypt_xts(QCryptoCipher *cipher,
+                                          const void *in, void *out,
+                                          size_t len, Error **errp)
+{
+    QCryptoCipherBuiltinAES *ctx
+        = container_of(cipher, QCryptoCipherBuiltinAES, base);
+
+    if (!qcrypto_length_check(len, AES_BLOCK_SIZE, errp)) {
+        return -1;
+    }
+    xts_decrypt(&ctx->key, &ctx->key_tweak,
+                do_aes_encrypt_ecb, do_aes_decrypt_ecb,
+                ctx->iv, len, out, in);
     return 0;
 }
 
 
-static int qcrypto_cipher_decrypt_aes(QCryptoCipher *cipher,
-                                      const void *in,
-                                      void *out,
-                                      size_t len,
-                                      Error **errp)
+static int qcrypto_cipher_aes_setiv(QCryptoCipher *cipher, const uint8_t *iv,
+                             size_t niv, Error **errp)
 {
-    QCryptoCipherBuiltin *ctxt
-        = container_of(cipher, QCryptoCipherBuiltin, base);
-
-    switch (cipher->mode) {
-    case QCRYPTO_CIPHER_MODE_ECB:
-        do_aes_decrypt_ecb(&ctxt->state.aes.key, len, out, in);
-        break;
-    case QCRYPTO_CIPHER_MODE_CBC:
-        do_aes_decrypt_cbc(&ctxt->state.aes.key.dec, len, out, in,
-                           ctxt->state.aes.iv);
-        break;
-    case QCRYPTO_CIPHER_MODE_XTS:
-        xts_decrypt(&ctxt->state.aes.key,
-                    &ctxt->state.aes.key_tweak,
-                    do_aes_encrypt_ecb,
-                    do_aes_decrypt_ecb,
-                    ctxt->state.aes.iv,
-                    len, out, in);
-        break;
-    default:
-        g_assert_not_reached();
-    }
-
-    return 0;
-}
-
-static int qcrypto_cipher_setiv_aes(QCryptoCipher *cipher,
-                                     const uint8_t *iv, size_t niv,
-                                     Error **errp)
-{
-    QCryptoCipherBuiltin *ctxt
-        = container_of(cipher, QCryptoCipherBuiltin, base);
+    QCryptoCipherBuiltinAES *ctx
+        = container_of(cipher, QCryptoCipherBuiltinAES, base);
 
     if (niv != AES_BLOCK_SIZE) {
         error_setg(errp, "IV must be %d bytes not %zu",
@@ -217,107 +229,53 @@ static int qcrypto_cipher_setiv_aes(QCryptoCipher *cipher,
         return -1;
     }
 
-    memcpy(ctxt->state.aes.iv, iv, AES_BLOCK_SIZE);
-
+    memcpy(ctx->iv, iv, AES_BLOCK_SIZE);
     return 0;
 }
 
+static const struct QCryptoCipherDriver qcrypto_cipher_aes_driver_ecb = {
+    .cipher_encrypt = qcrypto_cipher_aes_encrypt_ecb,
+    .cipher_decrypt = qcrypto_cipher_aes_decrypt_ecb,
+    .cipher_setiv = qcrypto_cipher_no_setiv,
+    .cipher_free = qcrypto_cipher_ctx_free,
+};
+
+static const struct QCryptoCipherDriver qcrypto_cipher_aes_driver_cbc = {
+    .cipher_encrypt = qcrypto_cipher_aes_encrypt_cbc,
+    .cipher_decrypt = qcrypto_cipher_aes_decrypt_cbc,
+    .cipher_setiv = qcrypto_cipher_aes_setiv,
+    .cipher_free = qcrypto_cipher_ctx_free,
+};
+
+static const struct QCryptoCipherDriver qcrypto_cipher_aes_driver_xts = {
+    .cipher_encrypt = qcrypto_cipher_aes_encrypt_xts,
+    .cipher_decrypt = qcrypto_cipher_aes_decrypt_xts,
+    .cipher_setiv = qcrypto_cipher_aes_setiv,
+    .cipher_free = qcrypto_cipher_ctx_free,
+};
 
 
+typedef struct QCryptoCipherBuiltinDESRFB QCryptoCipherBuiltinDESRFB;
+struct QCryptoCipherBuiltinDESRFB {
+    QCryptoCipher base;
 
-static QCryptoCipher *
-qcrypto_cipher_init_aes(QCryptoCipherMode mode,
-                        const uint8_t *key, size_t nkey,
-                        Error **errp)
-{
-    QCryptoCipherBuiltin *ctxt;
-
-    if (mode != QCRYPTO_CIPHER_MODE_CBC &&
-        mode != QCRYPTO_CIPHER_MODE_ECB &&
-        mode != QCRYPTO_CIPHER_MODE_XTS) {
-        error_setg(errp, "Unsupported cipher mode %s",
-                   QCryptoCipherMode_str(mode));
-        return NULL;
-    }
-
-    ctxt = g_new0(QCryptoCipherBuiltin, 1);
-
-    if (mode == QCRYPTO_CIPHER_MODE_XTS) {
-        if (AES_set_encrypt_key(key, nkey * 4, &ctxt->state.aes.key.enc) != 0) {
-            error_setg(errp, "Failed to set encryption key");
-            goto error;
-        }
-
-        if (AES_set_decrypt_key(key, nkey * 4, &ctxt->state.aes.key.dec) != 0) {
-            error_setg(errp, "Failed to set decryption key");
-            goto error;
-        }
-
-        if (AES_set_encrypt_key(key + (nkey / 2), nkey * 4,
-                                &ctxt->state.aes.key_tweak.enc) != 0) {
-            error_setg(errp, "Failed to set encryption key");
-            goto error;
-        }
-
-        if (AES_set_decrypt_key(key + (nkey / 2), nkey * 4,
-                                &ctxt->state.aes.key_tweak.dec) != 0) {
-            error_setg(errp, "Failed to set decryption key");
-            goto error;
-        }
-    } else {
-        if (AES_set_encrypt_key(key, nkey * 8, &ctxt->state.aes.key.enc) != 0) {
-            error_setg(errp, "Failed to set encryption key");
-            goto error;
-        }
-
-        if (AES_set_decrypt_key(key, nkey * 8, &ctxt->state.aes.key.dec) != 0) {
-            error_setg(errp, "Failed to set decryption key");
-            goto error;
-        }
-    }
-
-    ctxt->blocksize = AES_BLOCK_SIZE;
-    ctxt->free = qcrypto_cipher_free_aes;
-    ctxt->setiv = qcrypto_cipher_setiv_aes;
-    ctxt->encrypt = qcrypto_cipher_encrypt_aes;
-    ctxt->decrypt = qcrypto_cipher_decrypt_aes;
-
-    ctxt->base.driver = &qcrypto_cipher_lib_driver;
-    return &ctxt->base;
-
- error:
-    g_free(ctxt);
-    return NULL;
-}
-
-
-static void qcrypto_cipher_free_des_rfb(QCryptoCipher *cipher)
-{
-    QCryptoCipherBuiltin *ctxt
-        = container_of(cipher, QCryptoCipherBuiltin, base);
-
-    g_free(ctxt->state.desrfb.key);
-    g_free(ctxt);
-}
-
+    /* C.f. alg_key_len[QCRYPTO_CIPHER_ALG_DES_RFB] */
+    uint8_t key[8];
+};
 
 static int qcrypto_cipher_encrypt_des_rfb(QCryptoCipher *cipher,
-                                          const void *in,
-                                          void *out,
-                                          size_t len,
-                                          Error **errp)
+                                          const void *in, void *out,
+                                          size_t len, Error **errp)
 {
-    QCryptoCipherBuiltin *ctxt
-        = container_of(cipher, QCryptoCipherBuiltin, base);
+    QCryptoCipherBuiltinDESRFB *ctx
+        = container_of(cipher, QCryptoCipherBuiltinDESRFB, base);
     size_t i;
 
-    if (len % 8) {
-        error_setg(errp, "Buffer size must be multiple of 8 not %zu",
-                   len);
+    if (!qcrypto_length_check(len, 8, errp)) {
         return -1;
     }
 
-    deskey(ctxt->state.desrfb.key, EN0);
+    deskey(ctx->key, EN0);
 
     for (i = 0; i < len; i += 8) {
         des((void *)in + i, out + i);
@@ -326,24 +284,19 @@ static int qcrypto_cipher_encrypt_des_rfb(QCryptoCipher *cipher,
     return 0;
 }
 
-
 static int qcrypto_cipher_decrypt_des_rfb(QCryptoCipher *cipher,
-                                          const void *in,
-                                          void *out,
-                                          size_t len,
-                                          Error **errp)
+                                          const void *in, void *out,
+                                          size_t len, Error **errp)
 {
-    QCryptoCipherBuiltin *ctxt
-        = container_of(cipher, QCryptoCipherBuiltin, base);
+    QCryptoCipherBuiltinDESRFB *ctx
+        = container_of(cipher, QCryptoCipherBuiltinDESRFB, base);
     size_t i;
 
-    if (len % 8) {
-        error_setg(errp, "Buffer size must be multiple of 8 not %zu",
-                   len);
+    if (!qcrypto_length_check(len, 8, errp)) {
         return -1;
     }
 
-    deskey(ctxt->state.desrfb.key, DE1);
+    deskey(ctx->key, DE1);
 
     for (i = 0; i < len; i += 8) {
         des((void *)in + i, out + i);
@@ -352,173 +305,121 @@ static int qcrypto_cipher_decrypt_des_rfb(QCryptoCipher *cipher,
     return 0;
 }
 
-
-static int qcrypto_cipher_setiv_des_rfb(QCryptoCipher *cipher,
-                                        const uint8_t *iv, size_t niv,
-                                        Error **errp)
-{
-    error_setg(errp, "Setting IV is not supported");
-    return -1;
-}
-
-
-static QCryptoCipher *
-qcrypto_cipher_init_des_rfb(QCryptoCipherMode mode,
-                            const uint8_t *key, size_t nkey,
-                            Error **errp)
-{
-    QCryptoCipherBuiltin *ctxt;
-
-    if (mode != QCRYPTO_CIPHER_MODE_ECB) {
-        error_setg(errp, "Unsupported cipher mode %s",
-                   QCryptoCipherMode_str(mode));
-        return NULL;
-    }
-
-    ctxt = g_new0(QCryptoCipherBuiltin, 1);
-
-    ctxt->state.desrfb.key = g_new0(uint8_t, nkey);
-    memcpy(ctxt->state.desrfb.key, key, nkey);
-    ctxt->state.desrfb.nkey = nkey;
-
-    ctxt->blocksize = 8;
-    ctxt->free = qcrypto_cipher_free_des_rfb;
-    ctxt->setiv = qcrypto_cipher_setiv_des_rfb;
-    ctxt->encrypt = qcrypto_cipher_encrypt_des_rfb;
-    ctxt->decrypt = qcrypto_cipher_decrypt_des_rfb;
-
-    ctxt->base.driver = &qcrypto_cipher_lib_driver;
-    return &ctxt->base;
-}
-
+static const struct QCryptoCipherDriver qcrypto_cipher_des_rfb_driver = {
+    .cipher_encrypt = qcrypto_cipher_encrypt_des_rfb,
+    .cipher_decrypt = qcrypto_cipher_decrypt_des_rfb,
+    .cipher_setiv = qcrypto_cipher_no_setiv,
+    .cipher_free = qcrypto_cipher_ctx_free,
+};
 
 bool qcrypto_cipher_supports(QCryptoCipherAlgorithm alg,
                              QCryptoCipherMode mode)
 {
     switch (alg) {
     case QCRYPTO_CIPHER_ALG_DES_RFB:
+        return mode == QCRYPTO_CIPHER_MODE_ECB;
     case QCRYPTO_CIPHER_ALG_AES_128:
     case QCRYPTO_CIPHER_ALG_AES_192:
     case QCRYPTO_CIPHER_ALG_AES_256:
+        switch (mode) {
+        case QCRYPTO_CIPHER_MODE_ECB:
+        case QCRYPTO_CIPHER_MODE_CBC:
+        case QCRYPTO_CIPHER_MODE_XTS:
+            return true;
+        default:
+            return false;
+        }
         break;
     default:
         return false;
     }
-
-    switch (mode) {
-    case QCRYPTO_CIPHER_MODE_ECB:
-    case QCRYPTO_CIPHER_MODE_CBC:
-    case QCRYPTO_CIPHER_MODE_XTS:
-        return true;
-    case QCRYPTO_CIPHER_MODE_CTR:
-        return false;
-    default:
-        return false;
-    }
 }
 
-
 static QCryptoCipher *qcrypto_cipher_ctx_new(QCryptoCipherAlgorithm alg,
                                              QCryptoCipherMode mode,
                                              const uint8_t *key,
                                              size_t nkey,
                                              Error **errp)
 {
-    switch (mode) {
-    case QCRYPTO_CIPHER_MODE_ECB:
-    case QCRYPTO_CIPHER_MODE_CBC:
-    case QCRYPTO_CIPHER_MODE_XTS:
-        break;
-    default:
-        error_setg(errp, "Unsupported cipher mode %s",
-                   QCryptoCipherMode_str(mode));
-        return NULL;
-    }
-
     if (!qcrypto_cipher_validate_key_length(alg, mode, nkey, errp)) {
         return NULL;
     }
 
     switch (alg) {
     case QCRYPTO_CIPHER_ALG_DES_RFB:
-        return qcrypto_cipher_init_des_rfb(mode, key, nkey, errp);
+        if (mode == QCRYPTO_CIPHER_MODE_ECB) {
+            QCryptoCipherBuiltinDESRFB *ctx;
+
+            ctx = g_new0(QCryptoCipherBuiltinDESRFB, 1);
+            ctx->base.driver = &qcrypto_cipher_des_rfb_driver;
+            memcpy(ctx->key, key, sizeof(ctx->key));
+
+            return &ctx->base;
+        }
+        goto bad_mode;
+
     case QCRYPTO_CIPHER_ALG_AES_128:
     case QCRYPTO_CIPHER_ALG_AES_192:
     case QCRYPTO_CIPHER_ALG_AES_256:
-        return qcrypto_cipher_init_aes(mode, key, nkey, errp);
+        {
+            QCryptoCipherBuiltinAES *ctx;
+            const QCryptoCipherDriver *drv;
+
+            switch (mode) {
+            case QCRYPTO_CIPHER_MODE_ECB:
+                drv = &qcrypto_cipher_aes_driver_ecb;
+                break;
+            case QCRYPTO_CIPHER_MODE_CBC:
+                drv = &qcrypto_cipher_aes_driver_cbc;
+                break;
+            case QCRYPTO_CIPHER_MODE_XTS:
+                drv = &qcrypto_cipher_aes_driver_xts;
+                break;
+            default:
+                goto bad_mode;
+            }
+
+            ctx = g_new0(QCryptoCipherBuiltinAES, 1);
+            ctx->base.driver = drv;
+
+            if (mode == QCRYPTO_CIPHER_MODE_XTS) {
+                nkey /= 2;
+                if (AES_set_encrypt_key(key + nkey, nkey * 8,
+                                        &ctx->key_tweak.enc)) {
+                    error_setg(errp, "Failed to set encryption key");
+                    goto error;
+                }
+                if (AES_set_decrypt_key(key + nkey, nkey * 8,
+                                        &ctx->key_tweak.dec)) {
+                    error_setg(errp, "Failed to set decryption key");
+                    goto error;
+                }
+            }
+            if (AES_set_encrypt_key(key, nkey * 8, &ctx->key.enc)) {
+                error_setg(errp, "Failed to set encryption key");
+                goto error;
+            }
+            if (AES_set_decrypt_key(key, nkey * 8, &ctx->key.dec)) {
+                error_setg(errp, "Failed to set decryption key");
+                goto error;
+            }
+
+            return &ctx->base;
+
+        error:
+            g_free(ctx);
+            return NULL;
+        }
+
     default:
         error_setg(errp,
                    "Unsupported cipher algorithm %s",
                    QCryptoCipherAlgorithm_str(alg));
         return NULL;
     }
+
+ bad_mode:
+    error_setg(errp, "Unsupported cipher mode %s",
+               QCryptoCipherMode_str(mode));
+    return NULL;
 }
-
-static void
-qcrypto_builtin_cipher_ctx_free(QCryptoCipher *cipher)
-{
-    QCryptoCipherBuiltin *ctxt
-        = container_of(cipher, QCryptoCipherBuiltin, base);
-
-    ctxt->free(cipher);
-}
-
-
-static int
-qcrypto_builtin_cipher_encrypt(QCryptoCipher *cipher,
-                               const void *in,
-                               void *out,
-                               size_t len,
-                               Error **errp)
-{
-    QCryptoCipherBuiltin *ctxt
-        = container_of(cipher, QCryptoCipherBuiltin, base);
-
-    if (len & (ctxt->blocksize - 1)) {
-        error_setg(errp, "Length %zu must be a multiple of block size %zu",
-                   len, ctxt->blocksize);
-        return -1;
-    }
-
-    return ctxt->encrypt(cipher, in, out, len, errp);
-}
-
-
-static int
-qcrypto_builtin_cipher_decrypt(QCryptoCipher *cipher,
-                               const void *in,
-                               void *out,
-                               size_t len,
-                               Error **errp)
-{
-    QCryptoCipherBuiltin *ctxt
-        = container_of(cipher, QCryptoCipherBuiltin, base);
-
-    if (len & (ctxt->blocksize - 1)) {
-        error_setg(errp, "Length %zu must be a multiple of block size %zu",
-                   len, ctxt->blocksize);
-        return -1;
-    }
-
-    return ctxt->decrypt(cipher, in, out, len, errp);
-}
-
-
-static int
-qcrypto_builtin_cipher_setiv(QCryptoCipher *cipher,
-                             const uint8_t *iv, size_t niv,
-                             Error **errp)
-{
-    QCryptoCipherBuiltin *ctxt
-        = container_of(cipher, QCryptoCipherBuiltin, base);
-
-    return ctxt->setiv(cipher, iv, niv, errp);
-}
-
-
-static const struct QCryptoCipherDriver qcrypto_cipher_lib_driver = {
-    .cipher_encrypt = qcrypto_builtin_cipher_encrypt,
-    .cipher_decrypt = qcrypto_builtin_cipher_decrypt,
-    .cipher_setiv = qcrypto_builtin_cipher_setiv,
-    .cipher_free = qcrypto_builtin_cipher_ctx_free,
-};
-- 
2.25.1



^ permalink raw reply related	[flat|nested] 52+ messages in thread

* [PATCH 16/17] crypto/nettle: Split QCryptoCipherNettle into subclasses
  2020-08-13  3:25 [PATCH 00/17] crypto/cipher: Class hierarchy cleanups Richard Henderson
                   ` (14 preceding siblings ...)
  2020-08-13  3:25 ` [PATCH 15/17] crypto/builtin: Split QCryptoCipherBuiltin into subclasses Richard Henderson
@ 2020-08-13  3:25 ` Richard Henderson
  2020-08-28 13:46   ` Daniel P. Berrangé
  2020-08-13  3:25 ` [PATCH 17/17] crypto/gcrypt: Split QCryptoCipherGcrypt " Richard Henderson
                   ` (4 subsequent siblings)
  20 siblings, 1 reply; 52+ messages in thread
From: Richard Henderson @ 2020-08-13  3:25 UTC (permalink / raw)
  To: qemu-devel; +Cc: berrange

Use separate classes for each cipher entry point: des_rfb, des3,
aes128, aes192, aes256, cast128, serpent, and twofish.

Generate wrappers for XTS only for CONFIG_QEMU_PRIVATE_XTS.
This eliminates unreachable wrappers for DES_RFB, DES3 and
CAST128, which have blocksizes that do not allow XTS mode.

Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
---
 crypto/cipher-nettle.inc.c | 987 +++++++++++++++++++------------------
 1 file changed, 503 insertions(+), 484 deletions(-)

diff --git a/crypto/cipher-nettle.inc.c b/crypto/cipher-nettle.inc.c
index 36d57ef430..a1f4f6eac6 100644
--- a/crypto/cipher-nettle.inc.c
+++ b/crypto/cipher-nettle.inc.c
@@ -34,8 +34,6 @@
 #include <nettle/xts.h>
 #endif
 
-static const struct QCryptoCipherDriver qcrypto_cipher_lib_driver;
-
 typedef void (*QCryptoCipherNettleFuncWrapper)(const void *ctx,
                                                size_t length,
                                                uint8_t *dst,
@@ -75,62 +73,212 @@ typedef const void * cipher_ctx_t;
 typedef size_t       cipher_length_t;
 #endif
 
-typedef struct QCryptoNettleAES128 {
-    struct aes128_ctx enc;
-    struct aes128_ctx dec;
-} QCryptoNettleAES128;
-
-typedef struct QCryptoNettleAES192 {
-    struct aes192_ctx enc;
-    struct aes192_ctx dec;
-} QCryptoNettleAES192;
-
-typedef struct QCryptoNettleAES256 {
-    struct aes256_ctx enc;
-    struct aes256_ctx dec;
-} QCryptoNettleAES256;
-
-static void aes128_encrypt_native(cipher_ctx_t ctx, cipher_length_t length,
-                                  uint8_t *dst, const uint8_t *src)
+static inline bool qcrypto_length_check(size_t len, size_t blocksize,
+                                        Error **errp)
 {
-    const QCryptoNettleAES128 *aesctx = ctx;
-    aes128_encrypt(&aesctx->enc, length, dst, src);
+    if (unlikely(len & (blocksize - 1))) {
+        error_setg(errp, "Length %zu must be a multiple of block size %zu",
+                   len, blocksize);
+        return false;
+    }
+    return true;
 }
 
-static void aes128_decrypt_native(cipher_ctx_t ctx, cipher_length_t length,
-                                  uint8_t *dst, const uint8_t *src)
+
+static void qcrypto_cipher_ctx_free(QCryptoCipher *ctx)
 {
-    const QCryptoNettleAES128 *aesctx = ctx;
-    aes128_decrypt(&aesctx->dec, length, dst, src);
+    g_free(ctx);
 }
 
-static void aes192_encrypt_native(cipher_ctx_t ctx, cipher_length_t length,
-                               uint8_t *dst, const uint8_t *src)
+static int qcrypto_cipher_no_setiv(QCryptoCipher *cipher,
+                                   const uint8_t *iv, size_t niv,
+                                   Error **errp)
 {
-    const QCryptoNettleAES192 *aesctx = ctx;
-    aes192_encrypt(&aesctx->enc, length, dst, src);
+    error_setg(errp, "Setting IV is not supported");
+    return -1;
 }
 
-static void aes192_decrypt_native(cipher_ctx_t ctx, cipher_length_t length,
-                               uint8_t *dst, const uint8_t *src)
-{
-    const QCryptoNettleAES192 *aesctx = ctx;
-    aes192_decrypt(&aesctx->dec, length, dst, src);
+
+#define DEFINE_SETIV(NAME, TYPE, BLEN)                                  \
+static int NAME##_setiv(QCryptoCipher *cipher, const uint8_t *iv,       \
+                        size_t niv, Error **errp)                       \
+{                                                                       \
+    TYPE *ctx = container_of(cipher, TYPE, base);                       \
+    if (niv != BLEN) {                                                  \
+        error_setg(errp, "Expected IV size %d not %zu", BLEN, niv);     \
+        return -1;                                                      \
+    }                                                                   \
+    memcpy(ctx->iv, iv, niv);                                           \
+    return 0;                                                           \
 }
 
-static void aes256_encrypt_native(cipher_ctx_t ctx, cipher_length_t length,
-                               uint8_t *dst, const uint8_t *src)
-{
-    const QCryptoNettleAES256 *aesctx = ctx;
-    aes256_encrypt(&aesctx->enc, length, dst, src);
-}
 
-static void aes256_decrypt_native(cipher_ctx_t ctx, cipher_length_t length,
-                               uint8_t *dst, const uint8_t *src)
-{
-    const QCryptoNettleAES256 *aesctx = ctx;
-    aes256_decrypt(&aesctx->dec, length, dst, src);
+#define DEFINE_ECB(NAME, TYPE, BLEN, ENCRYPT, DECRYPT)                  \
+static int NAME##_encrypt_ecb(QCryptoCipher *cipher, const void *in,    \
+                              void *out, size_t len, Error **errp)      \
+{                                                                       \
+    TYPE *ctx = container_of(cipher, TYPE, base);                       \
+    if (!qcrypto_length_check(len, BLEN, errp)) {                       \
+        return -1;                                                      \
+    }                                                                   \
+    ENCRYPT(&ctx->key, len, out, in);                                   \
+    return 0;                                                           \
+}                                                                       \
+static int NAME##_decrypt_ecb(QCryptoCipher *cipher, const void *in,    \
+                              void *out, size_t len, Error **errp)      \
+{                                                                       \
+    TYPE *ctx = container_of(cipher, TYPE, base);                       \
+    if (!qcrypto_length_check(len, BLEN, errp)) {                       \
+        return -1;                                                      \
+    }                                                                   \
+    DECRYPT(&ctx->key, len, out, in);                                   \
+    return 0;                                                           \
+}                                                                       \
+static const struct QCryptoCipherDriver NAME##_driver_ecb = {           \
+    .cipher_encrypt = NAME##_encrypt_ecb,                               \
+    .cipher_decrypt = NAME##_decrypt_ecb,                               \
+    .cipher_setiv = qcrypto_cipher_no_setiv,                            \
+    .cipher_free = qcrypto_cipher_ctx_free,                             \
+};
+
+
+#define DEFINE_CBC(NAME, TYPE, BLEN, ENCRYPT, DECRYPT)                  \
+static int NAME##_encrypt_cbc(QCryptoCipher *cipher, const void *in,    \
+                              void *out, size_t len, Error **errp)      \
+{                                                                       \
+    TYPE *ctx = container_of(cipher, TYPE, base);                       \
+    if (!qcrypto_length_check(len, BLEN, errp)) {                       \
+        return -1;                                                      \
+    }                                                                   \
+    cbc_encrypt(&ctx->key, ENCRYPT, BLEN, ctx->iv, len, out, in);       \
+    return 0;                                                           \
+}                                                                       \
+static int NAME##_decrypt_cbc(QCryptoCipher *cipher, const void *in,    \
+                              void *out, size_t len, Error **errp)      \
+{                                                                       \
+    TYPE *ctx = container_of(cipher, TYPE, base);                       \
+    if (!qcrypto_length_check(len, BLEN, errp)) {                       \
+        return -1;                                                      \
+    }                                                                   \
+    cbc_decrypt(&ctx->key, DECRYPT, BLEN, ctx->iv, len, out, in);       \
+    return 0;                                                           \
+}                                                                       \
+static const struct QCryptoCipherDriver NAME##_driver_cbc = {           \
+    .cipher_encrypt = NAME##_encrypt_cbc,                               \
+    .cipher_decrypt = NAME##_decrypt_cbc,                               \
+    .cipher_setiv = NAME##_setiv,                                       \
+    .cipher_free = qcrypto_cipher_ctx_free,                             \
+};
+
+
+#define DEFINE_CTR(NAME, TYPE, BLEN, ENCRYPT)                           \
+static int NAME##_encrypt_ctr(QCryptoCipher *cipher, const void *in,    \
+                              void *out, size_t len, Error **errp)      \
+{                                                                       \
+    TYPE *ctx = container_of(cipher, TYPE, base);                       \
+    if (!qcrypto_length_check(len, BLEN, errp)) {                       \
+        return -1;                                                      \
+    }                                                                   \
+    ctr_crypt(&ctx->key, ENCRYPT, BLEN, ctx->iv, len, out, in);         \
+    return 0;                                                           \
+}                                                                       \
+static const struct QCryptoCipherDriver NAME##_driver_ctr = {           \
+    .cipher_encrypt = NAME##_encrypt_ctr,                               \
+    .cipher_decrypt = NAME##_encrypt_ctr,                               \
+    .cipher_setiv = NAME##_setiv,                                       \
+    .cipher_free = qcrypto_cipher_ctx_free,                             \
+};
+
+
+#ifdef CONFIG_QEMU_PRIVATE_XTS
+#define DEFINE__XTS(NAME, TYPE, BLEN, ENCRYPT, DECRYPT)                 \
+static void NAME##_xts_wrape(const void *ctx, size_t length,            \
+                             uint8_t *dst, const uint8_t *src)          \
+{                                                                       \
+    ENCRYPT(ctx, length, dst, src);                                     \
+}                                                                       \
+static void NAME##_xts_wrapd(const void *ctx, size_t length,            \
+                             uint8_t *dst, const uint8_t *src)          \
+{                                                                       \
+    DECRYPT(ctx, length, dst, src);                                     \
+}                                                                       \
+static int NAME##_encrypt_xts(QCryptoCipher *cipher, const void *in,    \
+                              void *out, size_t len, Error **errp)      \
+{                                                                       \
+    TYPE *ctx = container_of(cipher, TYPE, base);                       \
+    if (!qcrypto_length_check(len, BLEN, errp)) {                       \
+        return -1;                                                      \
+    }                                                                   \
+    xts_encrypt(&ctx->key, &ctx->key_xts,                               \
+                NAME##_xts_wrape, NAME##_xts_wrapd,                     \
+                ctx->iv, len, out, in);                                 \
+    return 0;                                                           \
+}                                                                       \
+static int NAME##_decrypt_xts(QCryptoCipher *cipher, const void *in,    \
+                              void *out, size_t len, Error **errp)      \
+{                                                                       \
+    TYPE *ctx = container_of(cipher, TYPE, base);                       \
+    if (!qcrypto_length_check(len, BLEN, errp)) {                       \
+        return -1;                                                      \
+    }                                                                   \
+    xts_decrypt(&ctx->key, &ctx->key_xts,                               \
+                NAME##_xts_wrape, NAME##_xts_wrapd,                     \
+                ctx->iv, len, out, in);                                 \
+    return 0;                                                           \
 }
+#else
+#define DEFINE__XTS(NAME, TYPE, BLEN, ENCRYPT, DECRYPT)                 \
+static int NAME##_encrypt_xts(QCryptoCipher *cipher, const void *in,    \
+                              void *out, size_t len, Error **errp)      \
+{                                                                       \
+    TYPE *ctx = container_of(cipher, TYPE, base);                       \
+    if (!qcrypto_length_check(len, BLEN, errp)) {                       \
+        return -1;                                                      \
+    }                                                                   \
+    xts_encrypt_message(&ctx->key, &ctx->key_xts, ENCRYPT,              \
+                        ctx->iv, len, out, in);                         \
+    return 0;                                                           \
+}                                                                       \
+static int NAME##_decrypt_xts(QCryptoCipher *cipher, const void *in,    \
+                              void *out, size_t len, Error **errp)      \
+{                                                                       \
+    TYPE *ctx = container_of(cipher, TYPE, base);                       \
+    if (!qcrypto_length_check(len, BLEN, errp)) {                       \
+        return -1;                                                      \
+    }                                                                   \
+    xts_decrypt_message(&ctx->key, &ctx->key_xts, DECRYPT, ENCRYPT,     \
+                        ctx->iv, len, out, in);                         \
+    return 0;                                                           \
+}
+#endif
+
+#define DEFINE_XTS(NAME, TYPE, BLEN, ENCRYPT, DECRYPT)          \
+    QEMU_BUILD_BUG_ON(BLEN != XTS_BLOCK_SIZE);                  \
+    DEFINE__XTS(NAME, TYPE, BLEN, ENCRYPT, DECRYPT)             \
+static const struct QCryptoCipherDriver NAME##_driver_xts = {   \
+    .cipher_encrypt = NAME##_encrypt_xts,                       \
+    .cipher_decrypt = NAME##_decrypt_xts,                       \
+    .cipher_setiv = NAME##_setiv,                               \
+    .cipher_free = qcrypto_cipher_ctx_free,                     \
+};
+
+
+#define DEFINE_ECB_CBC_CTR(NAME, TYPE, BLEN, ENCRYPT, DECRYPT)  \
+    DEFINE_SETIV(NAME, TYPE, BLEN)                              \
+    DEFINE_ECB(NAME, TYPE, BLEN, ENCRYPT, DECRYPT)              \
+    DEFINE_CBC(NAME, TYPE, BLEN, ENCRYPT, DECRYPT)              \
+    DEFINE_CTR(NAME, TYPE, BLEN, ENCRYPT)
+
+#define DEFINE_ECB_CBC_CTR_XTS(NAME, TYPE, BLEN, ENCRYPT, DECRYPT)      \
+    DEFINE_ECB_CBC_CTR(NAME, TYPE, BLEN, ENCRYPT, DECRYPT)              \
+    DEFINE_XTS(NAME, TYPE, BLEN, ENCRYPT, DECRYPT)
+
+
+typedef struct QCryptoNettleDESRFB {
+    QCryptoCipher base;
+    struct des_ctx key;
+    uint8_t iv[DES_BLOCK_SIZE];
+} QCryptoNettleDESRFB;
 
 static void des_encrypt_native(cipher_ctx_t ctx, cipher_length_t length,
                                uint8_t *dst, const uint8_t *src)
@@ -144,6 +292,16 @@ static void des_decrypt_native(cipher_ctx_t ctx, cipher_length_t length,
     des_decrypt(ctx, length, dst, src);
 }
 
+DEFINE_ECB_CBC_CTR(qcrypto_nettle_des_rfb, QCryptoNettleDESRFB,
+                   DES_BLOCK_SIZE, des_encrypt_native, des_decrypt_native)
+
+
+typedef struct QCryptoNettleDES3 {
+    QCryptoCipher base;
+    struct des3_ctx key;
+    uint8_t iv[DES3_BLOCK_SIZE];
+} QCryptoNettleDES3;
+
 static void des3_encrypt_native(cipher_ctx_t ctx, cipher_length_t length,
                                 uint8_t *dst, const uint8_t *src)
 {
@@ -156,6 +314,94 @@ static void des3_decrypt_native(cipher_ctx_t ctx, cipher_length_t length,
     des3_decrypt(ctx, length, dst, src);
 }
 
+DEFINE_ECB_CBC_CTR(qcrypto_nettle_des3, QCryptoNettleDES3, DES3_BLOCK_SIZE,
+                   des3_encrypt_native, des3_decrypt_native)
+
+
+typedef struct QCryptoNettleAES128 {
+    QCryptoCipher base;
+    uint8_t iv[AES_BLOCK_SIZE];
+    /* First key from pair is encode, second key is decode. */
+    struct aes128_ctx key[2], key_xts[2];
+} QCryptoNettleAES128;
+
+static void aes128_encrypt_native(cipher_ctx_t ctx, cipher_length_t length,
+                                  uint8_t *dst, const uint8_t *src)
+{
+    const struct aes128_ctx *keys = ctx;
+    aes128_encrypt(&keys[0], length, dst, src);
+}
+
+static void aes128_decrypt_native(cipher_ctx_t ctx, cipher_length_t length,
+                                  uint8_t *dst, const uint8_t *src)
+{
+    const struct aes128_ctx *keys = ctx;
+    aes128_decrypt(&keys[1], length, dst, src);
+}
+
+DEFINE_ECB_CBC_CTR_XTS(qcrypto_nettle_aes128,
+                       QCryptoNettleAES128, AES_BLOCK_SIZE,
+                       aes128_encrypt_native, aes128_decrypt_native)
+
+
+typedef struct QCryptoNettleAES192 {
+    QCryptoCipher base;
+    uint8_t iv[AES_BLOCK_SIZE];
+    /* First key from pair is encode, second key is decode. */
+    struct aes192_ctx key[2], key_xts[2];
+} QCryptoNettleAES192;
+
+static void aes192_encrypt_native(cipher_ctx_t ctx, cipher_length_t length,
+                                  uint8_t *dst, const uint8_t *src)
+{
+    const struct aes192_ctx *keys = ctx;
+    aes192_encrypt(&keys[0], length, dst, src);
+}
+
+static void aes192_decrypt_native(cipher_ctx_t ctx, cipher_length_t length,
+                                  uint8_t *dst, const uint8_t *src)
+{
+    const struct aes192_ctx *keys = ctx;
+    aes192_decrypt(&keys[1], length, dst, src);
+}
+
+DEFINE_ECB_CBC_CTR_XTS(qcrypto_nettle_aes192,
+                       QCryptoNettleAES192, AES_BLOCK_SIZE,
+                       aes192_encrypt_native, aes192_decrypt_native)
+
+
+typedef struct QCryptoNettleAES256 {
+    QCryptoCipher base;
+    uint8_t iv[AES_BLOCK_SIZE];
+    /* First key from pair is encode, second key is decode. */
+    struct aes256_ctx key[2], key_xts[2];
+} QCryptoNettleAES256;
+
+static void aes256_encrypt_native(cipher_ctx_t ctx, cipher_length_t length,
+                                  uint8_t *dst, const uint8_t *src)
+{
+    const struct aes256_ctx *keys = ctx;
+    aes256_encrypt(&keys[0], length, dst, src);
+}
+
+static void aes256_decrypt_native(cipher_ctx_t ctx, cipher_length_t length,
+                               uint8_t *dst, const uint8_t *src)
+{
+    const struct aes256_ctx *keys = ctx;
+    aes256_decrypt(&keys[1], length, dst, src);
+}
+
+DEFINE_ECB_CBC_CTR_XTS(qcrypto_nettle_aes256,
+                       QCryptoNettleAES256, AES_BLOCK_SIZE,
+                       aes256_encrypt_native, aes256_decrypt_native)
+
+
+typedef struct QCryptoNettleCAST128 {
+    QCryptoCipher base;
+    uint8_t iv[CAST128_BLOCK_SIZE];
+    struct cast128_ctx key, key_xts;
+} QCryptoNettleCAST128;
+
 static void cast128_encrypt_native(cipher_ctx_t ctx, cipher_length_t length,
                                    uint8_t *dst, const uint8_t *src)
 {
@@ -168,6 +414,18 @@ static void cast128_decrypt_native(cipher_ctx_t ctx, cipher_length_t length,
     cast128_decrypt(ctx, length, dst, src);
 }
 
+DEFINE_ECB_CBC_CTR(qcrypto_nettle_cast128,
+                   QCryptoNettleCAST128, CAST128_BLOCK_SIZE,
+                   cast128_encrypt_native, cast128_decrypt_native)
+
+
+typedef struct QCryptoNettleSerpent {
+    QCryptoCipher base;
+    uint8_t iv[SERPENT_BLOCK_SIZE];
+    struct serpent_ctx key, key_xts;
+} QCryptoNettleSerpent;
+
+
 static void serpent_encrypt_native(cipher_ctx_t ctx, cipher_length_t length,
                                    uint8_t *dst, const uint8_t *src)
 {
@@ -180,6 +438,17 @@ static void serpent_decrypt_native(cipher_ctx_t ctx, cipher_length_t length,
     serpent_decrypt(ctx, length, dst, src);
 }
 
+DEFINE_ECB_CBC_CTR_XTS(qcrypto_nettle_serpent,
+                       QCryptoNettleSerpent, SERPENT_BLOCK_SIZE,
+                       serpent_encrypt_native, serpent_decrypt_native)
+
+
+typedef struct QCryptoNettleTwofish {
+    QCryptoCipher base;
+    uint8_t iv[TWOFISH_BLOCK_SIZE];
+    struct twofish_ctx key, key_xts;
+} QCryptoNettleTwofish;
+
 static void twofish_encrypt_native(cipher_ctx_t ctx, cipher_length_t length,
                                    uint8_t *dst, const uint8_t *src)
 {
@@ -192,125 +461,10 @@ static void twofish_decrypt_native(cipher_ctx_t ctx, cipher_length_t length,
     twofish_decrypt(ctx, length, dst, src);
 }
 
-static void aes128_encrypt_wrapper(const void *ctx, size_t length,
-                                uint8_t *dst, const uint8_t *src)
-{
-    const QCryptoNettleAES128 *aesctx = ctx;
-    aes128_encrypt(&aesctx->enc, length, dst, src);
-}
+DEFINE_ECB_CBC_CTR_XTS(qcrypto_nettle_twofish,
+                       QCryptoNettleTwofish, TWOFISH_BLOCK_SIZE,
+                       twofish_encrypt_native, twofish_decrypt_native)
 
-static void aes128_decrypt_wrapper(const void *ctx, size_t length,
-                                uint8_t *dst, const uint8_t *src)
-{
-    const QCryptoNettleAES128 *aesctx = ctx;
-    aes128_decrypt(&aesctx->dec, length, dst, src);
-}
-
-static void aes192_encrypt_wrapper(const void *ctx, size_t length,
-                                uint8_t *dst, const uint8_t *src)
-{
-    const QCryptoNettleAES192 *aesctx = ctx;
-    aes192_encrypt(&aesctx->enc, length, dst, src);
-}
-
-static void aes192_decrypt_wrapper(const void *ctx, size_t length,
-                                uint8_t *dst, const uint8_t *src)
-{
-    const QCryptoNettleAES192 *aesctx = ctx;
-    aes192_decrypt(&aesctx->dec, length, dst, src);
-}
-
-static void aes256_encrypt_wrapper(const void *ctx, size_t length,
-                                uint8_t *dst, const uint8_t *src)
-{
-    const QCryptoNettleAES256 *aesctx = ctx;
-    aes256_encrypt(&aesctx->enc, length, dst, src);
-}
-
-static void aes256_decrypt_wrapper(const void *ctx, size_t length,
-                                uint8_t *dst, const uint8_t *src)
-{
-    const QCryptoNettleAES256 *aesctx = ctx;
-    aes256_decrypt(&aesctx->dec, length, dst, src);
-}
-
-static void des_encrypt_wrapper(const void *ctx, size_t length,
-                                uint8_t *dst, const uint8_t *src)
-{
-    des_encrypt(ctx, length, dst, src);
-}
-
-static void des_decrypt_wrapper(const void *ctx, size_t length,
-                                uint8_t *dst, const uint8_t *src)
-{
-    des_decrypt(ctx, length, dst, src);
-}
-
-static void des3_encrypt_wrapper(const void *ctx, size_t length,
-                                uint8_t *dst, const uint8_t *src)
-{
-    des3_encrypt(ctx, length, dst, src);
-}
-
-static void des3_decrypt_wrapper(const void *ctx, size_t length,
-                                uint8_t *dst, const uint8_t *src)
-{
-    des3_decrypt(ctx, length, dst, src);
-}
-
-static void cast128_encrypt_wrapper(const void *ctx, size_t length,
-                                    uint8_t *dst, const uint8_t *src)
-{
-    cast128_encrypt(ctx, length, dst, src);
-}
-
-static void cast128_decrypt_wrapper(const void *ctx, size_t length,
-                                    uint8_t *dst, const uint8_t *src)
-{
-    cast128_decrypt(ctx, length, dst, src);
-}
-
-static void serpent_encrypt_wrapper(const void *ctx, size_t length,
-                                    uint8_t *dst, const uint8_t *src)
-{
-    serpent_encrypt(ctx, length, dst, src);
-}
-
-static void serpent_decrypt_wrapper(const void *ctx, size_t length,
-                                    uint8_t *dst, const uint8_t *src)
-{
-    serpent_decrypt(ctx, length, dst, src);
-}
-
-static void twofish_encrypt_wrapper(const void *ctx, size_t length,
-                                    uint8_t *dst, const uint8_t *src)
-{
-    twofish_encrypt(ctx, length, dst, src);
-}
-
-static void twofish_decrypt_wrapper(const void *ctx, size_t length,
-                                    uint8_t *dst, const uint8_t *src)
-{
-    twofish_decrypt(ctx, length, dst, src);
-}
-
-typedef struct QCryptoCipherNettle QCryptoCipherNettle;
-struct QCryptoCipherNettle {
-    QCryptoCipher base;
-
-    /* Primary cipher context for all modes */
-    void *ctx;
-    /* Second cipher context for XTS mode only */
-    void *ctx_tweak;
-    /* Cipher callbacks for both contexts */
-    QCryptoCipherNettleFuncNative alg_encrypt_native;
-    QCryptoCipherNettleFuncNative alg_decrypt_native;
-    QCryptoCipherNettleFuncWrapper alg_encrypt_wrapper;
-    QCryptoCipherNettleFuncWrapper alg_decrypt_wrapper;
-    /* Initialization vector or Counter */
-    uint8_t *iv;
-    size_t blocksize;
-};
 
 bool qcrypto_cipher_supports(QCryptoCipherAlgorithm alg,
                              QCryptoCipherMode mode)
@@ -344,30 +498,12 @@ bool qcrypto_cipher_supports(QCryptoCipherAlgorithm alg,
     }
 }
 
-
-static void
-qcrypto_nettle_cipher_free_ctx(QCryptoCipherNettle *ctx)
-{
-    if (!ctx) {
-        return;
-    }
-
-    g_free(ctx->iv);
-    g_free(ctx->ctx);
-    g_free(ctx->ctx_tweak);
-    g_free(ctx);
-}
-
-
 static QCryptoCipher *qcrypto_cipher_ctx_new(QCryptoCipherAlgorithm alg,
                                              QCryptoCipherMode mode,
                                              const uint8_t *key,
                                              size_t nkey,
                                              Error **errp)
 {
-    QCryptoCipherNettle *ctx;
-    uint8_t *rfbkey;
-
     switch (mode) {
     case QCRYPTO_CIPHER_MODE_ECB:
     case QCRYPTO_CIPHER_MODE_CBC:
@@ -375,6 +511,7 @@ static QCryptoCipher *qcrypto_cipher_ctx_new(QCryptoCipherAlgorithm alg,
     case QCRYPTO_CIPHER_MODE_CTR:
         break;
     default:
+    bad_cipher_mode:
         error_setg(errp, "Unsupported cipher mode %s",
                    QCryptoCipherMode_str(mode));
         return NULL;
@@ -384,354 +521,236 @@ static QCryptoCipher *qcrypto_cipher_ctx_new(QCryptoCipherAlgorithm alg,
         return NULL;
     }
 
-    ctx = g_new0(QCryptoCipherNettle, 1);
-
     switch (alg) {
     case QCRYPTO_CIPHER_ALG_DES_RFB:
-        ctx->ctx = g_new0(struct des_ctx, 1);
-        rfbkey = qcrypto_cipher_munge_des_rfb_key(key, nkey);
-        des_set_key(ctx->ctx, rfbkey);
-        g_free(rfbkey);
+        {
+            QCryptoNettleDESRFB *ctx;
+            const QCryptoCipherDriver *drv;
+            uint8_t *rfbkey;
 
-        ctx->alg_encrypt_native = des_encrypt_native;
-        ctx->alg_decrypt_native = des_decrypt_native;
-        ctx->alg_encrypt_wrapper = des_encrypt_wrapper;
-        ctx->alg_decrypt_wrapper = des_decrypt_wrapper;
+            switch (mode) {
+            case QCRYPTO_CIPHER_MODE_ECB:
+                drv = &qcrypto_nettle_des_rfb_driver_ecb;
+                break;
+            case QCRYPTO_CIPHER_MODE_CBC:
+                drv = &qcrypto_nettle_des_rfb_driver_cbc;
+                break;
+            case QCRYPTO_CIPHER_MODE_CTR:
+                drv = &qcrypto_nettle_des_rfb_driver_ctr;
+                break;
+            default:
+                goto bad_cipher_mode;
+            }
 
-        ctx->blocksize = DES_BLOCK_SIZE;
-        break;
+            ctx = g_new0(QCryptoNettleDESRFB, 1);
+            ctx->base.driver = drv;
+
+            rfbkey = qcrypto_cipher_munge_des_rfb_key(key, nkey);
+            des_set_key(&ctx->key, rfbkey);
+            g_free(rfbkey);
+
+            return &ctx->base;
+        }
 
     case QCRYPTO_CIPHER_ALG_3DES:
-        ctx->ctx = g_new0(struct des3_ctx, 1);
-        des3_set_key(ctx->ctx, key);
+        {
+            QCryptoNettleDES3 *ctx;
+            const QCryptoCipherDriver *drv;
 
-        ctx->alg_encrypt_native = des3_encrypt_native;
-        ctx->alg_decrypt_native = des3_decrypt_native;
-        ctx->alg_encrypt_wrapper = des3_encrypt_wrapper;
-        ctx->alg_decrypt_wrapper = des3_decrypt_wrapper;
+            switch (mode) {
+            case QCRYPTO_CIPHER_MODE_ECB:
+                drv = &qcrypto_nettle_des3_driver_ecb;
+                break;
+            case QCRYPTO_CIPHER_MODE_CBC:
+                drv = &qcrypto_nettle_des3_driver_cbc;
+                break;
+            case QCRYPTO_CIPHER_MODE_CTR:
+                drv = &qcrypto_nettle_des3_driver_ctr;
+                break;
+            default:
+                goto bad_cipher_mode;
+            }
 
-        ctx->blocksize = DES3_BLOCK_SIZE;
-        break;
+            ctx = g_new0(QCryptoNettleDES3, 1);
+            ctx->base.driver = drv;
+            des3_set_key(&ctx->key, key);
+            return &ctx->base;
+        }
 
     case QCRYPTO_CIPHER_ALG_AES_128:
-        ctx->ctx = g_new0(QCryptoNettleAES128, 1);
+        {
+            QCryptoNettleAES128 *ctx = g_new0(QCryptoNettleAES128, 1);
 
-        if (mode == QCRYPTO_CIPHER_MODE_XTS) {
-            ctx->ctx_tweak = g_new0(QCryptoNettleAES128, 1);
+            switch (mode) {
+            case QCRYPTO_CIPHER_MODE_ECB:
+                ctx->base.driver = &qcrypto_nettle_aes128_driver_ecb;
+                break;
+            case QCRYPTO_CIPHER_MODE_CBC:
+                ctx->base.driver = &qcrypto_nettle_aes128_driver_cbc;
+                break;
+            case QCRYPTO_CIPHER_MODE_CTR:
+                ctx->base.driver = &qcrypto_nettle_aes128_driver_ctr;
+                break;
+            case QCRYPTO_CIPHER_MODE_XTS:
+                ctx->base.driver = &qcrypto_nettle_aes128_driver_xts;
+                nkey /= 2;
+                aes128_set_encrypt_key(&ctx->key_xts[0], key + nkey);
+                aes128_set_decrypt_key(&ctx->key_xts[1], key + nkey);
+                break;
+            default:
+                g_assert_not_reached();
+            }
+            aes128_set_encrypt_key(&ctx->key[0], key);
+            aes128_set_decrypt_key(&ctx->key[1], key);
 
-            nkey /= 2;
-            aes128_set_encrypt_key(&((QCryptoNettleAES128 *)ctx->ctx)->enc,
-                                   key);
-            aes128_set_decrypt_key(&((QCryptoNettleAES128 *)ctx->ctx)->dec,
-                                   key);
-
-            aes128_set_encrypt_key(&((QCryptoNettleAES128 *)ctx->ctx_tweak)->
-                                   enc, key + nkey);
-            aes128_set_decrypt_key(&((QCryptoNettleAES128 *)ctx->ctx_tweak)->
-                                   dec, key + nkey);
-        } else {
-            aes128_set_encrypt_key(&((QCryptoNettleAES128 *)ctx->ctx)->enc,
-                                   key);
-            aes128_set_decrypt_key(&((QCryptoNettleAES128 *)ctx->ctx)->dec,
-                                   key);
+            return &ctx->base;
         }
 
-        ctx->alg_encrypt_native = aes128_encrypt_native;
-        ctx->alg_decrypt_native = aes128_decrypt_native;
-        ctx->alg_encrypt_wrapper = aes128_encrypt_wrapper;
-        ctx->alg_decrypt_wrapper = aes128_decrypt_wrapper;
-
-        ctx->blocksize = AES_BLOCK_SIZE;
-        break;
-
     case QCRYPTO_CIPHER_ALG_AES_192:
-        ctx->ctx = g_new0(QCryptoNettleAES192, 1);
+        {
+            QCryptoNettleAES192 *ctx = g_new0(QCryptoNettleAES192, 1);
 
-        if (mode == QCRYPTO_CIPHER_MODE_XTS) {
-            ctx->ctx_tweak = g_new0(QCryptoNettleAES192, 1);
+            switch (mode) {
+            case QCRYPTO_CIPHER_MODE_ECB:
+                ctx->base.driver = &qcrypto_nettle_aes192_driver_ecb;
+                break;
+            case QCRYPTO_CIPHER_MODE_CBC:
+                ctx->base.driver = &qcrypto_nettle_aes192_driver_cbc;
+                break;
+            case QCRYPTO_CIPHER_MODE_CTR:
+                ctx->base.driver = &qcrypto_nettle_aes192_driver_ctr;
+                break;
+            case QCRYPTO_CIPHER_MODE_XTS:
+                ctx->base.driver = &qcrypto_nettle_aes192_driver_xts;
+                nkey /= 2;
+                aes192_set_encrypt_key(&ctx->key_xts[0], key + nkey);
+                aes192_set_decrypt_key(&ctx->key_xts[1], key + nkey);
+                break;
+            default:
+                g_assert_not_reached();
+            }
+            aes192_set_encrypt_key(&ctx->key[0], key);
+            aes192_set_decrypt_key(&ctx->key[1], key);
 
-            nkey /= 2;
-            aes192_set_encrypt_key(&((QCryptoNettleAES192 *)ctx->ctx)->enc,
-                                   key);
-            aes192_set_decrypt_key(&((QCryptoNettleAES192 *)ctx->ctx)->dec,
-                                   key);
-
-            aes192_set_encrypt_key(&((QCryptoNettleAES192 *)ctx->ctx_tweak)->
-                                   enc, key + nkey);
-            aes192_set_decrypt_key(&((QCryptoNettleAES192 *)ctx->ctx_tweak)->
-                                   dec, key + nkey);
-        } else {
-            aes192_set_encrypt_key(&((QCryptoNettleAES192 *)ctx->ctx)->enc,
-                                   key);
-            aes192_set_decrypt_key(&((QCryptoNettleAES192 *)ctx->ctx)->dec,
-                                   key);
+            return &ctx->base;
         }
 
-        ctx->alg_encrypt_native = aes192_encrypt_native;
-        ctx->alg_decrypt_native = aes192_decrypt_native;
-        ctx->alg_encrypt_wrapper = aes192_encrypt_wrapper;
-        ctx->alg_decrypt_wrapper = aes192_decrypt_wrapper;
-
-        ctx->blocksize = AES_BLOCK_SIZE;
-        break;
-
     case QCRYPTO_CIPHER_ALG_AES_256:
-        ctx->ctx = g_new0(QCryptoNettleAES256, 1);
+        {
+            QCryptoNettleAES256 *ctx = g_new0(QCryptoNettleAES256, 1);
 
-        if (mode == QCRYPTO_CIPHER_MODE_XTS) {
-            ctx->ctx_tweak = g_new0(QCryptoNettleAES256, 1);
+            switch (mode) {
+            case QCRYPTO_CIPHER_MODE_ECB:
+                ctx->base.driver = &qcrypto_nettle_aes256_driver_ecb;
+                break;
+            case QCRYPTO_CIPHER_MODE_CBC:
+                ctx->base.driver = &qcrypto_nettle_aes256_driver_cbc;
+                break;
+            case QCRYPTO_CIPHER_MODE_CTR:
+                ctx->base.driver = &qcrypto_nettle_aes256_driver_ctr;
+                break;
+            case QCRYPTO_CIPHER_MODE_XTS:
+                ctx->base.driver = &qcrypto_nettle_aes256_driver_xts;
+                nkey /= 2;
+                aes256_set_encrypt_key(&ctx->key_xts[0], key + nkey);
+                aes256_set_decrypt_key(&ctx->key_xts[1], key + nkey);
+                break;
+            default:
+                g_assert_not_reached();
+            }
+            aes256_set_encrypt_key(&ctx->key[0], key);
+            aes256_set_decrypt_key(&ctx->key[1], key);
 
-            nkey /= 2;
-            aes256_set_encrypt_key(&((QCryptoNettleAES256 *)ctx->ctx)->enc,
-                                   key);
-            aes256_set_decrypt_key(&((QCryptoNettleAES256 *)ctx->ctx)->dec,
-                                   key);
-
-            aes256_set_encrypt_key(&((QCryptoNettleAES256 *)ctx->ctx_tweak)->
-                                   enc, key + nkey);
-            aes256_set_decrypt_key(&((QCryptoNettleAES256 *)ctx->ctx_tweak)->
-                                   dec, key + nkey);
-        } else {
-            aes256_set_encrypt_key(&((QCryptoNettleAES256 *)ctx->ctx)->enc,
-                                   key);
-            aes256_set_decrypt_key(&((QCryptoNettleAES256 *)ctx->ctx)->dec,
-                                   key);
+            return &ctx->base;
         }
 
-        ctx->alg_encrypt_native = aes256_encrypt_native;
-        ctx->alg_decrypt_native = aes256_decrypt_native;
-        ctx->alg_encrypt_wrapper = aes256_encrypt_wrapper;
-        ctx->alg_decrypt_wrapper = aes256_decrypt_wrapper;
-
-        ctx->blocksize = AES_BLOCK_SIZE;
-        break;
-
     case QCRYPTO_CIPHER_ALG_CAST5_128:
-        ctx->ctx = g_new0(struct cast128_ctx, 1);
+        {
+            QCryptoNettleCAST128 *ctx;
+            const QCryptoCipherDriver *drv;
 
-        if (mode == QCRYPTO_CIPHER_MODE_XTS) {
-            ctx->ctx_tweak = g_new0(struct cast128_ctx, 1);
+            switch (mode) {
+            case QCRYPTO_CIPHER_MODE_ECB:
+                drv = &qcrypto_nettle_cast128_driver_ecb;
+                break;
+            case QCRYPTO_CIPHER_MODE_CBC:
+                drv = &qcrypto_nettle_cast128_driver_cbc;
+                break;
+            case QCRYPTO_CIPHER_MODE_CTR:
+                drv = &qcrypto_nettle_cast128_driver_ctr;
+                break;
+            default:
+                goto bad_cipher_mode;
+            }
 
-            nkey /= 2;
-            cast5_set_key(ctx->ctx, nkey, key);
-            cast5_set_key(ctx->ctx_tweak, nkey, key + nkey);
-        } else {
-            cast5_set_key(ctx->ctx, nkey, key);
+            ctx = g_new0(QCryptoNettleCAST128, 1);
+            ctx->base.driver = drv;
+            cast5_set_key(&ctx->key, nkey, key);
+
+            return &ctx->base;
         }
 
-        ctx->alg_encrypt_native = cast128_encrypt_native;
-        ctx->alg_decrypt_native = cast128_decrypt_native;
-        ctx->alg_encrypt_wrapper = cast128_encrypt_wrapper;
-        ctx->alg_decrypt_wrapper = cast128_decrypt_wrapper;
-
-        ctx->blocksize = CAST128_BLOCK_SIZE;
-        break;
-
     case QCRYPTO_CIPHER_ALG_SERPENT_128:
     case QCRYPTO_CIPHER_ALG_SERPENT_192:
     case QCRYPTO_CIPHER_ALG_SERPENT_256:
-        ctx->ctx = g_new0(struct serpent_ctx, 1);
+        {
+            QCryptoNettleSerpent *ctx = g_new0(QCryptoNettleSerpent, 1);
 
-        if (mode == QCRYPTO_CIPHER_MODE_XTS) {
-            ctx->ctx_tweak = g_new0(struct serpent_ctx, 1);
+            switch (mode) {
+            case QCRYPTO_CIPHER_MODE_ECB:
+                ctx->base.driver = &qcrypto_nettle_serpent_driver_ecb;
+                break;
+            case QCRYPTO_CIPHER_MODE_CBC:
+                ctx->base.driver = &qcrypto_nettle_serpent_driver_cbc;
+                break;
+            case QCRYPTO_CIPHER_MODE_CTR:
+                ctx->base.driver = &qcrypto_nettle_serpent_driver_ctr;
+                break;
+            case QCRYPTO_CIPHER_MODE_XTS:
+                ctx->base.driver = &qcrypto_nettle_serpent_driver_xts;
+                nkey /= 2;
+                serpent_set_key(&ctx->key_xts, nkey, key + nkey);
+                break;
+            default:
+                g_assert_not_reached();
+            }
+            serpent_set_key(&ctx->key, nkey, key);
 
-            nkey /= 2;
-            serpent_set_key(ctx->ctx, nkey, key);
-            serpent_set_key(ctx->ctx_tweak, nkey, key + nkey);
-        } else {
-            serpent_set_key(ctx->ctx, nkey, key);
+            return &ctx->base;
         }
 
-        ctx->alg_encrypt_native = serpent_encrypt_native;
-        ctx->alg_decrypt_native = serpent_decrypt_native;
-        ctx->alg_encrypt_wrapper = serpent_encrypt_wrapper;
-        ctx->alg_decrypt_wrapper = serpent_decrypt_wrapper;
-
-        ctx->blocksize = SERPENT_BLOCK_SIZE;
-        break;
-
     case QCRYPTO_CIPHER_ALG_TWOFISH_128:
     case QCRYPTO_CIPHER_ALG_TWOFISH_192:
     case QCRYPTO_CIPHER_ALG_TWOFISH_256:
-        ctx->ctx = g_new0(struct twofish_ctx, 1);
+        {
+            QCryptoNettleTwofish *ctx = g_new0(QCryptoNettleTwofish, 1);
 
-        if (mode == QCRYPTO_CIPHER_MODE_XTS) {
-            ctx->ctx_tweak = g_new0(struct twofish_ctx, 1);
+            switch (mode) {
+            case QCRYPTO_CIPHER_MODE_ECB:
+                ctx->base.driver = &qcrypto_nettle_twofish_driver_ecb;
+                break;
+            case QCRYPTO_CIPHER_MODE_CBC:
+                ctx->base.driver = &qcrypto_nettle_twofish_driver_cbc;
+                break;
+            case QCRYPTO_CIPHER_MODE_CTR:
+                ctx->base.driver = &qcrypto_nettle_twofish_driver_ctr;
+                break;
+            case QCRYPTO_CIPHER_MODE_XTS:
+                ctx->base.driver = &qcrypto_nettle_twofish_driver_xts;
+                nkey /= 2;
+                twofish_set_key(&ctx->key_xts, nkey, key + nkey);
+                break;
+            default:
+                g_assert_not_reached();
+            }
+            twofish_set_key(&ctx->key, nkey, key);
 
-            nkey /= 2;
-            twofish_set_key(ctx->ctx, nkey, key);
-            twofish_set_key(ctx->ctx_tweak, nkey, key + nkey);
-        } else {
-            twofish_set_key(ctx->ctx, nkey, key);
+            return &ctx->base;
         }
 
-        ctx->alg_encrypt_native = twofish_encrypt_native;
-        ctx->alg_decrypt_native = twofish_decrypt_native;
-        ctx->alg_encrypt_wrapper = twofish_encrypt_wrapper;
-        ctx->alg_decrypt_wrapper = twofish_decrypt_wrapper;
-
-        ctx->blocksize = TWOFISH_BLOCK_SIZE;
-        break;
-
     default:
         error_setg(errp, "Unsupported cipher algorithm %s",
                    QCryptoCipherAlgorithm_str(alg));
-        goto error;
+        return NULL;
     }
-    g_assert(is_power_of_2(ctx->blocksize));
-
-    if (mode == QCRYPTO_CIPHER_MODE_XTS &&
-        ctx->blocksize != XTS_BLOCK_SIZE) {
-        error_setg(errp, "Cipher block size %zu must equal XTS block size %d",
-                   ctx->blocksize, XTS_BLOCK_SIZE);
-        goto error;
-    }
-
-    ctx->iv = g_new0(uint8_t, ctx->blocksize);
-
-    ctx->base.driver = &qcrypto_cipher_lib_driver;
-    return &ctx->base;
-
- error:
-    qcrypto_nettle_cipher_free_ctx(ctx);
-    return NULL;
 }
-
-
-static void
-qcrypto_nettle_cipher_ctx_free(QCryptoCipher *cipher)
-{
-    QCryptoCipherNettle *ctx = container_of(cipher, QCryptoCipherNettle, base);
-
-    qcrypto_nettle_cipher_free_ctx(ctx);
-}
-
-
-static int
-qcrypto_nettle_cipher_encrypt(QCryptoCipher *cipher,
-                              const void *in,
-                              void *out,
-                              size_t len,
-                              Error **errp)
-{
-    QCryptoCipherNettle *ctx = container_of(cipher, QCryptoCipherNettle, base);
-
-    if (len & (ctx->blocksize - 1)) {
-        error_setg(errp, "Length %zu must be a multiple of block size %zu",
-                   len, ctx->blocksize);
-        return -1;
-    }
-
-    switch (cipher->mode) {
-    case QCRYPTO_CIPHER_MODE_ECB:
-        ctx->alg_encrypt_wrapper(ctx->ctx, len, out, in);
-        break;
-
-    case QCRYPTO_CIPHER_MODE_CBC:
-        cbc_encrypt(ctx->ctx, ctx->alg_encrypt_native,
-                    ctx->blocksize, ctx->iv,
-                    len, out, in);
-        break;
-
-    case QCRYPTO_CIPHER_MODE_XTS:
-#ifdef CONFIG_QEMU_PRIVATE_XTS
-        xts_encrypt(ctx->ctx, ctx->ctx_tweak,
-                    ctx->alg_encrypt_wrapper, ctx->alg_decrypt_wrapper,
-                    ctx->iv, len, out, in);
-#else
-        xts_encrypt_message(ctx->ctx, ctx->ctx_tweak,
-                            ctx->alg_encrypt_native,
-                            ctx->iv, len, out, in);
-#endif
-        break;
-
-    case QCRYPTO_CIPHER_MODE_CTR:
-        ctr_crypt(ctx->ctx, ctx->alg_encrypt_native,
-                    ctx->blocksize, ctx->iv,
-                    len, out, in);
-        break;
-
-    default:
-        error_setg(errp, "Unsupported cipher mode %s",
-                   QCryptoCipherMode_str(cipher->mode));
-        return -1;
-    }
-    return 0;
-}
-
-
-static int
-qcrypto_nettle_cipher_decrypt(QCryptoCipher *cipher,
-                              const void *in,
-                              void *out,
-                              size_t len,
-                              Error **errp)
-{
-    QCryptoCipherNettle *ctx = container_of(cipher, QCryptoCipherNettle, base);
-
-    if (len & (ctx->blocksize - 1)) {
-        error_setg(errp, "Length %zu must be a multiple of block size %zu",
-                   len, ctx->blocksize);
-        return -1;
-    }
-
-    switch (cipher->mode) {
-    case QCRYPTO_CIPHER_MODE_ECB:
-        ctx->alg_decrypt_wrapper(ctx->ctx, len, out, in);
-        break;
-
-    case QCRYPTO_CIPHER_MODE_CBC:
-        cbc_decrypt(ctx->ctx, ctx->alg_decrypt_native,
-                    ctx->blocksize, ctx->iv,
-                    len, out, in);
-        break;
-
-    case QCRYPTO_CIPHER_MODE_XTS:
-#ifdef CONFIG_QEMU_PRIVATE_XTS
-        xts_decrypt(ctx->ctx, ctx->ctx_tweak,
-                    ctx->alg_encrypt_wrapper, ctx->alg_decrypt_wrapper,
-                    ctx->iv, len, out, in);
-#else
-        xts_decrypt_message(ctx->ctx, ctx->ctx_tweak,
-                            ctx->alg_decrypt_native,
-                            ctx->alg_encrypt_native,
-                            ctx->iv, len, out, in);
-#endif
-        break;
-    case QCRYPTO_CIPHER_MODE_CTR:
-        ctr_crypt(ctx->ctx, ctx->alg_encrypt_native,
-                    ctx->blocksize, ctx->iv,
-                    len, out, in);
-        break;
-
-    default:
-        error_setg(errp, "Unsupported cipher mode %s",
-                   QCryptoCipherMode_str(cipher->mode));
-        return -1;
-    }
-    return 0;
-}
-
-static int
-qcrypto_nettle_cipher_setiv(QCryptoCipher *cipher,
-                            const uint8_t *iv, size_t niv,
-                            Error **errp)
-{
-    QCryptoCipherNettle *ctx = container_of(cipher, QCryptoCipherNettle, base);
-
-    if (niv != ctx->blocksize) {
-        error_setg(errp, "Expected IV size %zu not %zu",
-                   ctx->blocksize, niv);
-        return -1;
-    }
-    memcpy(ctx->iv, iv, niv);
-    return 0;
-}
-
-
-static const struct QCryptoCipherDriver qcrypto_cipher_lib_driver = {
-    .cipher_encrypt = qcrypto_nettle_cipher_encrypt,
-    .cipher_decrypt = qcrypto_nettle_cipher_decrypt,
-    .cipher_setiv = qcrypto_nettle_cipher_setiv,
-    .cipher_free = qcrypto_nettle_cipher_ctx_free,
-};
-- 
2.25.1



^ permalink raw reply related	[flat|nested] 52+ messages in thread

* [PATCH 17/17] crypto/gcrypt: Split QCryptoCipherGcrypt into subclasses
  2020-08-13  3:25 [PATCH 00/17] crypto/cipher: Class hierarchy cleanups Richard Henderson
                   ` (15 preceding siblings ...)
  2020-08-13  3:25 ` [PATCH 16/17] crypto/nettle: Split QCryptoCipherNettle " Richard Henderson
@ 2020-08-13  3:25 ` Richard Henderson
  2020-08-28 13:52   ` Daniel P. Berrangé
  2020-08-13  8:54 ` [PATCH 00/17] crypto/cipher: Class hierarchy cleanups Philippe Mathieu-Daudé
                   ` (3 subsequent siblings)
  20 siblings, 1 reply; 52+ messages in thread
From: Richard Henderson @ 2020-08-13  3:25 UTC (permalink / raw)
  To: qemu-devel; +Cc: berrange

With gcrypt, most of the dispatch happens in the library,
so there aren't many classes to create.  However, we can
still create separate dispatch for CTR mode, and for
CONFIG_QEMU_PRIVATE_XTS, which avoids needing to check
for these modes at runtime.

Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
---
 crypto/cipher-gcrypt.inc.c | 512 ++++++++++++++++++-------------------
 1 file changed, 250 insertions(+), 262 deletions(-)

diff --git a/crypto/cipher-gcrypt.inc.c b/crypto/cipher-gcrypt.inc.c
index 7a1fbc9745..1f9d08a7fa 100644
--- a/crypto/cipher-gcrypt.inc.c
+++ b/crypto/cipher-gcrypt.inc.c
@@ -24,8 +24,6 @@
 
 #include <gcrypt.h>
 
-static const struct QCryptoCipherDriver qcrypto_cipher_lib_driver;
-
 bool qcrypto_cipher_supports(QCryptoCipherAlgorithm alg,
                              QCryptoCipherMode mode)
 {
@@ -57,36 +55,215 @@ bool qcrypto_cipher_supports(QCryptoCipherAlgorithm alg,
     }
 }
 
-typedef struct QCryptoCipherGcrypt QCryptoCipherGcrypt;
-struct QCryptoCipherGcrypt {
+typedef struct QCryptoCipherGcrypt {
     QCryptoCipher base;
     gcry_cipher_hd_t handle;
     size_t blocksize;
 #ifdef CONFIG_QEMU_PRIVATE_XTS
     gcry_cipher_hd_t tweakhandle;
-    /* Initialization vector or Counter */
-    uint8_t *iv;
+    uint8_t iv[XTS_BLOCK_SIZE];
 #endif
-};
+} QCryptoCipherGcrypt;
 
-static void
-qcrypto_gcrypt_cipher_free_ctx(QCryptoCipherGcrypt *ctx,
-                               QCryptoCipherMode mode)
+
+static void qcrypto_gcrypt_ctx_free(QCryptoCipher *cipher)
 {
-    if (!ctx) {
-        return;
-    }
+    QCryptoCipherGcrypt *ctx = container_of(cipher, QCryptoCipherGcrypt, base);
 
     gcry_cipher_close(ctx->handle);
-#ifdef CONFIG_QEMU_PRIVATE_XTS
-    if (mode == QCRYPTO_CIPHER_MODE_XTS) {
-        gcry_cipher_close(ctx->tweakhandle);
-    }
-    g_free(ctx->iv);
-#endif
     g_free(ctx);
 }
 
+static int qcrypto_gcrypt_encrypt(QCryptoCipher *cipher, const void *in,
+                                  void *out, size_t len, Error **errp)
+{
+    QCryptoCipherGcrypt *ctx = container_of(cipher, QCryptoCipherGcrypt, base);
+    gcry_error_t err;
+
+    if (len & (ctx->blocksize - 1)) {
+        error_setg(errp, "Length %zu must be a multiple of block size %zu",
+                   len, ctx->blocksize);
+        return -1;
+    }
+
+    err = gcry_cipher_encrypt(ctx->handle, out, len, in, len);
+    if (err != 0) {
+        error_setg(errp, "Cannot encrypt data: %s", gcry_strerror(err));
+        return -1;
+    }
+
+    return 0;
+}
+
+
+static int qcrypto_gcrypt_decrypt(QCryptoCipher *cipher, const void *in,
+                                  void *out, size_t len, Error **errp)
+{
+    QCryptoCipherGcrypt *ctx = container_of(cipher, QCryptoCipherGcrypt, base);
+    gcry_error_t err;
+
+    if (len & (ctx->blocksize - 1)) {
+        error_setg(errp, "Length %zu must be a multiple of block size %zu",
+                   len, ctx->blocksize);
+        return -1;
+    }
+
+    err = gcry_cipher_decrypt(ctx->handle, out, len, in, len);
+    if (err != 0) {
+        error_setg(errp, "Cannot decrypt data: %s",
+                   gcry_strerror(err));
+        return -1;
+    }
+
+    return 0;
+}
+
+static int qcrypto_gcrypt_setiv(QCryptoCipher *cipher,
+                                const uint8_t *iv, size_t niv,
+                                Error **errp)
+{
+    QCryptoCipherGcrypt *ctx = container_of(cipher, QCryptoCipherGcrypt, base);
+    gcry_error_t err;
+
+    if (niv != ctx->blocksize) {
+        error_setg(errp, "Expected IV size %zu not %zu",
+                   ctx->blocksize, niv);
+        return -1;
+    }
+
+    gcry_cipher_reset(ctx->handle);
+    err = gcry_cipher_setiv(ctx->handle, iv, niv);
+    if (err != 0) {
+        error_setg(errp, "Cannot set IV: %s", gcry_strerror(err));
+        return -1;
+    }
+
+    return 0;
+}
+
+static int qcrypto_gcrypt_ctr_setiv(QCryptoCipher *cipher,
+                                    const uint8_t *iv, size_t niv,
+                                    Error **errp)
+{
+    QCryptoCipherGcrypt *ctx = container_of(cipher, QCryptoCipherGcrypt, base);
+    gcry_error_t err;
+
+    if (niv != ctx->blocksize) {
+        error_setg(errp, "Expected IV size %zu not %zu",
+                   ctx->blocksize, niv);
+        return -1;
+    }
+
+    err = gcry_cipher_setctr(ctx->handle, iv, niv);
+    if (err != 0) {
+        error_setg(errp, "Cannot set Counter: %s", gcry_strerror(err));
+        return -1;
+    }
+
+    return 0;
+}
+
+
+static const struct QCryptoCipherDriver qcrypto_gcrypt_driver = {
+    .cipher_encrypt = qcrypto_gcrypt_encrypt,
+    .cipher_decrypt = qcrypto_gcrypt_decrypt,
+    .cipher_setiv = qcrypto_gcrypt_setiv,
+    .cipher_free = qcrypto_gcrypt_ctx_free,
+};
+
+static const struct QCryptoCipherDriver qcrypto_gcrypt_ctr_driver = {
+    .cipher_encrypt = qcrypto_gcrypt_encrypt,
+    .cipher_decrypt = qcrypto_gcrypt_decrypt,
+    .cipher_setiv = qcrypto_gcrypt_ctr_setiv,
+    .cipher_free = qcrypto_gcrypt_ctx_free,
+};
+
+#ifdef CONFIG_QEMU_PRIVATE_XTS
+static void qcrypto_gcrypt_xts_ctx_free(QCryptoCipher *cipher)
+{
+    QCryptoCipherGcrypt *ctx = container_of(cipher, QCryptoCipherGcrypt, base);
+
+    gcry_cipher_close(ctx->tweakhandle);
+    qcrypto_gcrypt_ctx_free(cipher);
+}
+
+static void qcrypto_gcrypt_xts_wrape(const void *ctx, size_t length,
+                                     uint8_t *dst, const uint8_t *src)
+{
+    gcry_error_t err;
+    err = gcry_cipher_encrypt((gcry_cipher_hd_t)ctx, dst, length, src, length);
+    g_assert(err == 0);
+}
+
+static void qcrypto_gcrypt_xts_wrapd(const void *ctx, size_t length,
+                                     uint8_t *dst, const uint8_t *src)
+{
+    gcry_error_t err;
+    err = gcry_cipher_decrypt((gcry_cipher_hd_t)ctx, dst, length, src, length);
+    g_assert(err == 0);
+}
+
+static int qcrypto_gcrypt_xts_encrypt(QCryptoCipher *cipher, const void *in,
+                                      void *out, size_t len, Error **errp)
+{
+    QCryptoCipherGcrypt *ctx = container_of(cipher, QCryptoCipherGcrypt, base);
+    gcry_error_t err;
+
+    if (len & (ctx->blocksize - 1)) {
+        error_setg(errp, "Length %zu must be a multiple of block size %zu",
+                   len, ctx->blocksize);
+        return -1;
+    }
+
+    xts_encrypt(ctx->handle, ctx->tweakhandle,
+                qcrypto_gcrypt_xts_wrape, qcrypto_gcrypt_xts_wrapd,
+                ctx->iv, len, out, in);
+    return 0;
+}
+
+static int qcrypto_gcrypt_xts_decrypt(QCryptoCipher *cipher, const void *in,
+                                      void *out, size_t len, Error **errp)
+{
+    QCryptoCipherGcrypt *ctx = container_of(cipher, QCryptoCipherGcrypt, base);
+    gcry_error_t err;
+
+    if (len & (ctx->blocksize - 1)) {
+        error_setg(errp, "Length %zu must be a multiple of block size %zu",
+                   len, ctx->blocksize);
+        return -1;
+    }
+
+    xts_decrypt(ctx->handle, ctx->tweakhandle,
+                qcrypto_gcrypt_xts_encrypt, qcrypto_gcrypt_xts_decrypt,
+                ctx->iv, len, out, in);
+    return 0;
+}
+
+static int qcrypto_gcrypt_xts_setiv(QCryptoCipher *cipher,
+                                    const uint8_t *iv, size_t niv,
+                                    Error **errp)
+{
+    QCryptoCipherGcrypt *ctx = container_of(cipher, QCryptoCipherGcrypt, base);
+    gcry_error_t err;
+
+    if (niv != ctx->blocksize) {
+        error_setg(errp, "Expected IV size %zu not %zu",
+                   ctx->blocksize, niv);
+        return -1;
+    }
+
+    memcpy(ctx->iv, iv, niv);
+    return 0;
+}
+
+static const struct QCryptoCipherDriver qcrypto_gcrypt_xts_driver = {
+    .cipher_encrypt = qcrypto_gcrypt_xts_encrypt,
+    .cipher_decrypt = qcrypto_gcrypt_xts_decrypt,
+    .cipher_setiv = qcrypto_gcrypt_xts_setiv,
+    .cipher_free = qcrypto_gcrypt_xts_ctx_free,
+};
+#endif /* CONFIG_QEMU_PRIVATE_XTS */
+
 
 static QCryptoCipher *qcrypto_cipher_ctx_new(QCryptoCipherAlgorithm alg,
                                              QCryptoCipherMode mode,
@@ -95,32 +272,10 @@ static QCryptoCipher *qcrypto_cipher_ctx_new(QCryptoCipherAlgorithm alg,
                                              Error **errp)
 {
     QCryptoCipherGcrypt *ctx;
+    const QCryptoCipherDriver *drv;
     gcry_error_t err;
     int gcryalg, gcrymode;
 
-    switch (mode) {
-    case QCRYPTO_CIPHER_MODE_ECB:
-        gcrymode = GCRY_CIPHER_MODE_ECB;
-        break;
-    case QCRYPTO_CIPHER_MODE_XTS:
-#ifdef CONFIG_QEMU_PRIVATE_XTS
-        gcrymode = GCRY_CIPHER_MODE_ECB;
-#else
-        gcrymode = GCRY_CIPHER_MODE_XTS;
-#endif
-        break;
-    case QCRYPTO_CIPHER_MODE_CBC:
-        gcrymode = GCRY_CIPHER_MODE_CBC;
-        break;
-    case QCRYPTO_CIPHER_MODE_CTR:
-        gcrymode = GCRY_CIPHER_MODE_CTR;
-        break;
-    default:
-        error_setg(errp, "Unsupported cipher mode %s",
-                   QCryptoCipherMode_str(mode));
-        return NULL;
-    }
-
     if (!qcrypto_cipher_validate_key_length(alg, mode, nkey, errp)) {
         return NULL;
     }
@@ -129,68 +284,92 @@ static QCryptoCipher *qcrypto_cipher_ctx_new(QCryptoCipherAlgorithm alg,
     case QCRYPTO_CIPHER_ALG_DES_RFB:
         gcryalg = GCRY_CIPHER_DES;
         break;
-
     case QCRYPTO_CIPHER_ALG_3DES:
         gcryalg = GCRY_CIPHER_3DES;
         break;
-
     case QCRYPTO_CIPHER_ALG_AES_128:
         gcryalg = GCRY_CIPHER_AES128;
         break;
-
     case QCRYPTO_CIPHER_ALG_AES_192:
         gcryalg = GCRY_CIPHER_AES192;
         break;
-
     case QCRYPTO_CIPHER_ALG_AES_256:
         gcryalg = GCRY_CIPHER_AES256;
         break;
-
     case QCRYPTO_CIPHER_ALG_CAST5_128:
         gcryalg = GCRY_CIPHER_CAST5;
         break;
-
     case QCRYPTO_CIPHER_ALG_SERPENT_128:
         gcryalg = GCRY_CIPHER_SERPENT128;
         break;
-
     case QCRYPTO_CIPHER_ALG_SERPENT_192:
         gcryalg = GCRY_CIPHER_SERPENT192;
         break;
-
     case QCRYPTO_CIPHER_ALG_SERPENT_256:
         gcryalg = GCRY_CIPHER_SERPENT256;
         break;
-
     case QCRYPTO_CIPHER_ALG_TWOFISH_128:
         gcryalg = GCRY_CIPHER_TWOFISH128;
         break;
-
     case QCRYPTO_CIPHER_ALG_TWOFISH_256:
         gcryalg = GCRY_CIPHER_TWOFISH;
         break;
-
     default:
         error_setg(errp, "Unsupported cipher algorithm %s",
                    QCryptoCipherAlgorithm_str(alg));
         return NULL;
     }
 
+    drv = &qcrypto_gcrypt_driver;
+    switch (mode) {
+    case QCRYPTO_CIPHER_MODE_ECB:
+        gcrymode = GCRY_CIPHER_MODE_ECB;
+        break;
+    case QCRYPTO_CIPHER_MODE_XTS:
+#ifdef CONFIG_QEMU_PRIVATE_XTS
+        drv = &qcrypto_gcrypt_xts_driver;
+        gcrymode = GCRY_CIPHER_MODE_ECB;
+#else
+        gcrymode = GCRY_CIPHER_MODE_XTS;
+#endif
+        break;
+    case QCRYPTO_CIPHER_MODE_CBC:
+        gcrymode = GCRY_CIPHER_MODE_CBC;
+        break;
+    case QCRYPTO_CIPHER_MODE_CTR:
+        drv = &qcrypto_gcrypt_ctr_driver;
+        gcrymode = GCRY_CIPHER_MODE_CTR;
+        break;
+    default:
+        error_setg(errp, "Unsupported cipher mode %s",
+                   QCryptoCipherMode_str(mode));
+        return NULL;
+    }
+
     ctx = g_new0(QCryptoCipherGcrypt, 1);
+    ctx->base.driver = drv;
 
     err = gcry_cipher_open(&ctx->handle, gcryalg, gcrymode, 0);
     if (err != 0) {
         error_setg(errp, "Cannot initialize cipher: %s",
                    gcry_strerror(err));
-        goto error;
+        goto error1;
     }
+    ctx->blocksize = gcry_cipher_get_algo_blklen(gcryalg);
+
 #ifdef CONFIG_QEMU_PRIVATE_XTS
     if (mode == QCRYPTO_CIPHER_MODE_XTS) {
+        if (ctx->blocksize != XTS_BLOCK_SIZE) {
+            error_setg(errp,
+                       "Cipher block size %zu must equal XTS block size %d",
+                       blocksize, XTS_BLOCK_SIZE);
+            goto error2;
+        }
         err = gcry_cipher_open(&ctx->tweakhandle, gcryalg, gcrymode, 0);
         if (err != 0) {
             error_setg(errp, "Cannot initialize cipher: %s",
                        gcry_strerror(err));
-            goto error;
+            goto error2;
         }
     }
 #endif
@@ -203,224 +382,33 @@ static QCryptoCipher *qcrypto_cipher_ctx_new(QCryptoCipherAlgorithm alg,
         uint8_t *rfbkey = qcrypto_cipher_munge_des_rfb_key(key, nkey);
         err = gcry_cipher_setkey(ctx->handle, rfbkey, nkey);
         g_free(rfbkey);
-        ctx->blocksize = 8;
     } else {
 #ifdef CONFIG_QEMU_PRIVATE_XTS
         if (mode == QCRYPTO_CIPHER_MODE_XTS) {
             nkey /= 2;
-            err = gcry_cipher_setkey(ctx->handle, key, nkey);
-            if (err != 0) {
-                error_setg(errp, "Cannot set key: %s",
-                           gcry_strerror(err));
-                goto error;
-            }
             err = gcry_cipher_setkey(ctx->tweakhandle, key + nkey, nkey);
-        } else {
-#endif
-            err = gcry_cipher_setkey(ctx->handle, key, nkey);
-#ifdef CONFIG_QEMU_PRIVATE_XTS
+            if (err != 0) {
+                error_setg(errp, "Cannot set key: %s", gcry_strerror(err));
+                goto error3;
+            }
         }
 #endif
-        if (err != 0) {
-            error_setg(errp, "Cannot set key: %s",
-                       gcry_strerror(err));
-            goto error;
-        }
-        switch (alg) {
-        case QCRYPTO_CIPHER_ALG_AES_128:
-        case QCRYPTO_CIPHER_ALG_AES_192:
-        case QCRYPTO_CIPHER_ALG_AES_256:
-        case QCRYPTO_CIPHER_ALG_SERPENT_128:
-        case QCRYPTO_CIPHER_ALG_SERPENT_192:
-        case QCRYPTO_CIPHER_ALG_SERPENT_256:
-        case QCRYPTO_CIPHER_ALG_TWOFISH_128:
-        case QCRYPTO_CIPHER_ALG_TWOFISH_256:
-            ctx->blocksize = 16;
-            break;
-        case QCRYPTO_CIPHER_ALG_3DES:
-        case QCRYPTO_CIPHER_ALG_CAST5_128:
-            ctx->blocksize = 8;
-            break;
-        default:
-            g_assert_not_reached();
-        }
+        err = gcry_cipher_setkey(ctx->handle, key, nkey);
     }
-    g_assert(is_power_of_2(ctx->blocksize));
-
-#ifdef CONFIG_QEMU_PRIVATE_XTS
-    if (mode == QCRYPTO_CIPHER_MODE_XTS) {
-        if (ctx->blocksize != XTS_BLOCK_SIZE) {
-            error_setg(errp,
-                       "Cipher block size %zu must equal XTS block size %d",
-                       ctx->blocksize, XTS_BLOCK_SIZE);
-            goto error;
-        }
-        ctx->iv = g_new0(uint8_t, ctx->blocksize);
+    if (err != 0) {
+        error_setg(errp, "Cannot set key: %s", gcry_strerror(err));
+        goto error3;
     }
-#endif
 
-    ctx->base.driver = &qcrypto_cipher_lib_driver;
     return &ctx->base;
 
- error:
-    qcrypto_gcrypt_cipher_free_ctx(ctx, mode);
+ error3:
+#ifdef CONFIG_QEMU_PRIVATE_XTS
+    gcry_cipher_close(ctx->tweakhandle);
+ error2:
+#endif
+    gcry_cipher_close(ctx->handle);
+ error1:
+    g_free(ctx);
     return NULL;
 }
-
-
-static void
-qcrypto_gcrypt_cipher_ctx_free(QCryptoCipher *cipher)
-{
-    QCryptoCipherGcrypt *ctx = container_of(cipher, QCryptoCipherGcrypt, base);
-
-    qcrypto_gcrypt_cipher_free_ctx(ctx, cipher->mode);
-}
-
-
-#ifdef CONFIG_QEMU_PRIVATE_XTS
-static void qcrypto_gcrypt_xts_encrypt(const void *ctx,
-                                       size_t length,
-                                       uint8_t *dst,
-                                       const uint8_t *src)
-{
-    gcry_error_t err;
-    err = gcry_cipher_encrypt((gcry_cipher_hd_t)ctx, dst, length, src, length);
-    g_assert(err == 0);
-}
-
-static void qcrypto_gcrypt_xts_decrypt(const void *ctx,
-                                       size_t length,
-                                       uint8_t *dst,
-                                       const uint8_t *src)
-{
-    gcry_error_t err;
-    err = gcry_cipher_decrypt((gcry_cipher_hd_t)ctx, dst, length, src, length);
-    g_assert(err == 0);
-}
-#endif
-
-static int
-qcrypto_gcrypt_cipher_encrypt(QCryptoCipher *cipher,
-                              const void *in,
-                              void *out,
-                              size_t len,
-                              Error **errp)
-{
-    QCryptoCipherGcrypt *ctx = container_of(cipher, QCryptoCipherGcrypt, base);
-    gcry_error_t err;
-
-    if (len & (ctx->blocksize - 1)) {
-        error_setg(errp, "Length %zu must be a multiple of block size %zu",
-                   len, ctx->blocksize);
-        return -1;
-    }
-
-#ifdef CONFIG_QEMU_PRIVATE_XTS
-    if (cipher->mode == QCRYPTO_CIPHER_MODE_XTS) {
-        xts_encrypt(ctx->handle, ctx->tweakhandle,
-                    qcrypto_gcrypt_xts_encrypt,
-                    qcrypto_gcrypt_xts_decrypt,
-                    ctx->iv, len, out, in);
-        return 0;
-    }
-#endif
-
-    err = gcry_cipher_encrypt(ctx->handle,
-                              out, len,
-                              in, len);
-    if (err != 0) {
-        error_setg(errp, "Cannot encrypt data: %s",
-                   gcry_strerror(err));
-        return -1;
-    }
-
-    return 0;
-}
-
-
-static int
-qcrypto_gcrypt_cipher_decrypt(QCryptoCipher *cipher,
-                              const void *in,
-                              void *out,
-                              size_t len,
-                              Error **errp)
-{
-    QCryptoCipherGcrypt *ctx = container_of(cipher, QCryptoCipherGcrypt, base);
-    gcry_error_t err;
-
-    if (len & (ctx->blocksize - 1)) {
-        error_setg(errp, "Length %zu must be a multiple of block size %zu",
-                   len, ctx->blocksize);
-        return -1;
-    }
-
-#ifdef CONFIG_QEMU_PRIVATE_XTS
-    if (cipher->mode == QCRYPTO_CIPHER_MODE_XTS) {
-        xts_decrypt(ctx->handle, ctx->tweakhandle,
-                    qcrypto_gcrypt_xts_encrypt,
-                    qcrypto_gcrypt_xts_decrypt,
-                    ctx->iv, len, out, in);
-        return 0;
-    }
-#endif
-
-    err = gcry_cipher_decrypt(ctx->handle,
-                              out, len,
-                              in, len);
-    if (err != 0) {
-        error_setg(errp, "Cannot decrypt data: %s",
-                   gcry_strerror(err));
-        return -1;
-    }
-
-    return 0;
-}
-
-static int
-qcrypto_gcrypt_cipher_setiv(QCryptoCipher *cipher,
-                            const uint8_t *iv, size_t niv,
-                            Error **errp)
-{
-    QCryptoCipherGcrypt *ctx = container_of(cipher, QCryptoCipherGcrypt, base);
-    gcry_error_t err;
-
-    if (niv != ctx->blocksize) {
-        error_setg(errp, "Expected IV size %zu not %zu",
-                   ctx->blocksize, niv);
-        return -1;
-    }
-
-#ifdef CONFIG_QEMU_PRIVATE_XTS
-    if (ctx->iv) {
-        memcpy(ctx->iv, iv, niv);
-        return 0;
-    }
-#endif
-
-    if (cipher->mode == QCRYPTO_CIPHER_MODE_CTR) {
-        err = gcry_cipher_setctr(ctx->handle, iv, niv);
-        if (err != 0) {
-            error_setg(errp, "Cannot set Counter: %s",
-                       gcry_strerror(err));
-            return -1;
-        }
-    } else {
-        gcry_cipher_reset(ctx->handle);
-        err = gcry_cipher_setiv(ctx->handle, iv, niv);
-        if (err != 0) {
-            error_setg(errp, "Cannot set IV: %s",
-                       gcry_strerror(err));
-            return -1;
-        }
-    }
-
-    return 0;
-}
-
-
-static const struct QCryptoCipherDriver qcrypto_cipher_lib_driver = {
-    .cipher_encrypt = qcrypto_gcrypt_cipher_encrypt,
-    .cipher_decrypt = qcrypto_gcrypt_cipher_decrypt,
-    .cipher_setiv = qcrypto_gcrypt_cipher_setiv,
-    .cipher_free = qcrypto_gcrypt_cipher_ctx_free,
-};
-- 
2.25.1



^ permalink raw reply related	[flat|nested] 52+ messages in thread

* Re: [PATCH 01/17] crypto: Move QCryptoCipher typedef to qemu/typedefs.h
  2020-08-13  3:25 ` [PATCH 01/17] crypto: Move QCryptoCipher typedef to qemu/typedefs.h Richard Henderson
@ 2020-08-13  7:59   ` Philippe Mathieu-Daudé
  2020-08-17 16:48   ` Daniel P. Berrangé
  1 sibling, 0 replies; 52+ messages in thread
From: Philippe Mathieu-Daudé @ 2020-08-13  7:59 UTC (permalink / raw)
  To: Richard Henderson, qemu-devel; +Cc: berrange

On 8/13/20 5:25 AM, Richard Henderson wrote:
> This allows header files to declare pointers without pulling
> in the entire crypto subsystem.
> 
> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
> ---
>  include/crypto/cipher.h | 2 --
>  include/qemu/typedefs.h | 1 +
>  2 files changed, 1 insertion(+), 2 deletions(-)

Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>



^ permalink raw reply	[flat|nested] 52+ messages in thread

* Re: [PATCH 02/17] crypto: Move QCryptoCipherDriver typedef to qemu/typedefs.h
  2020-08-13  3:25 ` [PATCH 02/17] crypto: Move QCryptoCipherDriver " Richard Henderson
@ 2020-08-13  7:59   ` Philippe Mathieu-Daudé
  0 siblings, 0 replies; 52+ messages in thread
From: Philippe Mathieu-Daudé @ 2020-08-13  7:59 UTC (permalink / raw)
  To: Richard Henderson, qemu-devel; +Cc: berrange

On 8/13/20 5:25 AM, Richard Henderson wrote:
> This will allow the pointer to be used in crypto/cipher.h,
> and not just in code using cipherpriv.h.
> 
> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
> ---
>  crypto/cipherpriv.h     | 2 --
>  include/qemu/typedefs.h | 1 +
>  2 files changed, 1 insertion(+), 2 deletions(-)

Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>



^ permalink raw reply	[flat|nested] 52+ messages in thread

* Re: [PATCH 04/17] crypto: Rename cipher include files to .inc.c
  2020-08-13  3:25 ` [PATCH 04/17] crypto: Rename cipher include files to .inc.c Richard Henderson
@ 2020-08-13  8:00   ` Philippe Mathieu-Daudé
  2020-08-17 16:52     ` Daniel P. Berrangé
  0 siblings, 1 reply; 52+ messages in thread
From: Philippe Mathieu-Daudé @ 2020-08-13  8:00 UTC (permalink / raw)
  To: Richard Henderson, qemu-devel; +Cc: berrange

On 8/13/20 5:25 AM, Richard Henderson wrote:
> QEMU standard procedure for included c files is to use *.inc.c.
> E.g. there are a different set of checks that are applied.
> 
> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
> ---
>  crypto/{cipher-builtin.c => cipher-builtin.inc.c} | 0
>  crypto/{cipher-gcrypt.c => cipher-gcrypt.inc.c}   | 0
>  crypto/{cipher-nettle.c => cipher-nettle.inc.c}   | 0
>  crypto/cipher.c                                   | 6 +++---
>  4 files changed, 3 insertions(+), 3 deletions(-)
>  rename crypto/{cipher-builtin.c => cipher-builtin.inc.c} (100%)
>  rename crypto/{cipher-gcrypt.c => cipher-gcrypt.inc.c} (100%)
>  rename crypto/{cipher-nettle.c => cipher-nettle.inc.c} (100%)

Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>
(This clashes with Paolo's Meson series renaming them to .c.inc).



^ permalink raw reply	[flat|nested] 52+ messages in thread

* Re: [PATCH 05/17] crypto: Remove redundant includes
  2020-08-13  3:25 ` [PATCH 05/17] crypto: Remove redundant includes Richard Henderson
@ 2020-08-13  8:01   ` Philippe Mathieu-Daudé
  2020-08-17 16:50   ` Daniel P. Berrangé
  1 sibling, 0 replies; 52+ messages in thread
From: Philippe Mathieu-Daudé @ 2020-08-13  8:01 UTC (permalink / raw)
  To: Richard Henderson, qemu-devel; +Cc: berrange

On 8/13/20 5:25 AM, Richard Henderson wrote:
> Both qemu/osdep.h and cipherpriv.h have already been
> included by the parent cipher.c.
> 
> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
> ---
>  crypto/cipher-builtin.inc.c | 2 --
>  crypto/cipher-gcrypt.inc.c  | 2 --
>  crypto/cipher-nettle.inc.c  | 2 --
>  3 files changed, 6 deletions(-)

Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>



^ permalink raw reply	[flat|nested] 52+ messages in thread

* Re: [PATCH 07/17] crypto: Use the correct const type for driver
  2020-08-13  3:25 ` [PATCH 07/17] crypto: Use the correct const type for driver Richard Henderson
@ 2020-08-13  8:02   ` Philippe Mathieu-Daudé
  2020-08-17 16:53   ` Daniel P. Berrangé
  1 sibling, 0 replies; 52+ messages in thread
From: Philippe Mathieu-Daudé @ 2020-08-13  8:02 UTC (permalink / raw)
  To: Richard Henderson, qemu-devel; +Cc: berrange

On 8/13/20 5:25 AM, Richard Henderson wrote:
> This allows the in memory structures to be read-only.
> 
> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
> ---
>  crypto/cipherpriv.h         |  2 +-
>  include/crypto/cipher.h     |  2 +-
>  crypto/cipher-afalg.c       |  2 +-
>  crypto/cipher-builtin.inc.c |  2 +-
>  crypto/cipher-gcrypt.inc.c  |  2 +-
>  crypto/cipher-nettle.inc.c  |  2 +-
>  crypto/cipher.c             | 12 ++++++------
>  7 files changed, 12 insertions(+), 12 deletions(-)

Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>



^ permalink raw reply	[flat|nested] 52+ messages in thread

* Re: [PATCH 10/17] crypto: Constify cipher data tables
  2020-08-13  3:25 ` [PATCH 10/17] crypto: Constify cipher data tables Richard Henderson
@ 2020-08-13  8:03   ` Philippe Mathieu-Daudé
  2020-08-17 16:56   ` Daniel P. Berrangé
  1 sibling, 0 replies; 52+ messages in thread
From: Philippe Mathieu-Daudé @ 2020-08-13  8:03 UTC (permalink / raw)
  To: Richard Henderson, qemu-devel; +Cc: berrange

On 8/13/20 5:25 AM, Richard Henderson wrote:
> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
> ---
>  crypto/cipher.c | 6 +++---
>  1 file changed, 3 insertions(+), 3 deletions(-)

Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>



^ permalink raw reply	[flat|nested] 52+ messages in thread

* Re: [PATCH 12/17] crypto/builtin: Merge qcrypto_cipher_aes_{ecb, xts}_{en, de}crypt
  2020-08-13  3:25 ` [PATCH 12/17] crypto/builtin: Merge qcrypto_cipher_aes_{ecb, xts}_{en, de}crypt Richard Henderson
@ 2020-08-13  8:05   ` Philippe Mathieu-Daudé
  2020-08-17 17:00   ` [PATCH 12/17] crypto/builtin: Merge qcrypto_cipher_aes_{ecb,xts}_{en,de}crypt Daniel P. Berrangé
  1 sibling, 0 replies; 52+ messages in thread
From: Philippe Mathieu-Daudé @ 2020-08-13  8:05 UTC (permalink / raw)
  To: Richard Henderson, qemu-devel; +Cc: berrange

On 8/13/20 5:25 AM, Richard Henderson wrote:
> There's no real reason we need two separate helper functions here.
> Standardize on the function signature required for xts_encrypt.
> Rename to do_aes_{en,de}crypt_ecb, since the helper does not
> itself do anything with respect to xts.
> 
> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
> ---
>  crypto/cipher-builtin.inc.c | 69 ++++++++++---------------------------
>  1 file changed, 18 insertions(+), 51 deletions(-)

Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>



^ permalink raw reply	[flat|nested] 52+ messages in thread

* Re: [PATCH 13/17] crypto/builtin: Move AES_cbc_encrypt into cipher-builtin.inc.c
  2020-08-13  3:25 ` [PATCH 13/17] crypto/builtin: Move AES_cbc_encrypt into cipher-builtin.inc.c Richard Henderson
@ 2020-08-13  8:11   ` Philippe Mathieu-Daudé
  2020-08-17 17:01   ` Daniel P. Berrangé
  1 sibling, 0 replies; 52+ messages in thread
From: Philippe Mathieu-Daudé @ 2020-08-13  8:11 UTC (permalink / raw)
  To: Richard Henderson, qemu-devel; +Cc: berrange

On 8/13/20 5:25 AM, Richard Henderson wrote:
> By making the function private, we will be able to make further
> simplifications.  Re-indent the migrated code and fix the missing
> braces for CODING_STYLE.

Patch easier to review using 'git-diff --color-moved=dimmed-zebra
--color-moved-ws=allow-indentation-change'.

Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>

> 
> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
> ---
>  include/crypto/aes.h        |  4 ---
>  crypto/aes.c                | 51 ---------------------------------
>  crypto/cipher-builtin.inc.c | 56 +++++++++++++++++++++++++++++++++++++
>  3 files changed, 56 insertions(+), 55 deletions(-)
> 
> diff --git a/include/crypto/aes.h b/include/crypto/aes.h
> index 12fb321b89..ba297d6a73 100644
> --- a/include/crypto/aes.h
> +++ b/include/crypto/aes.h
> @@ -16,7 +16,6 @@ typedef struct aes_key_st AES_KEY;
>  #define AES_set_decrypt_key QEMU_AES_set_decrypt_key
>  #define AES_encrypt QEMU_AES_encrypt
>  #define AES_decrypt QEMU_AES_decrypt
> -#define AES_cbc_encrypt QEMU_AES_cbc_encrypt
>  
>  int AES_set_encrypt_key(const unsigned char *userKey, const int bits,
>  	AES_KEY *key);
> @@ -27,9 +26,6 @@ void AES_encrypt(const unsigned char *in, unsigned char *out,
>  	const AES_KEY *key);
>  void AES_decrypt(const unsigned char *in, unsigned char *out,
>  	const AES_KEY *key);
> -void AES_cbc_encrypt(const unsigned char *in, unsigned char *out,
> -		     const unsigned long length, const AES_KEY *key,
> -		     unsigned char *ivec, const int enc);
>  
>  extern const uint8_t AES_sbox[256];
>  extern const uint8_t AES_isbox[256];
> diff --git a/crypto/aes.c b/crypto/aes.c
> index 0f6a195af8..159800df65 100644
> --- a/crypto/aes.c
> +++ b/crypto/aes.c
> @@ -1599,54 +1599,3 @@ void AES_decrypt(const unsigned char *in, unsigned char *out,
>  }
>  
>  #endif /* AES_ASM */
> -
> -void AES_cbc_encrypt(const unsigned char *in, unsigned char *out,
> -                     const unsigned long length, const AES_KEY *key,
> -                     unsigned char *ivec, const int enc)
> -{
> -
> -        unsigned long n;
> -        unsigned long len = length;
> -        unsigned char tmp[AES_BLOCK_SIZE];
> -
> -        assert(in && out && key && ivec);
> -
> -        if (enc) {
> -                while (len >= AES_BLOCK_SIZE) {
> -                        for(n=0; n < AES_BLOCK_SIZE; ++n)
> -                                tmp[n] = in[n] ^ ivec[n];
> -                        AES_encrypt(tmp, out, key);
> -                        memcpy(ivec, out, AES_BLOCK_SIZE);
> -                        len -= AES_BLOCK_SIZE;
> -                        in += AES_BLOCK_SIZE;
> -                        out += AES_BLOCK_SIZE;
> -                }
> -                if (len) {
> -                        for(n=0; n < len; ++n)
> -                                tmp[n] = in[n] ^ ivec[n];
> -                        for(n=len; n < AES_BLOCK_SIZE; ++n)
> -                                tmp[n] = ivec[n];
> -                        AES_encrypt(tmp, tmp, key);
> -                        memcpy(out, tmp, AES_BLOCK_SIZE);
> -                        memcpy(ivec, tmp, AES_BLOCK_SIZE);
> -                }
> -        } else {
> -                while (len >= AES_BLOCK_SIZE) {
> -                        memcpy(tmp, in, AES_BLOCK_SIZE);
> -                        AES_decrypt(in, out, key);
> -                        for(n=0; n < AES_BLOCK_SIZE; ++n)
> -                                out[n] ^= ivec[n];
> -                        memcpy(ivec, tmp, AES_BLOCK_SIZE);
> -                        len -= AES_BLOCK_SIZE;
> -                        in += AES_BLOCK_SIZE;
> -                        out += AES_BLOCK_SIZE;
> -                }
> -                if (len) {
> -                        memcpy(tmp, in, AES_BLOCK_SIZE);
> -                        AES_decrypt(tmp, tmp, key);
> -                        for(n=0; n < len; ++n)
> -                                out[n] = tmp[n] ^ ivec[n];
> -                        memcpy(ivec, tmp, AES_BLOCK_SIZE);
> -                }
> -        }
> -}
> diff --git a/crypto/cipher-builtin.inc.c b/crypto/cipher-builtin.inc.c
> index 4d971a2b82..416d44b38e 100644
> --- a/crypto/cipher-builtin.inc.c
> +++ b/crypto/cipher-builtin.inc.c
> @@ -100,6 +100,62 @@ static void do_aes_decrypt_ecb(const void *vctx, size_t len,
>      }
>  }
>  
> +static void AES_cbc_encrypt(const unsigned char *in, unsigned char *out,
> +                            const unsigned long length, const AES_KEY *key,
> +                            unsigned char *ivec, const int enc)
> +{
> +    unsigned long n;
> +    unsigned long len = length;
> +    unsigned char tmp[AES_BLOCK_SIZE];
> +
> +    assert(in && out && key && ivec);
> +
> +    if (enc) {
> +        while (len >= AES_BLOCK_SIZE) {
> +            for (n = 0; n < AES_BLOCK_SIZE; ++n) {
> +                tmp[n] = in[n] ^ ivec[n];
> +            }
> +            AES_encrypt(tmp, out, key);
> +            memcpy(ivec, out, AES_BLOCK_SIZE);
> +            len -= AES_BLOCK_SIZE;
> +            in += AES_BLOCK_SIZE;
> +            out += AES_BLOCK_SIZE;
> +        }
> +        if (len) {
> +            for (n = 0; n < len; ++n) {
> +                tmp[n] = in[n] ^ ivec[n];
> +            }
> +            for (n = len; n < AES_BLOCK_SIZE; ++n) {
> +                tmp[n] = ivec[n];
> +            }
> +            AES_encrypt(tmp, tmp, key);
> +            memcpy(out, tmp, AES_BLOCK_SIZE);
> +            memcpy(ivec, tmp, AES_BLOCK_SIZE);
> +        }
> +    } else {
> +        while (len >= AES_BLOCK_SIZE) {
> +            memcpy(tmp, in, AES_BLOCK_SIZE);
> +            AES_decrypt(in, out, key);
> +            for (n = 0; n < AES_BLOCK_SIZE; ++n) {
> +                out[n] ^= ivec[n];
> +            }
> +            memcpy(ivec, tmp, AES_BLOCK_SIZE);
> +            len -= AES_BLOCK_SIZE;
> +            in += AES_BLOCK_SIZE;
> +            out += AES_BLOCK_SIZE;
> +        }
> +        if (len) {
> +            memcpy(tmp, in, AES_BLOCK_SIZE);
> +            AES_decrypt(tmp, tmp, key);
> +            for (n = 0; n < len; ++n) {
> +                out[n] = tmp[n] ^ ivec[n];
> +            }
> +            memcpy(ivec, tmp, AES_BLOCK_SIZE);
> +        }
> +    }
> +}
> +
> +
>  static int qcrypto_cipher_encrypt_aes(QCryptoCipher *cipher,
>                                        const void *in,
>                                        void *out,
> 



^ permalink raw reply	[flat|nested] 52+ messages in thread

* Re: [PATCH 00/17] crypto/cipher: Class hierarchy cleanups
  2020-08-13  3:25 [PATCH 00/17] crypto/cipher: Class hierarchy cleanups Richard Henderson
                   ` (16 preceding siblings ...)
  2020-08-13  3:25 ` [PATCH 17/17] crypto/gcrypt: Split QCryptoCipherGcrypt " Richard Henderson
@ 2020-08-13  8:54 ` Philippe Mathieu-Daudé
  2020-08-13 10:48 ` no-reply
                   ` (2 subsequent siblings)
  20 siblings, 0 replies; 52+ messages in thread
From: Philippe Mathieu-Daudé @ 2020-08-13  8:54 UTC (permalink / raw)
  To: Richard Henderson, qemu-devel; +Cc: berrange

On 8/13/20 5:25 AM, Richard Henderson wrote:
> Mostly this is intended to cleanup the class hierarchy
> used for the ciphers.  We currently have multiple levels
> of dispatch, and multiple separate allocations.  The final
> patches rearrange this to one level of indirect call, and
> all memory allocated contiguously.
> 
> But on the way there are a number of other misc cleanups.
> 
> I know those final patches are somewhat big, but I don't
> immediately see how to split them apart.
> 
> I noticed this while profiling patches to make ARM PAUTH
> use the crypto subsystem.  The qcrypto_cipher_* dispatch
> routines were consuming a noticeable portion of the runtime,
> and with these changes they were down below 1% where they
> ought to be.
> 
> While I did not continue with PAUTH using AES, I still think
> these are good cleanups.
> 
> 
> r~
> 
> 
> Richard Henderson (17):
>   crypto: Move QCryptoCipher typedef to qemu/typedefs.h
>   crypto: Move QCryptoCipherDriver typedef to qemu/typedefs.h
>   crypto: Assume blocksize is a power of 2
>   crypto: Rename cipher include files to .inc.c
>   crypto: Remove redundant includes
>   crypto/nettle: Fix xts_encrypt arguments
>   crypto: Use the correct const type for driver
>   crypto: Allocate QCryptoCipher with the subclass
>   crypto: Move cipher->driver init to qcrypto_*_cipher_ctx_new
>   crypto: Constify cipher data tables
>   crypto/builtin: Remove odd-sized AES block handling
>   crypto/builtin: Merge qcrypto_cipher_aes_{ecb,xts}_{en,de}crypt
>   crypto/builtin: Move AES_cbc_encrypt into cipher-builtin.inc.c
>   crypto/builtin: Split and simplify AES_encrypt_cbc
>   crypto/builtin: Split QCryptoCipherBuiltin into subclasses
>   crypto/nettle: Split QCryptoCipherNettle into subclasses
>   crypto/gcrypt: Split QCryptoCipherGcrypt into subclasses
> 
>  crypto/afalgpriv.h                            |   3 +
>  crypto/cipherpriv.h                           |   6 +-
>  include/crypto/aes.h                          |   4 -
>  include/crypto/cipher.h                       |   5 +-
>  include/qemu/typedefs.h                       |   2 +
>  crypto/aes.c                                  |  51 --
>  crypto/cipher-afalg.c                         |  25 +-
>  crypto/cipher-builtin.c                       | 532 ------------
>  crypto/cipher-builtin.inc.c                   | 425 ++++++++++
>  .../{cipher-gcrypt.c => cipher-gcrypt.inc.c}  | 522 ++++++------
>  crypto/cipher-nettle.c                        | 733 -----------------
>  crypto/cipher-nettle.inc.c                    | 756 ++++++++++++++++++
>  crypto/cipher.c                               |  44 +-
>  13 files changed, 1477 insertions(+), 1631 deletions(-)
>  delete mode 100644 crypto/cipher-builtin.c
>  create mode 100644 crypto/cipher-builtin.inc.c
>  rename crypto/{cipher-gcrypt.c => cipher-gcrypt.inc.c} (51%)
>  delete mode 100644 crypto/cipher-nettle.c
>  create mode 100644 crypto/cipher-nettle.inc.c
> 

Tested-by: Philippe Mathieu-Daudé <philmd@redhat.com>



^ permalink raw reply	[flat|nested] 52+ messages in thread

* Re: [PATCH 00/17] crypto/cipher: Class hierarchy cleanups
  2020-08-13  3:25 [PATCH 00/17] crypto/cipher: Class hierarchy cleanups Richard Henderson
                   ` (17 preceding siblings ...)
  2020-08-13  8:54 ` [PATCH 00/17] crypto/cipher: Class hierarchy cleanups Philippe Mathieu-Daudé
@ 2020-08-13 10:48 ` no-reply
  2020-08-13 11:11 ` no-reply
  2020-08-17 17:09 ` Daniel P. Berrangé
  20 siblings, 0 replies; 52+ messages in thread
From: no-reply @ 2020-08-13 10:48 UTC (permalink / raw)
  To: richard.henderson; +Cc: berrange, qemu-devel

Patchew URL: https://patchew.org/QEMU/20200813032537.2888593-1-richard.henderson@linaro.org/



Hi,

This series seems to have some coding style problems. See output below for
more information:

Type: series
Message-id: 20200813032537.2888593-1-richard.henderson@linaro.org
Subject: [PATCH 00/17] crypto/cipher: Class hierarchy cleanups

=== TEST SCRIPT BEGIN ===
#!/bin/bash
git rev-parse base > /dev/null || exit 0
git config --local diff.renamelimit 0
git config --local diff.renames True
git config --local diff.algorithm histogram
./scripts/checkpatch.pl --mailback base..
=== TEST SCRIPT END ===

From https://github.com/patchew-project/qemu
 - [tag update]      patchew/20200813032537.2888593-1-richard.henderson@linaro.org -> patchew/20200813032537.2888593-1-richard.henderson@linaro.org
Switched to a new branch 'test'
a39d494 crypto/gcrypt: Split QCryptoCipherGcrypt into subclasses
ca787d9 crypto/nettle: Split QCryptoCipherNettle into subclasses
81bf19b crypto/builtin: Split QCryptoCipherBuiltin into subclasses
11aed74 crypto/builtin: Split and simplify AES_encrypt_cbc
c628150 crypto/builtin: Move AES_cbc_encrypt into cipher-builtin.inc.c
3707370 crypto/builtin: Merge qcrypto_cipher_aes_{ecb, xts}_{en, de}crypt
efe1005 crypto/builtin: Remove odd-sized AES block handling
628aa8a crypto: Constify cipher data tables
01f1215 crypto: Move cipher->driver init to qcrypto_*_cipher_ctx_new
93dc38a crypto: Allocate QCryptoCipher with the subclass
81aaa54 crypto: Use the correct const type for driver
8c43775 crypto/nettle: Fix xts_encrypt arguments
f7c0f04 crypto: Remove redundant includes
490fd1b crypto: Rename cipher include files to .inc.c
4ba136f crypto: Assume blocksize is a power of 2
bf305e2 crypto: Move QCryptoCipherDriver typedef to qemu/typedefs.h
680954e crypto: Move QCryptoCipher typedef to qemu/typedefs.h

=== OUTPUT BEGIN ===
1/17 Checking commit 680954e19b44 (crypto: Move QCryptoCipher typedef to qemu/typedefs.h)
2/17 Checking commit bf305e28fb65 (crypto: Move QCryptoCipherDriver typedef to qemu/typedefs.h)
3/17 Checking commit 4ba136fd50f5 (crypto: Assume blocksize is a power of 2)
4/17 Checking commit 490fd1bf6368 (crypto: Rename cipher include files to .inc.c)
WARNING: added, moved or deleted file(s), does MAINTAINERS need updating?
#16: 
rename from crypto/cipher-builtin.c

total: 0 errors, 1 warnings, 14 lines checked

Patch 4/17 has style problems, please review.  If any of these errors
are false positives report them to the maintainer, see
CHECKPATCH in MAINTAINERS.
5/17 Checking commit f7c0f04877bd (crypto: Remove redundant includes)
6/17 Checking commit 8c4377523504 (crypto/nettle: Fix xts_encrypt arguments)
7/17 Checking commit 81aaa54855c5 (crypto: Use the correct const type for driver)
8/17 Checking commit 93dc38a2a468 (crypto: Allocate QCryptoCipher with the subclass)
9/17 Checking commit 01f121573c8b (crypto: Move cipher->driver init to qcrypto_*_cipher_ctx_new)
10/17 Checking commit 628aa8ac5fbc (crypto: Constify cipher data tables)
11/17 Checking commit efe100595b61 (crypto/builtin: Remove odd-sized AES block handling)
12/17 Checking commit 3707370f4f52 (crypto/builtin: Merge qcrypto_cipher_aes_{ecb, xts}_{en, de}crypt)
13/17 Checking commit c6281504f1df (crypto/builtin: Move AES_cbc_encrypt into cipher-builtin.inc.c)
14/17 Checking commit 11aed74bf453 (crypto/builtin: Split and simplify AES_encrypt_cbc)
15/17 Checking commit 81bf19bde54b (crypto/builtin: Split QCryptoCipherBuiltin into subclasses)
16/17 Checking commit ca787d9c34d4 (crypto/nettle: Split QCryptoCipherNettle into subclasses)
ERROR: Macros with multiple statements should be enclosed in a do - while loop
#257: FILE: crypto/cipher-nettle.inc.c:255:
+#define DEFINE_XTS(NAME, TYPE, BLEN, ENCRYPT, DECRYPT)          \
+    QEMU_BUILD_BUG_ON(BLEN != XTS_BLOCK_SIZE);                  \
+    DEFINE__XTS(NAME, TYPE, BLEN, ENCRYPT, DECRYPT)             \
+static const struct QCryptoCipherDriver NAME##_driver_xts = {   \
+    .cipher_encrypt = NAME##_encrypt_xts,                       \
+    .cipher_decrypt = NAME##_decrypt_xts,                       \
+    .cipher_setiv = NAME##_setiv,                               \
+    .cipher_free = qcrypto_cipher_ctx_free,                     \
+};

total: 1 errors, 0 warnings, 973 lines checked

Patch 16/17 has style problems, please review.  If any of these errors
are false positives report them to the maintainer, see
CHECKPATCH in MAINTAINERS.

17/17 Checking commit a39d494a93ce (crypto/gcrypt: Split QCryptoCipherGcrypt into subclasses)
=== OUTPUT END ===

Test command exited with code: 1


The full log is available at
http://patchew.org/logs/20200813032537.2888593-1-richard.henderson@linaro.org/testing.checkpatch/?type=message.
---
Email generated automatically by Patchew [https://patchew.org/].
Please send your feedback to patchew-devel@redhat.com

^ permalink raw reply	[flat|nested] 52+ messages in thread

* Re: [PATCH 00/17] crypto/cipher: Class hierarchy cleanups
  2020-08-13  3:25 [PATCH 00/17] crypto/cipher: Class hierarchy cleanups Richard Henderson
                   ` (18 preceding siblings ...)
  2020-08-13 10:48 ` no-reply
@ 2020-08-13 11:11 ` no-reply
  2020-08-17 16:45   ` Daniel P. Berrangé
  2020-08-17 17:09 ` Daniel P. Berrangé
  20 siblings, 1 reply; 52+ messages in thread
From: no-reply @ 2020-08-13 11:11 UTC (permalink / raw)
  To: richard.henderson; +Cc: berrange, qemu-devel

Patchew URL: https://patchew.org/QEMU/20200813032537.2888593-1-richard.henderson@linaro.org/



Hi,

This series failed the docker-quick@centos7 build test. Please find the testing commands and
their output below. If you have Docker installed, you can probably reproduce it
locally.

=== TEST SCRIPT BEGIN ===
#!/bin/bash
make docker-image-centos7 V=1 NETWORK=1
time make docker-test-quick@centos7 SHOW_ENV=1 J=14 NETWORK=1
=== TEST SCRIPT END ===

  CC      crypto/pbkdf-nettle.o
In file included from /tmp/qemu-test/src/crypto/cipher.c:156:0:
/tmp/qemu-test/src/crypto/cipher-nettle.inc.c: In function 'qcrypto_nettle_aes128_xts_wrape':
/tmp/qemu-test/src/crypto/cipher-nettle.inc.c:185:21: error: passing argument 1 of 'aes128_encrypt_native' discards 'const' qualifier from pointer target type [-Werror]
 static const struct QCryptoCipherDriver NAME##_driver_ctr = {           \
                     ^
/tmp/qemu-test/src/crypto/cipher-nettle.inc.c:270:5: note: in expansion of macro 'DEFINE_CTR'
---
 static void aes128_encrypt_native(cipher_ctx_t ctx, cipher_length_t length,
             ^
/tmp/qemu-test/src/crypto/cipher-nettle.inc.c: In function 'qcrypto_nettle_aes128_xts_wrapd':
/tmp/qemu-test/src/crypto/cipher-nettle.inc.c:185:21: error: passing argument 1 of 'aes128_decrypt_native' discards 'const' qualifier from pointer target type [-Werror]
 static const struct QCryptoCipherDriver NAME##_driver_ctr = {           \
                     ^
/tmp/qemu-test/src/crypto/cipher-nettle.inc.c:270:5: note: in expansion of macro 'DEFINE_CTR'
---
 static void aes128_decrypt_native(cipher_ctx_t ctx, cipher_length_t length,
             ^
/tmp/qemu-test/src/crypto/cipher-nettle.inc.c: In function 'qcrypto_nettle_aes192_xts_wrape':
/tmp/qemu-test/src/crypto/cipher-nettle.inc.c:185:21: error: passing argument 1 of 'aes192_encrypt_native' discards 'const' qualifier from pointer target type [-Werror]
 static const struct QCryptoCipherDriver NAME##_driver_ctr = {           \
                     ^
/tmp/qemu-test/src/crypto/cipher-nettle.inc.c:270:5: note: in expansion of macro 'DEFINE_CTR'
---
 static void aes192_encrypt_native(cipher_ctx_t ctx, cipher_length_t length,
             ^
/tmp/qemu-test/src/crypto/cipher-nettle.inc.c: In function 'qcrypto_nettle_aes192_xts_wrapd':
/tmp/qemu-test/src/crypto/cipher-nettle.inc.c:185:21: error: passing argument 1 of 'aes192_decrypt_native' discards 'const' qualifier from pointer target type [-Werror]
 static const struct QCryptoCipherDriver NAME##_driver_ctr = {           \
                     ^
/tmp/qemu-test/src/crypto/cipher-nettle.inc.c:270:5: note: in expansion of macro 'DEFINE_CTR'
---
 static void aes192_decrypt_native(cipher_ctx_t ctx, cipher_length_t length,
             ^
/tmp/qemu-test/src/crypto/cipher-nettle.inc.c: In function 'qcrypto_nettle_aes256_xts_wrape':
/tmp/qemu-test/src/crypto/cipher-nettle.inc.c:185:21: error: passing argument 1 of 'aes256_encrypt_native' discards 'const' qualifier from pointer target type [-Werror]
 static const struct QCryptoCipherDriver NAME##_driver_ctr = {           \
                     ^
/tmp/qemu-test/src/crypto/cipher-nettle.inc.c:270:5: note: in expansion of macro 'DEFINE_CTR'
---
 static void aes256_encrypt_native(cipher_ctx_t ctx, cipher_length_t length,
             ^
/tmp/qemu-test/src/crypto/cipher-nettle.inc.c: In function 'qcrypto_nettle_aes256_xts_wrapd':
/tmp/qemu-test/src/crypto/cipher-nettle.inc.c:185:21: error: passing argument 1 of 'aes256_decrypt_native' discards 'const' qualifier from pointer target type [-Werror]
 static const struct QCryptoCipherDriver NAME##_driver_ctr = {           \
                     ^
/tmp/qemu-test/src/crypto/cipher-nettle.inc.c:270:5: note: in expansion of macro 'DEFINE_CTR'
---
 static void aes256_decrypt_native(cipher_ctx_t ctx, cipher_length_t length,
             ^
/tmp/qemu-test/src/crypto/cipher-nettle.inc.c: In function 'qcrypto_nettle_serpent_xts_wrape':
/tmp/qemu-test/src/crypto/cipher-nettle.inc.c:185:21: error: passing argument 1 of 'serpent_encrypt_native' discards 'const' qualifier from pointer target type [-Werror]
 static const struct QCryptoCipherDriver NAME##_driver_ctr = {           \
                     ^
/tmp/qemu-test/src/crypto/cipher-nettle.inc.c:270:5: note: in expansion of macro 'DEFINE_CTR'
---
 static void serpent_encrypt_native(cipher_ctx_t ctx, cipher_length_t length,
             ^
/tmp/qemu-test/src/crypto/cipher-nettle.inc.c: In function 'qcrypto_nettle_serpent_xts_wrapd':
/tmp/qemu-test/src/crypto/cipher-nettle.inc.c:185:21: error: passing argument 1 of 'serpent_decrypt_native' discards 'const' qualifier from pointer target type [-Werror]
 static const struct QCryptoCipherDriver NAME##_driver_ctr = {           \
                     ^
/tmp/qemu-test/src/crypto/cipher-nettle.inc.c:270:5: note: in expansion of macro 'DEFINE_CTR'
---
 static void serpent_decrypt_native(cipher_ctx_t ctx, cipher_length_t length,
             ^
/tmp/qemu-test/src/crypto/cipher-nettle.inc.c: In function 'qcrypto_nettle_twofish_xts_wrape':
/tmp/qemu-test/src/crypto/cipher-nettle.inc.c:185:21: error: passing argument 1 of 'twofish_encrypt_native' discards 'const' qualifier from pointer target type [-Werror]
 static const struct QCryptoCipherDriver NAME##_driver_ctr = {           \
                     ^
/tmp/qemu-test/src/crypto/cipher-nettle.inc.c:270:5: note: in expansion of macro 'DEFINE_CTR'
---
 static void twofish_encrypt_native(cipher_ctx_t ctx, cipher_length_t length,
             ^
/tmp/qemu-test/src/crypto/cipher-nettle.inc.c: In function 'qcrypto_nettle_twofish_xts_wrapd':
/tmp/qemu-test/src/crypto/cipher-nettle.inc.c:185:21: error: passing argument 1 of 'twofish_decrypt_native' discards 'const' qualifier from pointer target type [-Werror]
 static const struct QCryptoCipherDriver NAME##_driver_ctr = {           \
                     ^
/tmp/qemu-test/src/crypto/cipher-nettle.inc.c:270:5: note: in expansion of macro 'DEFINE_CTR'
---
 static void twofish_decrypt_native(cipher_ctx_t ctx, cipher_length_t length,
             ^
cc1: all warnings being treated as errors
make: *** [crypto/cipher.o] Error 1
make: *** Waiting for unfinished jobs....
Traceback (most recent call last):
  File "./tests/docker/docker.py", line 709, in <module>
---
    raise CalledProcessError(retcode, cmd)
subprocess.CalledProcessError: Command '['sudo', '-n', 'docker', 'run', '--label', 'com.qemu.instance.uuid=73d17a80a3a34b9caef4371277896653', '-u', '1003', '--security-opt', 'seccomp=unconfined', '--rm', '-e', 'TARGET_LIST=', '-e', 'EXTRA_CONFIGURE_OPTS=', '-e', 'V=', '-e', 'J=14', '-e', 'DEBUG=', '-e', 'SHOW_ENV=1', '-e', 'CCACHE_DIR=/var/tmp/ccache', '-v', '/home/patchew2/.cache/qemu-docker-ccache:/var/tmp/ccache:z', '-v', '/var/tmp/patchew-tester-tmp-k265h_76/src/docker-src.2020-08-13-07.09.18.27263:/var/tmp/qemu:z,ro', 'qemu/centos7', '/var/tmp/qemu/run', 'test-quick']' returned non-zero exit status 2.
filter=--filter=label=com.qemu.instance.uuid=73d17a80a3a34b9caef4371277896653
make[1]: *** [docker-run] Error 1
make[1]: Leaving directory `/var/tmp/patchew-tester-tmp-k265h_76/src'
make: *** [docker-run-test-quick@centos7] Error 2

real    2m20.871s
user    0m8.035s


The full log is available at
http://patchew.org/logs/20200813032537.2888593-1-richard.henderson@linaro.org/testing.docker-quick@centos7/?type=message.
---
Email generated automatically by Patchew [https://patchew.org/].
Please send your feedback to patchew-devel@redhat.com

^ permalink raw reply	[flat|nested] 52+ messages in thread

* Re: [PATCH 00/17] crypto/cipher: Class hierarchy cleanups
  2020-08-13 11:11 ` no-reply
@ 2020-08-17 16:45   ` Daniel P. Berrangé
  0 siblings, 0 replies; 52+ messages in thread
From: Daniel P. Berrangé @ 2020-08-17 16:45 UTC (permalink / raw)
  To: qemu-devel; +Cc: richard.henderson

On Thu, Aug 13, 2020 at 04:11:40AM -0700, no-reply@patchew.org wrote:
> Patchew URL: https://patchew.org/QEMU/20200813032537.2888593-1-richard.henderson@linaro.org/
> 
> 
> 
> Hi,
> 
> This series failed the docker-quick@centos7 build test. Please find the testing commands and
> their output below. If you have Docker installed, you can probably reproduce it
> locally.
> 
> === TEST SCRIPT BEGIN ===
> #!/bin/bash
> make docker-image-centos7 V=1 NETWORK=1
> time make docker-test-quick@centos7 SHOW_ENV=1 J=14 NETWORK=1
> === TEST SCRIPT END ===
> 
>   CC      crypto/pbkdf-nettle.o
> In file included from /tmp/qemu-test/src/crypto/cipher.c:156:0:
> /tmp/qemu-test/src/crypto/cipher-nettle.inc.c: In function 'qcrypto_nettle_aes128_xts_wrape':
> /tmp/qemu-test/src/crypto/cipher-nettle.inc.c:185:21: error: passing argument 1 of 'aes128_encrypt_native' discards 'const' qualifier from pointer target type [-Werror]
>  static const struct QCryptoCipherDriver NAME##_driver_ctr = {           \
>                      ^
> /tmp/qemu-test/src/crypto/cipher-nettle.inc.c:270:5: note: in expansion of macro 'DEFINE_CTR'

Older versions of nettle had a different API declaration for various
functions. This failure suggests the code changes in this series only
work for the modern nettle.


Regards,
Daniel
-- 
|: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org         -o-            https://fstop138.berrange.com :|
|: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|



^ permalink raw reply	[flat|nested] 52+ messages in thread

* Re: [PATCH 01/17] crypto: Move QCryptoCipher typedef to qemu/typedefs.h
  2020-08-13  3:25 ` [PATCH 01/17] crypto: Move QCryptoCipher typedef to qemu/typedefs.h Richard Henderson
  2020-08-13  7:59   ` Philippe Mathieu-Daudé
@ 2020-08-17 16:48   ` Daniel P. Berrangé
  2020-08-17 20:38     ` Richard Henderson
  1 sibling, 1 reply; 52+ messages in thread
From: Daniel P. Berrangé @ 2020-08-17 16:48 UTC (permalink / raw)
  To: Richard Henderson; +Cc: qemu-devel

On Wed, Aug 12, 2020 at 08:25:21PM -0700, Richard Henderson wrote:
> This allows header files to declare pointers without pulling
> in the entire crypto subsystem.
> 
> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
> ---
>  include/crypto/cipher.h | 2 --
>  include/qemu/typedefs.h | 1 +
>  2 files changed, 1 insertion(+), 2 deletions(-)

I'm not in favour of this change or the next. Using #include "cipher.h"
is not a burden on the users of the crypto code. Moving typedefs away
from the associated struct is a step backwards IMHO.

Regards,
Daniel
-- 
|: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org         -o-            https://fstop138.berrange.com :|
|: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|



^ permalink raw reply	[flat|nested] 52+ messages in thread

* Re: [PATCH 05/17] crypto: Remove redundant includes
  2020-08-13  3:25 ` [PATCH 05/17] crypto: Remove redundant includes Richard Henderson
  2020-08-13  8:01   ` Philippe Mathieu-Daudé
@ 2020-08-17 16:50   ` Daniel P. Berrangé
  1 sibling, 0 replies; 52+ messages in thread
From: Daniel P. Berrangé @ 2020-08-17 16:50 UTC (permalink / raw)
  To: Richard Henderson; +Cc: qemu-devel

On Wed, Aug 12, 2020 at 08:25:25PM -0700, Richard Henderson wrote:
> Both qemu/osdep.h and cipherpriv.h have already been
> included by the parent cipher.c.
> 
> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
> ---
>  crypto/cipher-builtin.inc.c | 2 --
>  crypto/cipher-gcrypt.inc.c  | 2 --
>  crypto/cipher-nettle.inc.c  | 2 --
>  3 files changed, 6 deletions(-)

Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>


Regards,
Daniel
-- 
|: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org         -o-            https://fstop138.berrange.com :|
|: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|



^ permalink raw reply	[flat|nested] 52+ messages in thread

* Re: [PATCH 03/17] crypto: Assume blocksize is a power of 2
  2020-08-13  3:25 ` [PATCH 03/17] crypto: Assume blocksize is a power of 2 Richard Henderson
@ 2020-08-17 16:51   ` Daniel P. Berrangé
  0 siblings, 0 replies; 52+ messages in thread
From: Daniel P. Berrangé @ 2020-08-17 16:51 UTC (permalink / raw)
  To: Richard Henderson; +Cc: qemu-devel

On Wed, Aug 12, 2020 at 08:25:23PM -0700, Richard Henderson wrote:
> The check in the encode/decode path using full division has a
> noticeable amount of overhead.  By asserting the blocksize is
> a power of 2, we can reduce this check to a mask.
> 
> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
> ---
>  crypto/cipher-builtin.c | 4 ++--
>  crypto/cipher-gcrypt.c  | 5 +++--
>  crypto/cipher-nettle.c  | 5 +++--
>  crypto/cipher.c         | 1 +
>  4 files changed, 9 insertions(+), 6 deletions(-)

Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>


Regards,
Daniel
-- 
|: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org         -o-            https://fstop138.berrange.com :|
|: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|



^ permalink raw reply	[flat|nested] 52+ messages in thread

* Re: [PATCH 04/17] crypto: Rename cipher include files to .inc.c
  2020-08-13  8:00   ` Philippe Mathieu-Daudé
@ 2020-08-17 16:52     ` Daniel P. Berrangé
  0 siblings, 0 replies; 52+ messages in thread
From: Daniel P. Berrangé @ 2020-08-17 16:52 UTC (permalink / raw)
  To: Philippe Mathieu-Daudé; +Cc: Richard Henderson, qemu-devel

On Thu, Aug 13, 2020 at 10:00:29AM +0200, Philippe Mathieu-Daudé wrote:
> On 8/13/20 5:25 AM, Richard Henderson wrote:
> > QEMU standard procedure for included c files is to use *.inc.c.
> > E.g. there are a different set of checks that are applied.
> > 
> > Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
> > ---
> >  crypto/{cipher-builtin.c => cipher-builtin.inc.c} | 0
> >  crypto/{cipher-gcrypt.c => cipher-gcrypt.inc.c}   | 0
> >  crypto/{cipher-nettle.c => cipher-nettle.inc.c}   | 0
> >  crypto/cipher.c                                   | 6 +++---
> >  4 files changed, 3 insertions(+), 3 deletions(-)
> >  rename crypto/{cipher-builtin.c => cipher-builtin.inc.c} (100%)
> >  rename crypto/{cipher-gcrypt.c => cipher-gcrypt.inc.c} (100%)
> >  rename crypto/{cipher-nettle.c => cipher-nettle.inc.c} (100%)
> 
> Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>
> (This clashes with Paolo's Meson series renaming them to .c.inc).

IIRC, we need to use  c.inc, because Meson has specific semantics
around a file ending in ".c" that we don't want.

Regards,
Daniel
-- 
|: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org         -o-            https://fstop138.berrange.com :|
|: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|



^ permalink raw reply	[flat|nested] 52+ messages in thread

* Re: [PATCH 06/17] crypto/nettle: Fix xts_encrypt arguments
  2020-08-13  3:25 ` [PATCH 06/17] crypto/nettle: Fix xts_encrypt arguments Richard Henderson
@ 2020-08-17 16:53   ` Daniel P. Berrangé
  0 siblings, 0 replies; 52+ messages in thread
From: Daniel P. Berrangé @ 2020-08-17 16:53 UTC (permalink / raw)
  To: Richard Henderson; +Cc: qemu-devel

On Wed, Aug 12, 2020 at 08:25:26PM -0700, Richard Henderson wrote:
> The fourth argument to xts_encrypt should be the decrypt
> callback; we were accidentally passing encrypt twice.
> 
> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
> ---
>  crypto/cipher-nettle.inc.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)

Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>


Regards,
Daniel
-- 
|: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org         -o-            https://fstop138.berrange.com :|
|: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|



^ permalink raw reply	[flat|nested] 52+ messages in thread

* Re: [PATCH 07/17] crypto: Use the correct const type for driver
  2020-08-13  3:25 ` [PATCH 07/17] crypto: Use the correct const type for driver Richard Henderson
  2020-08-13  8:02   ` Philippe Mathieu-Daudé
@ 2020-08-17 16:53   ` Daniel P. Berrangé
  1 sibling, 0 replies; 52+ messages in thread
From: Daniel P. Berrangé @ 2020-08-17 16:53 UTC (permalink / raw)
  To: Richard Henderson; +Cc: qemu-devel

On Wed, Aug 12, 2020 at 08:25:27PM -0700, Richard Henderson wrote:
> This allows the in memory structures to be read-only.
> 
> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
> ---
>  crypto/cipherpriv.h         |  2 +-
>  include/crypto/cipher.h     |  2 +-
>  crypto/cipher-afalg.c       |  2 +-
>  crypto/cipher-builtin.inc.c |  2 +-
>  crypto/cipher-gcrypt.inc.c  |  2 +-
>  crypto/cipher-nettle.inc.c  |  2 +-
>  crypto/cipher.c             | 12 ++++++------
>  7 files changed, 12 insertions(+), 12 deletions(-)

Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>


Regards,
Daniel
-- 
|: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org         -o-            https://fstop138.berrange.com :|
|: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|



^ permalink raw reply	[flat|nested] 52+ messages in thread

* Re: [PATCH 08/17] crypto: Allocate QCryptoCipher with the subclass
  2020-08-13  3:25 ` [PATCH 08/17] crypto: Allocate QCryptoCipher with the subclass Richard Henderson
@ 2020-08-17 16:55   ` Daniel P. Berrangé
  0 siblings, 0 replies; 52+ messages in thread
From: Daniel P. Berrangé @ 2020-08-17 16:55 UTC (permalink / raw)
  To: Richard Henderson; +Cc: qemu-devel

On Wed, Aug 12, 2020 at 08:25:28PM -0700, Richard Henderson wrote:
> Merge the allocation of "opaque" into the allocation of "cipher".
> This is step one in reducing the indirection in these classes.
> 
> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
> ---
>  crypto/afalgpriv.h          |  3 ++
>  crypto/cipherpriv.h         |  2 +-
>  include/crypto/cipher.h     |  1 -
>  crypto/cipher-afalg.c       | 20 ++++++-----
>  crypto/cipher-builtin.inc.c | 68 +++++++++++++++++++------------------
>  crypto/cipher-gcrypt.inc.c  | 23 +++++++------
>  crypto/cipher-nettle.inc.c  | 24 +++++++------
>  crypto/cipher.c             | 20 ++++-------
>  8 files changed, 84 insertions(+), 77 deletions(-)

Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>


Regards,
Daniel
-- 
|: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org         -o-            https://fstop138.berrange.com :|
|: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|



^ permalink raw reply	[flat|nested] 52+ messages in thread

* Re: [PATCH 09/17] crypto: Move cipher->driver init to qcrypto_*_cipher_ctx_new
  2020-08-13  3:25 ` [PATCH 09/17] crypto: Move cipher->driver init to qcrypto_*_cipher_ctx_new Richard Henderson
@ 2020-08-17 16:56   ` Daniel P. Berrangé
  0 siblings, 0 replies; 52+ messages in thread
From: Daniel P. Berrangé @ 2020-08-17 16:56 UTC (permalink / raw)
  To: Richard Henderson; +Cc: qemu-devel

On Wed, Aug 12, 2020 at 08:25:29PM -0700, Richard Henderson wrote:
> The class vtable should be set by the class initializer.
> This will also allow additional subclassing, reducing the
> amount of indirection in the hierarchy.
> 
> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
> ---
>  crypto/cipherpriv.h         | 2 --
>  crypto/cipher-afalg.c       | 5 ++++-
>  crypto/cipher-builtin.inc.c | 4 ++++
>  crypto/cipher-gcrypt.inc.c  | 2 ++
>  crypto/cipher-nettle.inc.c  | 3 +++
>  crypto/cipher.c             | 7 -------
>  6 files changed, 13 insertions(+), 10 deletions(-)

Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>


Regards,
Daniel
-- 
|: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org         -o-            https://fstop138.berrange.com :|
|: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|



^ permalink raw reply	[flat|nested] 52+ messages in thread

* Re: [PATCH 10/17] crypto: Constify cipher data tables
  2020-08-13  3:25 ` [PATCH 10/17] crypto: Constify cipher data tables Richard Henderson
  2020-08-13  8:03   ` Philippe Mathieu-Daudé
@ 2020-08-17 16:56   ` Daniel P. Berrangé
  1 sibling, 0 replies; 52+ messages in thread
From: Daniel P. Berrangé @ 2020-08-17 16:56 UTC (permalink / raw)
  To: Richard Henderson; +Cc: qemu-devel

On Wed, Aug 12, 2020 at 08:25:30PM -0700, Richard Henderson wrote:
> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
> ---
>  crypto/cipher.c | 6 +++---
>  1 file changed, 3 insertions(+), 3 deletions(-)

Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>


Regards,
Daniel
-- 
|: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org         -o-            https://fstop138.berrange.com :|
|: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|



^ permalink raw reply	[flat|nested] 52+ messages in thread

* Re: [PATCH 11/17] crypto/builtin: Remove odd-sized AES block handling
  2020-08-13  3:25 ` [PATCH 11/17] crypto/builtin: Remove odd-sized AES block handling Richard Henderson
@ 2020-08-17 16:57   ` Daniel P. Berrangé
  0 siblings, 0 replies; 52+ messages in thread
From: Daniel P. Berrangé @ 2020-08-17 16:57 UTC (permalink / raw)
  To: Richard Henderson; +Cc: qemu-devel

On Wed, Aug 12, 2020 at 08:25:31PM -0700, Richard Henderson wrote:
> We verified that the data block is properly sized modulo
> AES_BLOCK_SIZE within qcrypto_builtin_cipher_{en,de}crypt.
> Therefore we will never have to handle odd sized blocks.
> 
> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
> ---
>  crypto/cipher-builtin.inc.c | 40 +++++++++++--------------------------
>  1 file changed, 12 insertions(+), 28 deletions(-)

Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>


Regards,
Daniel
-- 
|: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org         -o-            https://fstop138.berrange.com :|
|: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|



^ permalink raw reply	[flat|nested] 52+ messages in thread

* Re: [PATCH 12/17] crypto/builtin: Merge qcrypto_cipher_aes_{ecb,xts}_{en,de}crypt
  2020-08-13  3:25 ` [PATCH 12/17] crypto/builtin: Merge qcrypto_cipher_aes_{ecb, xts}_{en, de}crypt Richard Henderson
  2020-08-13  8:05   ` Philippe Mathieu-Daudé
@ 2020-08-17 17:00   ` Daniel P. Berrangé
  1 sibling, 0 replies; 52+ messages in thread
From: Daniel P. Berrangé @ 2020-08-17 17:00 UTC (permalink / raw)
  To: Richard Henderson; +Cc: qemu-devel

On Wed, Aug 12, 2020 at 08:25:32PM -0700, Richard Henderson wrote:
> There's no real reason we need two separate helper functions here.
> Standardize on the function signature required for xts_encrypt.
> Rename to do_aes_{en,de}crypt_ecb, since the helper does not
> itself do anything with respect to xts.
> 
> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
> ---
>  crypto/cipher-builtin.inc.c | 69 ++++++++++---------------------------
>  1 file changed, 18 insertions(+), 51 deletions(-)
> 
> diff --git a/crypto/cipher-builtin.inc.c b/crypto/cipher-builtin.inc.c
> index e2ae5d090c..4d971a2b82 100644
> --- a/crypto/cipher-builtin.inc.c
> +++ b/crypto/cipher-builtin.inc.c
> @@ -72,65 +72,34 @@ static void qcrypto_cipher_free_aes(QCryptoCipher *cipher)
>      g_free(cipher);
>  }
>  
> -
> -static void qcrypto_cipher_aes_ecb_encrypt(const AES_KEY *key,
> -                                           const void *in,
> -                                           void *out,
> -                                           size_t len)
> +static void do_aes_encrypt_ecb(const void *vctx, size_t len,
> +                               uint8_t *out, const uint8_t *in)

nitpick - stick with the 1-arg-per-line style, instead of packing
args.

>  {
> -    const uint8_t *inptr = in;
> -    uint8_t *outptr = out;
> +    const QCryptoCipherBuiltinAESContext *ctx = vctx;
>  
>      /* We have already verified that len % AES_BLOCK_SIZE == 0. */
>      while (len) {
> -        AES_encrypt(inptr, outptr, key);
> -        inptr += AES_BLOCK_SIZE;
> -        outptr += AES_BLOCK_SIZE;
> +        AES_encrypt(in, out, &ctx->enc);
> +        in += AES_BLOCK_SIZE;
> +        out += AES_BLOCK_SIZE;
>          len -= AES_BLOCK_SIZE;
>      }
>  }
>  
> -
> -static void qcrypto_cipher_aes_ecb_decrypt(const AES_KEY *key,
> -                                           const void *in,
> -                                           void *out,
> -                                           size_t len)
> +static void do_aes_decrypt_ecb(const void *vctx, size_t len,
> +                               uint8_t *out, const uint8_t *in)
>  {
> -    const uint8_t *inptr = in;
> -    uint8_t *outptr = out;
> +    const QCryptoCipherBuiltinAESContext *ctx = vctx;
>  
>      /* We have already verified that len % AES_BLOCK_SIZE == 0. */
>      while (len) {
> -        AES_decrypt(inptr, outptr, key);
> -        inptr += AES_BLOCK_SIZE;
> -        outptr += AES_BLOCK_SIZE;
> +        AES_decrypt(in, out, &ctx->dec);
> +        in += AES_BLOCK_SIZE;
> +        out += AES_BLOCK_SIZE;
>          len -= AES_BLOCK_SIZE;
>      }
>  }
>  
> -
> -static void qcrypto_cipher_aes_xts_encrypt(const void *ctx,
> -                                           size_t length,
> -                                           uint8_t *dst,
> -                                           const uint8_t *src)
> -{
> -    const QCryptoCipherBuiltinAESContext *aesctx = ctx;
> -
> -    qcrypto_cipher_aes_ecb_encrypt(&aesctx->enc, src, dst, length);
> -}
> -
> -
> -static void qcrypto_cipher_aes_xts_decrypt(const void *ctx,
> -                                           size_t length,
> -                                           uint8_t *dst,
> -                                           const uint8_t *src)
> -{
> -    const QCryptoCipherBuiltinAESContext *aesctx = ctx;
> -
> -    qcrypto_cipher_aes_ecb_decrypt(&aesctx->dec, src, dst, length);
> -}
> -
> -

Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>


Regards,
Daniel
-- 
|: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org         -o-            https://fstop138.berrange.com :|
|: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|



^ permalink raw reply	[flat|nested] 52+ messages in thread

* Re: [PATCH 13/17] crypto/builtin: Move AES_cbc_encrypt into cipher-builtin.inc.c
  2020-08-13  3:25 ` [PATCH 13/17] crypto/builtin: Move AES_cbc_encrypt into cipher-builtin.inc.c Richard Henderson
  2020-08-13  8:11   ` Philippe Mathieu-Daudé
@ 2020-08-17 17:01   ` Daniel P. Berrangé
  1 sibling, 0 replies; 52+ messages in thread
From: Daniel P. Berrangé @ 2020-08-17 17:01 UTC (permalink / raw)
  To: Richard Henderson; +Cc: qemu-devel

On Wed, Aug 12, 2020 at 08:25:33PM -0700, Richard Henderson wrote:
> By making the function private, we will be able to make further
> simplifications.  Re-indent the migrated code and fix the missing
> braces for CODING_STYLE.
> 
> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
> ---
>  include/crypto/aes.h        |  4 ---
>  crypto/aes.c                | 51 ---------------------------------
>  crypto/cipher-builtin.inc.c | 56 +++++++++++++++++++++++++++++++++++++
>  3 files changed, 56 insertions(+), 55 deletions(-)

Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>

Regards,
Daniel
-- 
|: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org         -o-            https://fstop138.berrange.com :|
|: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|



^ permalink raw reply	[flat|nested] 52+ messages in thread

* Re: [PATCH 14/17] crypto/builtin: Split and simplify AES_encrypt_cbc
  2020-08-13  3:25 ` [PATCH 14/17] crypto/builtin: Split and simplify AES_encrypt_cbc Richard Henderson
@ 2020-08-17 17:02   ` Daniel P. Berrangé
  0 siblings, 0 replies; 52+ messages in thread
From: Daniel P. Berrangé @ 2020-08-17 17:02 UTC (permalink / raw)
  To: Richard Henderson; +Cc: qemu-devel

On Wed, Aug 12, 2020 at 08:25:34PM -0700, Richard Henderson wrote:
> Split into encrypt/decrypt functions, dropping the "enc" argument.
> Now that the function is private to this file, we know that "len"
> is a multiple of AES_BLOCK_SIZE.  So drop the odd block size code.
> 
> Name the functions do_aes_*crypt_cbc to match the *_ecb functions.
> Reorder and re-type the arguments to match as well.
> 
> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
> ---
>  crypto/cipher-builtin.inc.c | 91 +++++++++++++++----------------------
>  1 file changed, 36 insertions(+), 55 deletions(-)

Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>


Regards,
Daniel
-- 
|: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org         -o-            https://fstop138.berrange.com :|
|: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|



^ permalink raw reply	[flat|nested] 52+ messages in thread

* Re: [PATCH 00/17] crypto/cipher: Class hierarchy cleanups
  2020-08-13  3:25 [PATCH 00/17] crypto/cipher: Class hierarchy cleanups Richard Henderson
                   ` (19 preceding siblings ...)
  2020-08-13 11:11 ` no-reply
@ 2020-08-17 17:09 ` Daniel P. Berrangé
  2020-08-17 20:49   ` Richard Henderson
  20 siblings, 1 reply; 52+ messages in thread
From: Daniel P. Berrangé @ 2020-08-17 17:09 UTC (permalink / raw)
  To: Richard Henderson; +Cc: qemu-devel

On Wed, Aug 12, 2020 at 08:25:20PM -0700, Richard Henderson wrote:
> Mostly this is intended to cleanup the class hierarchy
> used for the ciphers.  We currently have multiple levels
> of dispatch, and multiple separate allocations.  The final
> patches rearrange this to one level of indirect call, and
> all memory allocated contiguously.
> 
> But on the way there are a number of other misc cleanups.
> 
> I know those final patches are somewhat big, but I don't
> immediately see how to split them apart.

Yeah, I can't see a better way off hand.

> I noticed this while profiling patches to make ARM PAUTH
> use the crypto subsystem.  The qcrypto_cipher_* dispatch
> routines were consuming a noticeable portion of the runtime,
> and with these changes they were down below 1% where they
> ought to be.
> 
> While I did not continue with PAUTH using AES, I still think
> these are good cleanups.

They'll probably improve the LUKS block driver performance too.

What were you measuring performance with ?  Did you use the
benchmark-crypto-cipher  program or something else ?

Regards,
Daniel
-- 
|: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org         -o-            https://fstop138.berrange.com :|
|: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|



^ permalink raw reply	[flat|nested] 52+ messages in thread

* Re: [PATCH 01/17] crypto: Move QCryptoCipher typedef to qemu/typedefs.h
  2020-08-17 16:48   ` Daniel P. Berrangé
@ 2020-08-17 20:38     ` Richard Henderson
  2020-08-17 20:42       ` Richard Henderson
  0 siblings, 1 reply; 52+ messages in thread
From: Richard Henderson @ 2020-08-17 20:38 UTC (permalink / raw)
  To: Daniel P. Berrangé; +Cc: qemu-devel

On 8/17/20 9:48 AM, Daniel P. Berrangé wrote:
> On Wed, Aug 12, 2020 at 08:25:21PM -0700, Richard Henderson wrote:
>> This allows header files to declare pointers without pulling
>> in the entire crypto subsystem.
>>
>> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
>> ---
>>  include/crypto/cipher.h | 2 --
>>  include/qemu/typedefs.h | 1 +
>>  2 files changed, 1 insertion(+), 2 deletions(-)
> 
> I'm not in favour of this change or the next. Using #include "cipher.h"
> is not a burden on the users of the crypto code. Moving typedefs away
> from the associated struct is a step backwards IMHO.

Consider if you put a pointer to QCryptoCipher in a relatively generic header
file (e.g. "target/foo/cpu.h"), restricting "cipher.h" to a portion of the
implementation (e.g. target/foo/helper_crypto.c).

This sort of thing is exactly why "qemu/typedefs.h" exists.


r~


^ permalink raw reply	[flat|nested] 52+ messages in thread

* Re: [PATCH 01/17] crypto: Move QCryptoCipher typedef to qemu/typedefs.h
  2020-08-17 20:38     ` Richard Henderson
@ 2020-08-17 20:42       ` Richard Henderson
  2020-08-17 20:50         ` Richard Henderson
  0 siblings, 1 reply; 52+ messages in thread
From: Richard Henderson @ 2020-08-17 20:42 UTC (permalink / raw)
  To: Daniel P. Berrangé; +Cc: qemu-devel

On 8/17/20 1:38 PM, Richard Henderson wrote:
> On 8/17/20 9:48 AM, Daniel P. Berrangé wrote:
>> On Wed, Aug 12, 2020 at 08:25:21PM -0700, Richard Henderson wrote:
>>> This allows header files to declare pointers without pulling
>>> in the entire crypto subsystem.
>>>
>>> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
>>> ---
>>>  include/crypto/cipher.h | 2 --
>>>  include/qemu/typedefs.h | 1 +
>>>  2 files changed, 1 insertion(+), 2 deletions(-)
>>
>> I'm not in favour of this change or the next. Using #include "cipher.h"
>> is not a burden on the users of the crypto code. Moving typedefs away
>> from the associated struct is a step backwards IMHO.
> 
> Consider if you put a pointer to QCryptoCipher in a relatively generic header
> file (e.g. "target/foo/cpu.h"), restricting "cipher.h" to a portion of the
> implementation (e.g. target/foo/helper_crypto.c).
> 
> This sort of thing is exactly why "qemu/typedefs.h" exists.

As for the next patch for QCryptoCipherDriver, I could easily see not moving
the typedef to typedefs.h, but instaed to "crypto.h", where we do in fact want
to declare an incomplete structure.  I think it's a real mistake to be using
void* there at present.


r~


^ permalink raw reply	[flat|nested] 52+ messages in thread

* Re: [PATCH 00/17] crypto/cipher: Class hierarchy cleanups
  2020-08-17 17:09 ` Daniel P. Berrangé
@ 2020-08-17 20:49   ` Richard Henderson
  0 siblings, 0 replies; 52+ messages in thread
From: Richard Henderson @ 2020-08-17 20:49 UTC (permalink / raw)
  To: Daniel P. Berrangé; +Cc: qemu-devel

On 8/17/20 10:09 AM, Daniel P. Berrangé wrote:
> What were you measuring performance with ?  Did you use the
> benchmark-crypto-cipher  program or something else ?

Perf of a boot of an aarch64 kernel, which

  * debug enabled for regression testing,
  * the v8.3-pauth instructions enabled,
  * a local qemu patch to use aes128 for pauth.

I can dig up pointers if you want, but fairly niche.

Because of all the little debug-enabled functions, it meant we were doing a 1
block aes128 encrypt approximately every 40 guest instructions.


r~


^ permalink raw reply	[flat|nested] 52+ messages in thread

* Re: [PATCH 01/17] crypto: Move QCryptoCipher typedef to qemu/typedefs.h
  2020-08-17 20:42       ` Richard Henderson
@ 2020-08-17 20:50         ` Richard Henderson
  2020-08-28 13:28           ` Daniel P. Berrangé
  0 siblings, 1 reply; 52+ messages in thread
From: Richard Henderson @ 2020-08-17 20:50 UTC (permalink / raw)
  To: Daniel P. Berrangé; +Cc: qemu-devel

On 8/17/20 1:42 PM, Richard Henderson wrote:
> On 8/17/20 1:38 PM, Richard Henderson wrote:
>> On 8/17/20 9:48 AM, Daniel P. Berrangé wrote:
>>> On Wed, Aug 12, 2020 at 08:25:21PM -0700, Richard Henderson wrote:
>>>> This allows header files to declare pointers without pulling
>>>> in the entire crypto subsystem.
>>>>
>>>> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
>>>> ---
>>>>  include/crypto/cipher.h | 2 --
>>>>  include/qemu/typedefs.h | 1 +
>>>>  2 files changed, 1 insertion(+), 2 deletions(-)
>>>
>>> I'm not in favour of this change or the next. Using #include "cipher.h"
>>> is not a burden on the users of the crypto code. Moving typedefs away
>>> from the associated struct is a step backwards IMHO.
>>
>> Consider if you put a pointer to QCryptoCipher in a relatively generic header
>> file (e.g. "target/foo/cpu.h"), restricting "cipher.h" to a portion of the
>> implementation (e.g. target/foo/helper_crypto.c).
>>
>> This sort of thing is exactly why "qemu/typedefs.h" exists.
> 
> As for the next patch for QCryptoCipherDriver, I could easily see not moving
> the typedef to typedefs.h, but instaed to "crypto.h", where we do in fact want
> to declare an incomplete structure.  I think it's a real mistake to be using
> void* there at present.

That said, I can drop this first patch because, in the end, I'm *not* going to
put QCryptoCipher in target/arm/cpu.h.


r~


^ permalink raw reply	[flat|nested] 52+ messages in thread

* Re: [PATCH 01/17] crypto: Move QCryptoCipher typedef to qemu/typedefs.h
  2020-08-17 20:50         ` Richard Henderson
@ 2020-08-28 13:28           ` Daniel P. Berrangé
  0 siblings, 0 replies; 52+ messages in thread
From: Daniel P. Berrangé @ 2020-08-28 13:28 UTC (permalink / raw)
  To: Richard Henderson; +Cc: qemu-devel

On Mon, Aug 17, 2020 at 01:50:21PM -0700, Richard Henderson wrote:
> On 8/17/20 1:42 PM, Richard Henderson wrote:
> > On 8/17/20 1:38 PM, Richard Henderson wrote:
> >> On 8/17/20 9:48 AM, Daniel P. Berrangé wrote:
> >>> On Wed, Aug 12, 2020 at 08:25:21PM -0700, Richard Henderson wrote:
> >>>> This allows header files to declare pointers without pulling
> >>>> in the entire crypto subsystem.
> >>>>
> >>>> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
> >>>> ---
> >>>>  include/crypto/cipher.h | 2 --
> >>>>  include/qemu/typedefs.h | 1 +
> >>>>  2 files changed, 1 insertion(+), 2 deletions(-)
> >>>
> >>> I'm not in favour of this change or the next. Using #include "cipher.h"
> >>> is not a burden on the users of the crypto code. Moving typedefs away
> >>> from the associated struct is a step backwards IMHO.
> >>
> >> Consider if you put a pointer to QCryptoCipher in a relatively generic header
> >> file (e.g. "target/foo/cpu.h"), restricting "cipher.h" to a portion of the
> >> implementation (e.g. target/foo/helper_crypto.c).
> >>
> >> This sort of thing is exactly why "qemu/typedefs.h" exists.
> > 
> > As for the next patch for QCryptoCipherDriver, I could easily see not moving
> > the typedef to typedefs.h, but instaed to "crypto.h", where we do in fact want
> > to declare an incomplete structure.  I think it's a real mistake to be using
> > void* there at present.
> 
> That said, I can drop this first patch because, in the end, I'm *not* going to
> put QCryptoCipher in target/arm/cpu.h.

Thanks, I'd appreciate that. We can re-visit the discussion if needs
change again in future.

Regards,
Daniel
-- 
|: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org         -o-            https://fstop138.berrange.com :|
|: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|



^ permalink raw reply	[flat|nested] 52+ messages in thread

* Re: [PATCH 15/17] crypto/builtin: Split QCryptoCipherBuiltin into subclasses
  2020-08-13  3:25 ` [PATCH 15/17] crypto/builtin: Split QCryptoCipherBuiltin into subclasses Richard Henderson
@ 2020-08-28 13:44   ` Daniel P. Berrangé
  0 siblings, 0 replies; 52+ messages in thread
From: Daniel P. Berrangé @ 2020-08-28 13:44 UTC (permalink / raw)
  To: Richard Henderson; +Cc: qemu-devel

On Wed, Aug 12, 2020 at 08:25:35PM -0700, Richard Henderson wrote:
> We had a second set of function pointers in QCryptoCipherBuiltin,
> which are redundant with QCryptoCipherDriver.  Split the AES and
> DES implementations to avoid one level of indirection.
> 
> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
> ---
>  crypto/cipher-builtin.inc.c | 553 +++++++++++++++---------------------
>  1 file changed, 227 insertions(+), 326 deletions(-)

Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>


Regards,
Daniel
-- 
|: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org         -o-            https://fstop138.berrange.com :|
|: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|



^ permalink raw reply	[flat|nested] 52+ messages in thread

* Re: [PATCH 16/17] crypto/nettle: Split QCryptoCipherNettle into subclasses
  2020-08-13  3:25 ` [PATCH 16/17] crypto/nettle: Split QCryptoCipherNettle " Richard Henderson
@ 2020-08-28 13:46   ` Daniel P. Berrangé
  0 siblings, 0 replies; 52+ messages in thread
From: Daniel P. Berrangé @ 2020-08-28 13:46 UTC (permalink / raw)
  To: Richard Henderson; +Cc: qemu-devel

On Wed, Aug 12, 2020 at 08:25:36PM -0700, Richard Henderson wrote:
> Use separate classes for each cipher entry point: des_rfb, des3,
> aes128, aes192, aes256, cast128, serpent, and twofish.
> 
> Generate wrappers for XTS only for CONFIG_QEMU_PRIVATE_XTS.
> This eliminates unreachable wrappers for DES_RFB, DES3 and
> CAST128, which have blocksizes that do not allow XTS mode.
> 
> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
> ---
>  crypto/cipher-nettle.inc.c | 987 +++++++++++++++++++------------------
>  1 file changed, 503 insertions(+), 484 deletions(-)
> 
> diff --git a/crypto/cipher-nettle.inc.c b/crypto/cipher-nettle.inc.c
> index 36d57ef430..a1f4f6eac6 100644
> --- a/crypto/cipher-nettle.inc.c
> +++ b/crypto/cipher-nettle.inc.c
> @@ -34,8 +34,6 @@
>  #include <nettle/xts.h>
>  #endif
>  
> -static const struct QCryptoCipherDriver qcrypto_cipher_lib_driver;
> -
>  typedef void (*QCryptoCipherNettleFuncWrapper)(const void *ctx,
>                                                 size_t length,
>                                                 uint8_t *dst,
> @@ -75,62 +73,212 @@ typedef const void * cipher_ctx_t;
>  typedef size_t       cipher_length_t;
>  #endif
>  
> -typedef struct QCryptoNettleAES128 {
> -    struct aes128_ctx enc;
> -    struct aes128_ctx dec;
> -} QCryptoNettleAES128;
> -
> -typedef struct QCryptoNettleAES192 {
> -    struct aes192_ctx enc;
> -    struct aes192_ctx dec;
> -} QCryptoNettleAES192;
> -
> -typedef struct QCryptoNettleAES256 {
> -    struct aes256_ctx enc;
> -    struct aes256_ctx dec;
> -} QCryptoNettleAES256;
> -
> -static void aes128_encrypt_native(cipher_ctx_t ctx, cipher_length_t length,
> -                                  uint8_t *dst, const uint8_t *src)
> +static inline bool qcrypto_length_check(size_t len, size_t blocksize,
> +                                        Error **errp)
>  {
> -    const QCryptoNettleAES128 *aesctx = ctx;
> -    aes128_encrypt(&aesctx->enc, length, dst, src);
> +    if (unlikely(len & (blocksize - 1))) {
> +        error_setg(errp, "Length %zu must be a multiple of block size %zu",
> +                   len, blocksize);
> +        return false;
> +    }
> +    return true;
>  }
>  
> -static void aes128_decrypt_native(cipher_ctx_t ctx, cipher_length_t length,
> -                                  uint8_t *dst, const uint8_t *src)
> +
> +static void qcrypto_cipher_ctx_free(QCryptoCipher *ctx)
>  {
> -    const QCryptoNettleAES128 *aesctx = ctx;
> -    aes128_decrypt(&aesctx->dec, length, dst, src);
> +    g_free(ctx);
>  }
>  
> -static void aes192_encrypt_native(cipher_ctx_t ctx, cipher_length_t length,
> -                               uint8_t *dst, const uint8_t *src)
> +static int qcrypto_cipher_no_setiv(QCryptoCipher *cipher,
> +                                   const uint8_t *iv, size_t niv,
> +                                   Error **errp)
>  {
> -    const QCryptoNettleAES192 *aesctx = ctx;
> -    aes192_encrypt(&aesctx->enc, length, dst, src);
> +    error_setg(errp, "Setting IV is not supported");
> +    return -1;
>  }
>  
> -static void aes192_decrypt_native(cipher_ctx_t ctx, cipher_length_t length,
> -                               uint8_t *dst, const uint8_t *src)
> -{
> -    const QCryptoNettleAES192 *aesctx = ctx;
> -    aes192_decrypt(&aesctx->dec, length, dst, src);
> +
> +#define DEFINE_SETIV(NAME, TYPE, BLEN)                                  \
> +static int NAME##_setiv(QCryptoCipher *cipher, const uint8_t *iv,       \
> +                        size_t niv, Error **errp)                       \
> +{                                                                       \
> +    TYPE *ctx = container_of(cipher, TYPE, base);                       \
> +    if (niv != BLEN) {                                                  \
> +        error_setg(errp, "Expected IV size %d not %zu", BLEN, niv);     \
> +        return -1;                                                      \
> +    }                                                                   \
> +    memcpy(ctx->iv, iv, niv);                                           \
> +    return 0;                                                           \
>  }
>  
> -static void aes256_encrypt_native(cipher_ctx_t ctx, cipher_length_t length,
> -                               uint8_t *dst, const uint8_t *src)
> -{
> -    const QCryptoNettleAES256 *aesctx = ctx;
> -    aes256_encrypt(&aesctx->enc, length, dst, src);
> -}
>  
> -static void aes256_decrypt_native(cipher_ctx_t ctx, cipher_length_t length,
> -                               uint8_t *dst, const uint8_t *src)
> -{
> -    const QCryptoNettleAES256 *aesctx = ctx;
> -    aes256_decrypt(&aesctx->dec, length, dst, src);
> +#define DEFINE_ECB(NAME, TYPE, BLEN, ENCRYPT, DECRYPT)                  \
> +static int NAME##_encrypt_ecb(QCryptoCipher *cipher, const void *in,    \
> +                              void *out, size_t len, Error **errp)      \
> +{                                                                       \
> +    TYPE *ctx = container_of(cipher, TYPE, base);                       \
> +    if (!qcrypto_length_check(len, BLEN, errp)) {                       \
> +        return -1;                                                      \
> +    }                                                                   \
> +    ENCRYPT(&ctx->key, len, out, in);                                   \
> +    return 0;                                                           \
> +}                                                                       \
> +static int NAME##_decrypt_ecb(QCryptoCipher *cipher, const void *in,    \
> +                              void *out, size_t len, Error **errp)      \
> +{                                                                       \
> +    TYPE *ctx = container_of(cipher, TYPE, base);                       \
> +    if (!qcrypto_length_check(len, BLEN, errp)) {                       \
> +        return -1;                                                      \
> +    }                                                                   \
> +    DECRYPT(&ctx->key, len, out, in);                                   \
> +    return 0;                                                           \
> +}                                                                       \
> +static const struct QCryptoCipherDriver NAME##_driver_ecb = {           \
> +    .cipher_encrypt = NAME##_encrypt_ecb,                               \
> +    .cipher_decrypt = NAME##_decrypt_ecb,                               \
> +    .cipher_setiv = qcrypto_cipher_no_setiv,                            \
> +    .cipher_free = qcrypto_cipher_ctx_free,                             \
> +};
> +
> +
> +#define DEFINE_CBC(NAME, TYPE, BLEN, ENCRYPT, DECRYPT)                  \
> +static int NAME##_encrypt_cbc(QCryptoCipher *cipher, const void *in,    \
> +                              void *out, size_t len, Error **errp)      \
> +{                                                                       \
> +    TYPE *ctx = container_of(cipher, TYPE, base);                       \
> +    if (!qcrypto_length_check(len, BLEN, errp)) {                       \
> +        return -1;                                                      \
> +    }                                                                   \
> +    cbc_encrypt(&ctx->key, ENCRYPT, BLEN, ctx->iv, len, out, in);       \
> +    return 0;                                                           \
> +}                                                                       \
> +static int NAME##_decrypt_cbc(QCryptoCipher *cipher, const void *in,    \
> +                              void *out, size_t len, Error **errp)      \
> +{                                                                       \
> +    TYPE *ctx = container_of(cipher, TYPE, base);                       \
> +    if (!qcrypto_length_check(len, BLEN, errp)) {                       \
> +        return -1;                                                      \
> +    }                                                                   \
> +    cbc_decrypt(&ctx->key, DECRYPT, BLEN, ctx->iv, len, out, in);       \
> +    return 0;                                                           \
> +}                                                                       \
> +static const struct QCryptoCipherDriver NAME##_driver_cbc = {           \
> +    .cipher_encrypt = NAME##_encrypt_cbc,                               \
> +    .cipher_decrypt = NAME##_decrypt_cbc,                               \
> +    .cipher_setiv = NAME##_setiv,                                       \
> +    .cipher_free = qcrypto_cipher_ctx_free,                             \
> +};
> +
> +
> +#define DEFINE_CTR(NAME, TYPE, BLEN, ENCRYPT)                           \
> +static int NAME##_encrypt_ctr(QCryptoCipher *cipher, const void *in,    \
> +                              void *out, size_t len, Error **errp)      \
> +{                                                                       \
> +    TYPE *ctx = container_of(cipher, TYPE, base);                       \
> +    if (!qcrypto_length_check(len, BLEN, errp)) {                       \
> +        return -1;                                                      \
> +    }                                                                   \
> +    ctr_crypt(&ctx->key, ENCRYPT, BLEN, ctx->iv, len, out, in);         \
> +    return 0;                                                           \
> +}                                                                       \
> +static const struct QCryptoCipherDriver NAME##_driver_ctr = {           \
> +    .cipher_encrypt = NAME##_encrypt_ctr,                               \
> +    .cipher_decrypt = NAME##_encrypt_ctr,                               \
> +    .cipher_setiv = NAME##_setiv,                                       \
> +    .cipher_free = qcrypto_cipher_ctx_free,                             \
> +};
> +
> +
> +#ifdef CONFIG_QEMU_PRIVATE_XTS
> +#define DEFINE__XTS(NAME, TYPE, BLEN, ENCRYPT, DECRYPT)                 \
> +static void NAME##_xts_wrape(const void *ctx, size_t length,            \
> +                             uint8_t *dst, const uint8_t *src)          \
> +{                                                                       \
> +    ENCRYPT(ctx, length, dst, src);                                     \
> +}                                                                       \
> +static void NAME##_xts_wrapd(const void *ctx, size_t length,            \
> +                             uint8_t *dst, const uint8_t *src)          \
> +{                                                                       \
> +    DECRYPT(ctx, length, dst, src);                                     \
> +}                                                                       \
> +static int NAME##_encrypt_xts(QCryptoCipher *cipher, const void *in,    \
> +                              void *out, size_t len, Error **errp)      \
> +{                                                                       \
> +    TYPE *ctx = container_of(cipher, TYPE, base);                       \
> +    if (!qcrypto_length_check(len, BLEN, errp)) {                       \
> +        return -1;                                                      \
> +    }                                                                   \
> +    xts_encrypt(&ctx->key, &ctx->key_xts,                               \
> +                NAME##_xts_wrape, NAME##_xts_wrapd,                     \
> +                ctx->iv, len, out, in);                                 \
> +    return 0;                                                           \
> +}                                                                       \
> +static int NAME##_decrypt_xts(QCryptoCipher *cipher, const void *in,    \
> +                              void *out, size_t len, Error **errp)      \
> +{                                                                       \
> +    TYPE *ctx = container_of(cipher, TYPE, base);                       \
> +    if (!qcrypto_length_check(len, BLEN, errp)) {                       \
> +        return -1;                                                      \
> +    }                                                                   \
> +    xts_decrypt(&ctx->key, &ctx->key_xts,                               \
> +                NAME##_xts_wrape, NAME##_xts_wrapd,                     \
> +                ctx->iv, len, out, in);                                 \
> +    return 0;                                                           \
>  }
> +#else
> +#define DEFINE__XTS(NAME, TYPE, BLEN, ENCRYPT, DECRYPT)                 \
> +static int NAME##_encrypt_xts(QCryptoCipher *cipher, const void *in,    \
> +                              void *out, size_t len, Error **errp)      \
> +{                                                                       \
> +    TYPE *ctx = container_of(cipher, TYPE, base);                       \
> +    if (!qcrypto_length_check(len, BLEN, errp)) {                       \
> +        return -1;                                                      \
> +    }                                                                   \
> +    xts_encrypt_message(&ctx->key, &ctx->key_xts, ENCRYPT,              \
> +                        ctx->iv, len, out, in);                         \
> +    return 0;                                                           \
> +}                                                                       \
> +static int NAME##_decrypt_xts(QCryptoCipher *cipher, const void *in,    \
> +                              void *out, size_t len, Error **errp)      \
> +{                                                                       \
> +    TYPE *ctx = container_of(cipher, TYPE, base);                       \
> +    if (!qcrypto_length_check(len, BLEN, errp)) {                       \
> +        return -1;                                                      \
> +    }                                                                   \
> +    xts_decrypt_message(&ctx->key, &ctx->key_xts, DECRYPT, ENCRYPT,     \
> +                        ctx->iv, len, out, in);                         \
> +    return 0;                                                           \
> +}
> +#endif
> +
> +#define DEFINE_XTS(NAME, TYPE, BLEN, ENCRYPT, DECRYPT)          \
> +    QEMU_BUILD_BUG_ON(BLEN != XTS_BLOCK_SIZE);                  \
> +    DEFINE__XTS(NAME, TYPE, BLEN, ENCRYPT, DECRYPT)             \
> +static const struct QCryptoCipherDriver NAME##_driver_xts = {   \
> +    .cipher_encrypt = NAME##_encrypt_xts,                       \
> +    .cipher_decrypt = NAME##_decrypt_xts,                       \
> +    .cipher_setiv = NAME##_setiv,                               \
> +    .cipher_free = qcrypto_cipher_ctx_free,                     \
> +};
> +
> +
> +#define DEFINE_ECB_CBC_CTR(NAME, TYPE, BLEN, ENCRYPT, DECRYPT)  \
> +    DEFINE_SETIV(NAME, TYPE, BLEN)                              \
> +    DEFINE_ECB(NAME, TYPE, BLEN, ENCRYPT, DECRYPT)              \
> +    DEFINE_CBC(NAME, TYPE, BLEN, ENCRYPT, DECRYPT)              \
> +    DEFINE_CTR(NAME, TYPE, BLEN, ENCRYPT)
> +
> +#define DEFINE_ECB_CBC_CTR_XTS(NAME, TYPE, BLEN, ENCRYPT, DECRYPT)      \
> +    DEFINE_ECB_CBC_CTR(NAME, TYPE, BLEN, ENCRYPT, DECRYPT)              \
> +    DEFINE_XTS(NAME, TYPE, BLEN, ENCRYPT, DECRYPT)
> +
> +
> +typedef struct QCryptoNettleDESRFB {
> +    QCryptoCipher base;
> +    struct des_ctx key;
> +    uint8_t iv[DES_BLOCK_SIZE];
> +} QCryptoNettleDESRFB;
>  
>  static void des_encrypt_native(cipher_ctx_t ctx, cipher_length_t length,
>                                 uint8_t *dst, const uint8_t *src)
> @@ -144,6 +292,16 @@ static void des_decrypt_native(cipher_ctx_t ctx, cipher_length_t length,
>      des_decrypt(ctx, length, dst, src);
>  }
>  
> +DEFINE_ECB_CBC_CTR(qcrypto_nettle_des_rfb, QCryptoNettleDESRFB,
> +                   DES_BLOCK_SIZE, des_encrypt_native, des_decrypt_native)
> +
> +
> +typedef struct QCryptoNettleDES3 {
> +    QCryptoCipher base;
> +    struct des3_ctx key;
> +    uint8_t iv[DES3_BLOCK_SIZE];
> +} QCryptoNettleDES3;
> +
>  static void des3_encrypt_native(cipher_ctx_t ctx, cipher_length_t length,
>                                  uint8_t *dst, const uint8_t *src)
>  {
> @@ -156,6 +314,94 @@ static void des3_decrypt_native(cipher_ctx_t ctx, cipher_length_t length,
>      des3_decrypt(ctx, length, dst, src);
>  }
>  
> +DEFINE_ECB_CBC_CTR(qcrypto_nettle_des3, QCryptoNettleDES3, DES3_BLOCK_SIZE,
> +                   des3_encrypt_native, des3_decrypt_native)
> +
> +
> +typedef struct QCryptoNettleAES128 {
> +    QCryptoCipher base;
> +    uint8_t iv[AES_BLOCK_SIZE];
> +    /* First key from pair is encode, second key is decode. */
> +    struct aes128_ctx key[2], key_xts[2];
> +} QCryptoNettleAES128;
> +
> +static void aes128_encrypt_native(cipher_ctx_t ctx, cipher_length_t length,
> +                                  uint8_t *dst, const uint8_t *src)
> +{
> +    const struct aes128_ctx *keys = ctx;
> +    aes128_encrypt(&keys[0], length, dst, src);
> +}
> +
> +static void aes128_decrypt_native(cipher_ctx_t ctx, cipher_length_t length,
> +                                  uint8_t *dst, const uint8_t *src)
> +{
> +    const struct aes128_ctx *keys = ctx;
> +    aes128_decrypt(&keys[1], length, dst, src);
> +}
> +
> +DEFINE_ECB_CBC_CTR_XTS(qcrypto_nettle_aes128,
> +                       QCryptoNettleAES128, AES_BLOCK_SIZE,
> +                       aes128_encrypt_native, aes128_decrypt_native)
> +
> +
> +typedef struct QCryptoNettleAES192 {
> +    QCryptoCipher base;
> +    uint8_t iv[AES_BLOCK_SIZE];
> +    /* First key from pair is encode, second key is decode. */
> +    struct aes192_ctx key[2], key_xts[2];
> +} QCryptoNettleAES192;
> +
> +static void aes192_encrypt_native(cipher_ctx_t ctx, cipher_length_t length,
> +                                  uint8_t *dst, const uint8_t *src)
> +{
> +    const struct aes192_ctx *keys = ctx;
> +    aes192_encrypt(&keys[0], length, dst, src);
> +}
> +
> +static void aes192_decrypt_native(cipher_ctx_t ctx, cipher_length_t length,
> +                                  uint8_t *dst, const uint8_t *src)
> +{
> +    const struct aes192_ctx *keys = ctx;
> +    aes192_decrypt(&keys[1], length, dst, src);
> +}
> +
> +DEFINE_ECB_CBC_CTR_XTS(qcrypto_nettle_aes192,
> +                       QCryptoNettleAES192, AES_BLOCK_SIZE,
> +                       aes192_encrypt_native, aes192_decrypt_native)
> +
> +
> +typedef struct QCryptoNettleAES256 {
> +    QCryptoCipher base;
> +    uint8_t iv[AES_BLOCK_SIZE];
> +    /* First key from pair is encode, second key is decode. */
> +    struct aes256_ctx key[2], key_xts[2];
> +} QCryptoNettleAES256;
> +
> +static void aes256_encrypt_native(cipher_ctx_t ctx, cipher_length_t length,
> +                                  uint8_t *dst, const uint8_t *src)
> +{
> +    const struct aes256_ctx *keys = ctx;
> +    aes256_encrypt(&keys[0], length, dst, src);
> +}
> +
> +static void aes256_decrypt_native(cipher_ctx_t ctx, cipher_length_t length,
> +                               uint8_t *dst, const uint8_t *src)
> +{
> +    const struct aes256_ctx *keys = ctx;
> +    aes256_decrypt(&keys[1], length, dst, src);
> +}
> +
> +DEFINE_ECB_CBC_CTR_XTS(qcrypto_nettle_aes256,
> +                       QCryptoNettleAES256, AES_BLOCK_SIZE,
> +                       aes256_encrypt_native, aes256_decrypt_native)
> +
> +
> +typedef struct QCryptoNettleCAST128 {
> +    QCryptoCipher base;
> +    uint8_t iv[CAST128_BLOCK_SIZE];
> +    struct cast128_ctx key, key_xts;
> +} QCryptoNettleCAST128;
> +
>  static void cast128_encrypt_native(cipher_ctx_t ctx, cipher_length_t length,
>                                     uint8_t *dst, const uint8_t *src)
>  {
> @@ -168,6 +414,18 @@ static void cast128_decrypt_native(cipher_ctx_t ctx, cipher_length_t length,
>      cast128_decrypt(ctx, length, dst, src);
>  }
>  
> +DEFINE_ECB_CBC_CTR(qcrypto_nettle_cast128,
> +                   QCryptoNettleCAST128, CAST128_BLOCK_SIZE,
> +                   cast128_encrypt_native, cast128_decrypt_native)
> +
> +
> +typedef struct QCryptoNettleSerpent {
> +    QCryptoCipher base;
> +    uint8_t iv[SERPENT_BLOCK_SIZE];
> +    struct serpent_ctx key, key_xts;
> +} QCryptoNettleSerpent;
> +
> +
>  static void serpent_encrypt_native(cipher_ctx_t ctx, cipher_length_t length,
>                                     uint8_t *dst, const uint8_t *src)
>  {
> @@ -180,6 +438,17 @@ static void serpent_decrypt_native(cipher_ctx_t ctx, cipher_length_t length,
>      serpent_decrypt(ctx, length, dst, src);
>  }
>  
> +DEFINE_ECB_CBC_CTR_XTS(qcrypto_nettle_serpent,
> +                       QCryptoNettleSerpent, SERPENT_BLOCK_SIZE,
> +                       serpent_encrypt_native, serpent_decrypt_native)
> +
> +
> +typedef struct QCryptoNettleTwofish {
> +    QCryptoCipher base;
> +    uint8_t iv[TWOFISH_BLOCK_SIZE];
> +    struct twofish_ctx key, key_xts;
> +} QCryptoNettleTwofish;
> +
>  static void twofish_encrypt_native(cipher_ctx_t ctx, cipher_length_t length,
>                                     uint8_t *dst, const uint8_t *src)
>  {
> @@ -192,125 +461,10 @@ static void twofish_decrypt_native(cipher_ctx_t ctx, cipher_length_t length,
>      twofish_decrypt(ctx, length, dst, src);
>  }
>  
> -static void aes128_encrypt_wrapper(const void *ctx, size_t length,
> -                                uint8_t *dst, const uint8_t *src)
> -{
> -    const QCryptoNettleAES128 *aesctx = ctx;
> -    aes128_encrypt(&aesctx->enc, length, dst, src);
> -}
> +DEFINE_ECB_CBC_CTR_XTS(qcrypto_nettle_twofish,
> +                       QCryptoNettleTwofish, TWOFISH_BLOCK_SIZE,
> +                       twofish_encrypt_native, twofish_decrypt_native)
>  
> -static void aes128_decrypt_wrapper(const void *ctx, size_t length,
> -                                uint8_t *dst, const uint8_t *src)
> -{
> -    const QCryptoNettleAES128 *aesctx = ctx;
> -    aes128_decrypt(&aesctx->dec, length, dst, src);
> -}
> -
> -static void aes192_encrypt_wrapper(const void *ctx, size_t length,
> -                                uint8_t *dst, const uint8_t *src)
> -{
> -    const QCryptoNettleAES192 *aesctx = ctx;
> -    aes192_encrypt(&aesctx->enc, length, dst, src);
> -}
> -
> -static void aes192_decrypt_wrapper(const void *ctx, size_t length,
> -                                uint8_t *dst, const uint8_t *src)
> -{
> -    const QCryptoNettleAES192 *aesctx = ctx;
> -    aes192_decrypt(&aesctx->dec, length, dst, src);
> -}
> -
> -static void aes256_encrypt_wrapper(const void *ctx, size_t length,
> -                                uint8_t *dst, const uint8_t *src)
> -{
> -    const QCryptoNettleAES256 *aesctx = ctx;
> -    aes256_encrypt(&aesctx->enc, length, dst, src);
> -}
> -
> -static void aes256_decrypt_wrapper(const void *ctx, size_t length,
> -                                uint8_t *dst, const uint8_t *src)
> -{
> -    const QCryptoNettleAES256 *aesctx = ctx;
> -    aes256_decrypt(&aesctx->dec, length, dst, src);
> -}
> -
> -static void des_encrypt_wrapper(const void *ctx, size_t length,
> -                                uint8_t *dst, const uint8_t *src)
> -{
> -    des_encrypt(ctx, length, dst, src);
> -}
> -
> -static void des_decrypt_wrapper(const void *ctx, size_t length,
> -                                uint8_t *dst, const uint8_t *src)
> -{
> -    des_decrypt(ctx, length, dst, src);
> -}
> -
> -static void des3_encrypt_wrapper(const void *ctx, size_t length,
> -                                uint8_t *dst, const uint8_t *src)
> -{
> -    des3_encrypt(ctx, length, dst, src);
> -}
> -
> -static void des3_decrypt_wrapper(const void *ctx, size_t length,
> -                                uint8_t *dst, const uint8_t *src)
> -{
> -    des3_decrypt(ctx, length, dst, src);
> -}
> -
> -static void cast128_encrypt_wrapper(const void *ctx, size_t length,
> -                                    uint8_t *dst, const uint8_t *src)
> -{
> -    cast128_encrypt(ctx, length, dst, src);
> -}
> -
> -static void cast128_decrypt_wrapper(const void *ctx, size_t length,
> -                                    uint8_t *dst, const uint8_t *src)
> -{
> -    cast128_decrypt(ctx, length, dst, src);
> -}
> -
> -static void serpent_encrypt_wrapper(const void *ctx, size_t length,
> -                                    uint8_t *dst, const uint8_t *src)
> -{
> -    serpent_encrypt(ctx, length, dst, src);
> -}
> -
> -static void serpent_decrypt_wrapper(const void *ctx, size_t length,
> -                                    uint8_t *dst, const uint8_t *src)
> -{
> -    serpent_decrypt(ctx, length, dst, src);
> -}
> -
> -static void twofish_encrypt_wrapper(const void *ctx, size_t length,
> -                                    uint8_t *dst, const uint8_t *src)
> -{
> -    twofish_encrypt(ctx, length, dst, src);
> -}
> -
> -static void twofish_decrypt_wrapper(const void *ctx, size_t length,
> -                                    uint8_t *dst, const uint8_t *src)
> -{
> -    twofish_decrypt(ctx, length, dst, src);
> -}
> -
> -typedef struct QCryptoCipherNettle QCryptoCipherNettle;
> -struct QCryptoCipherNettle {
> -    QCryptoCipher base;
> -
> -    /* Primary cipher context for all modes */
> -    void *ctx;
> -    /* Second cipher context for XTS mode only */
> -    void *ctx_tweak;
> -    /* Cipher callbacks for both contexts */
> -    QCryptoCipherNettleFuncNative alg_encrypt_native;
> -    QCryptoCipherNettleFuncNative alg_decrypt_native;
> -    QCryptoCipherNettleFuncWrapper alg_encrypt_wrapper;
> -    QCryptoCipherNettleFuncWrapper alg_decrypt_wrapper;
> -    /* Initialization vector or Counter */
> -    uint8_t *iv;
> -    size_t blocksize;
> -};
>  
>  bool qcrypto_cipher_supports(QCryptoCipherAlgorithm alg,
>                               QCryptoCipherMode mode)
> @@ -344,30 +498,12 @@ bool qcrypto_cipher_supports(QCryptoCipherAlgorithm alg,
>      }
>  }
>  
> -
> -static void
> -qcrypto_nettle_cipher_free_ctx(QCryptoCipherNettle *ctx)
> -{
> -    if (!ctx) {
> -        return;
> -    }
> -
> -    g_free(ctx->iv);
> -    g_free(ctx->ctx);
> -    g_free(ctx->ctx_tweak);
> -    g_free(ctx);
> -}
> -
> -
>  static QCryptoCipher *qcrypto_cipher_ctx_new(QCryptoCipherAlgorithm alg,
>                                               QCryptoCipherMode mode,
>                                               const uint8_t *key,
>                                               size_t nkey,
>                                               Error **errp)
>  {
> -    QCryptoCipherNettle *ctx;
> -    uint8_t *rfbkey;
> -
>      switch (mode) {
>      case QCRYPTO_CIPHER_MODE_ECB:
>      case QCRYPTO_CIPHER_MODE_CBC:
> @@ -375,6 +511,7 @@ static QCryptoCipher *qcrypto_cipher_ctx_new(QCryptoCipherAlgorithm alg,
>      case QCRYPTO_CIPHER_MODE_CTR:
>          break;
>      default:
> +    bad_cipher_mode:
>          error_setg(errp, "Unsupported cipher mode %s",
>                     QCryptoCipherMode_str(mode));

Lets put this jump target at the end of the method, so we avoid having
to jump backwards in the code.


Aside from that

Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>



Regards,
Daniel
-- 
|: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org         -o-            https://fstop138.berrange.com :|
|: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|



^ permalink raw reply	[flat|nested] 52+ messages in thread

* Re: [PATCH 17/17] crypto/gcrypt: Split QCryptoCipherGcrypt into subclasses
  2020-08-13  3:25 ` [PATCH 17/17] crypto/gcrypt: Split QCryptoCipherGcrypt " Richard Henderson
@ 2020-08-28 13:52   ` Daniel P. Berrangé
  0 siblings, 0 replies; 52+ messages in thread
From: Daniel P. Berrangé @ 2020-08-28 13:52 UTC (permalink / raw)
  To: Richard Henderson; +Cc: qemu-devel

On Wed, Aug 12, 2020 at 08:25:37PM -0700, Richard Henderson wrote:
> With gcrypt, most of the dispatch happens in the library,
> so there aren't many classes to create.  However, we can
> still create separate dispatch for CTR mode, and for
> CONFIG_QEMU_PRIVATE_XTS, which avoids needing to check
> for these modes at runtime.
> 
> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
> ---
>  crypto/cipher-gcrypt.inc.c | 512 ++++++++++++++++++-------------------
>  1 file changed, 250 insertions(+), 262 deletions(-)
> 
> diff --git a/crypto/cipher-gcrypt.inc.c b/crypto/cipher-gcrypt.inc.c
> index 7a1fbc9745..1f9d08a7fa 100644
> --- a/crypto/cipher-gcrypt.inc.c
> +++ b/crypto/cipher-gcrypt.inc.c
> @@ -24,8 +24,6 @@
>  

>  
> - error:
> -    qcrypto_gcrypt_cipher_free_ctx(ctx, mode);
> + error3:
> +#ifdef CONFIG_QEMU_PRIVATE_XTS
> +    gcry_cipher_close(ctx->tweakhandle);
> + error2:
> +#endif
> +    gcry_cipher_close(ctx->handle);
> + error1:
> +    g_free(ctx);

gcry_cipher_close is a no-op if the handle is NULL, so I don't
think we need to have a chain of jump targets, just a single
"error:".


Regards,
Daniel
-- 
|: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org         -o-            https://fstop138.berrange.com :|
|: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|



^ permalink raw reply	[flat|nested] 52+ messages in thread

end of thread, other threads:[~2020-08-28 13:53 UTC | newest]

Thread overview: 52+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-08-13  3:25 [PATCH 00/17] crypto/cipher: Class hierarchy cleanups Richard Henderson
2020-08-13  3:25 ` [PATCH 01/17] crypto: Move QCryptoCipher typedef to qemu/typedefs.h Richard Henderson
2020-08-13  7:59   ` Philippe Mathieu-Daudé
2020-08-17 16:48   ` Daniel P. Berrangé
2020-08-17 20:38     ` Richard Henderson
2020-08-17 20:42       ` Richard Henderson
2020-08-17 20:50         ` Richard Henderson
2020-08-28 13:28           ` Daniel P. Berrangé
2020-08-13  3:25 ` [PATCH 02/17] crypto: Move QCryptoCipherDriver " Richard Henderson
2020-08-13  7:59   ` Philippe Mathieu-Daudé
2020-08-13  3:25 ` [PATCH 03/17] crypto: Assume blocksize is a power of 2 Richard Henderson
2020-08-17 16:51   ` Daniel P. Berrangé
2020-08-13  3:25 ` [PATCH 04/17] crypto: Rename cipher include files to .inc.c Richard Henderson
2020-08-13  8:00   ` Philippe Mathieu-Daudé
2020-08-17 16:52     ` Daniel P. Berrangé
2020-08-13  3:25 ` [PATCH 05/17] crypto: Remove redundant includes Richard Henderson
2020-08-13  8:01   ` Philippe Mathieu-Daudé
2020-08-17 16:50   ` Daniel P. Berrangé
2020-08-13  3:25 ` [PATCH 06/17] crypto/nettle: Fix xts_encrypt arguments Richard Henderson
2020-08-17 16:53   ` Daniel P. Berrangé
2020-08-13  3:25 ` [PATCH 07/17] crypto: Use the correct const type for driver Richard Henderson
2020-08-13  8:02   ` Philippe Mathieu-Daudé
2020-08-17 16:53   ` Daniel P. Berrangé
2020-08-13  3:25 ` [PATCH 08/17] crypto: Allocate QCryptoCipher with the subclass Richard Henderson
2020-08-17 16:55   ` Daniel P. Berrangé
2020-08-13  3:25 ` [PATCH 09/17] crypto: Move cipher->driver init to qcrypto_*_cipher_ctx_new Richard Henderson
2020-08-17 16:56   ` Daniel P. Berrangé
2020-08-13  3:25 ` [PATCH 10/17] crypto: Constify cipher data tables Richard Henderson
2020-08-13  8:03   ` Philippe Mathieu-Daudé
2020-08-17 16:56   ` Daniel P. Berrangé
2020-08-13  3:25 ` [PATCH 11/17] crypto/builtin: Remove odd-sized AES block handling Richard Henderson
2020-08-17 16:57   ` Daniel P. Berrangé
2020-08-13  3:25 ` [PATCH 12/17] crypto/builtin: Merge qcrypto_cipher_aes_{ecb, xts}_{en, de}crypt Richard Henderson
2020-08-13  8:05   ` Philippe Mathieu-Daudé
2020-08-17 17:00   ` [PATCH 12/17] crypto/builtin: Merge qcrypto_cipher_aes_{ecb,xts}_{en,de}crypt Daniel P. Berrangé
2020-08-13  3:25 ` [PATCH 13/17] crypto/builtin: Move AES_cbc_encrypt into cipher-builtin.inc.c Richard Henderson
2020-08-13  8:11   ` Philippe Mathieu-Daudé
2020-08-17 17:01   ` Daniel P. Berrangé
2020-08-13  3:25 ` [PATCH 14/17] crypto/builtin: Split and simplify AES_encrypt_cbc Richard Henderson
2020-08-17 17:02   ` Daniel P. Berrangé
2020-08-13  3:25 ` [PATCH 15/17] crypto/builtin: Split QCryptoCipherBuiltin into subclasses Richard Henderson
2020-08-28 13:44   ` Daniel P. Berrangé
2020-08-13  3:25 ` [PATCH 16/17] crypto/nettle: Split QCryptoCipherNettle " Richard Henderson
2020-08-28 13:46   ` Daniel P. Berrangé
2020-08-13  3:25 ` [PATCH 17/17] crypto/gcrypt: Split QCryptoCipherGcrypt " Richard Henderson
2020-08-28 13:52   ` Daniel P. Berrangé
2020-08-13  8:54 ` [PATCH 00/17] crypto/cipher: Class hierarchy cleanups Philippe Mathieu-Daudé
2020-08-13 10:48 ` no-reply
2020-08-13 11:11 ` no-reply
2020-08-17 16:45   ` Daniel P. Berrangé
2020-08-17 17:09 ` Daniel P. Berrangé
2020-08-17 20:49   ` Richard Henderson

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).