All of lore.kernel.org
 help / color / mirror / Atom feed
* [Qemu-devel] [PATCH v3 0/3] crypto: nettle fixes
@ 2015-07-16 16:03 Radim Krčmář
  2015-07-16 16:03 ` [Qemu-devel] [PATCH v3 1/3] crypto: fix build with nettle >= 3.0.0 Radim Krčmář
                   ` (2 more replies)
  0 siblings, 3 replies; 11+ messages in thread
From: Radim Krčmář @ 2015-07-16 16:03 UTC (permalink / raw)
  To: qemu-devel; +Cc: Peter Maydell

[1/3] fixes a build problem with new nettle libraries.
Peter Maydell found an undefined behavior with some architectures that
is fixed in [2/3].  (I haven't checked if QEMU runs on one.)
[3/3] refactors [2/3];  feel free to squish or drop it.

v3: make wrappers static [2-3/3]
v2: avoid undefined behavior[2-3/3]

Radim Krčmář (3):
  crypto: fix build with nettle >= 3.0.0
  crypto: avoid undefined behavior in nettle calls
  crypto: use CPP for wrapper definitions in nettle

 configure              |  4 +++-
 crypto/cipher-nettle.c | 38 ++++++++++++++++++++++++++++++++------
 2 files changed, 35 insertions(+), 7 deletions(-)

-- 
2.4.5

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

* [Qemu-devel] [PATCH v3 1/3] crypto: fix build with nettle >= 3.0.0
  2015-07-16 16:03 [Qemu-devel] [PATCH v3 0/3] crypto: nettle fixes Radim Krčmář
@ 2015-07-16 16:03 ` Radim Krčmář
  2015-07-17 10:54   ` Daniel P. Berrange
  2015-07-16 16:04 ` [Qemu-devel] [PATCH v3 2/3] crypto: avoid undefined behavior in nettle calls Radim Krčmář
  2015-07-16 16:04 ` [Qemu-devel] [PATCH v3 3/3] crypto: use CPP for wrapper definitions in nettle Radim Krčmář
  2 siblings, 1 reply; 11+ messages in thread
From: Radim Krčmář @ 2015-07-16 16:03 UTC (permalink / raw)
  To: qemu-devel; +Cc: Peter Maydell

In nettle 3, cbc_encrypt() accepts 'nettle_cipher_func' instead of
'nettle_crypt_func' and these two differ in 'const' qualifier of the
first argument.  The build fails with:

  In file included from crypto/cipher.c:71:0:
  ./crypto/cipher-nettle.c: In function ‘qcrypto_cipher_encrypt’:
  ./crypto/cipher-nettle.c:154:38: error: passing argument 2 of
  ‘nettle_cbc_encrypt’ from incompatible pointer type
           cbc_encrypt(ctx->ctx_encrypt, ctx->alg_encrypt,
                                               ^
  In file included from ./crypto/cipher-nettle.c:24:0,
                   from crypto/cipher.c:71:
  /usr/include/nettle/cbc.h:48:1: note: expected
  ‘void (*)(const void *, size_t, uint8_t *, const uint8_t *)
  but argument is of type
  ‘void (*)(      void *, size_t, uint8_t *, const uint8_t *)

To allow both versions, we switch to the new definition and #if typedef
it for old versions.

Signed-off-by: Radim Krčmář <rkrcmar@redhat.com>
---
 v3: no change
 v2: fix typo in crypto

 configure              |  4 +++-
 crypto/cipher-nettle.c | 16 ++++++++++------
 2 files changed, 13 insertions(+), 7 deletions(-)

diff --git a/configure b/configure
index 33b945530e64..cc0338ddbd14 100755
--- a/configure
+++ b/configure
@@ -2183,6 +2183,7 @@ if test "$gnutls_nettle" != "no"; then
     if $pkg_config --exists "nettle"; then
         nettle_cflags=`$pkg_config --cflags nettle`
         nettle_libs=`$pkg_config --libs nettle`
+        nettle_version=`$pkg_config --modversion nettle`
         libs_softmmu="$nettle_libs $libs_softmmu"
         libs_tools="$nettle_libs $libs_tools"
         QEMU_CFLAGS="$QEMU_CFLAGS $nettle_cflags"
@@ -4490,7 +4491,7 @@ echo "GTK support       $gtk"
 echo "GNUTLS support    $gnutls"
 echo "GNUTLS hash       $gnutls_hash"
 echo "GNUTLS gcrypt     $gnutls_gcrypt"
-echo "GNUTLS nettle     $gnutls_nettle"
+echo "GNUTLS nettle     $gnutls_nettle ${gnutls_nettle+($nettle_version)}"
 echo "VTE support       $vte"
 echo "curses support    $curses"
 echo "curl support      $curl"
@@ -4858,6 +4859,7 @@ if test "$gnutls_gcrypt" = "yes" ; then
 fi
 if test "$gnutls_nettle" = "yes" ; then
   echo "CONFIG_GNUTLS_NETTLE=y" >> $config_host_mak
+  echo "CONFIG_NETTLE_VERSION_MAJOR=${nettle_version%%.*}" >> $config_host_mak
 fi
 if test "$vte" = "yes" ; then
   echo "CONFIG_VTE=y" >> $config_host_mak
diff --git a/crypto/cipher-nettle.c b/crypto/cipher-nettle.c
index e5a14bc1393f..e61aaa29f049 100644
--- a/crypto/cipher-nettle.c
+++ b/crypto/cipher-nettle.c
@@ -23,12 +23,16 @@
 #include <nettle/des.h>
 #include <nettle/cbc.h>
 
+#if CONFIG_NETTLE_VERSION_MAJOR < 3
+typedef nettle_crypt_func nettle_cipher_func;
+#endif
+
 typedef struct QCryptoCipherNettle QCryptoCipherNettle;
 struct QCryptoCipherNettle {
     void *ctx_encrypt;
     void *ctx_decrypt;
-    nettle_crypt_func *alg_encrypt;
-    nettle_crypt_func *alg_decrypt;
+    nettle_cipher_func *alg_encrypt;
+    nettle_cipher_func *alg_decrypt;
     uint8_t *iv;
     size_t niv;
 };
@@ -83,8 +87,8 @@ QCryptoCipher *qcrypto_cipher_new(QCryptoCipherAlgorithm alg,
         des_set_key(ctx->ctx_encrypt, rfbkey);
         g_free(rfbkey);
 
-        ctx->alg_encrypt = (nettle_crypt_func *)des_encrypt;
-        ctx->alg_decrypt = (nettle_crypt_func *)des_decrypt;
+        ctx->alg_encrypt = (nettle_cipher_func *)des_encrypt;
+        ctx->alg_decrypt = (nettle_cipher_func *)des_decrypt;
 
         ctx->niv = DES_BLOCK_SIZE;
         break;
@@ -98,8 +102,8 @@ QCryptoCipher *qcrypto_cipher_new(QCryptoCipherAlgorithm alg,
         aes_set_encrypt_key(ctx->ctx_encrypt, nkey, key);
         aes_set_decrypt_key(ctx->ctx_decrypt, nkey, key);
 
-        ctx->alg_encrypt = (nettle_crypt_func *)aes_encrypt;
-        ctx->alg_decrypt = (nettle_crypt_func *)aes_decrypt;
+        ctx->alg_encrypt = (nettle_cipher_func *)aes_encrypt;
+        ctx->alg_decrypt = (nettle_cipher_func *)aes_decrypt;
 
         ctx->niv = AES_BLOCK_SIZE;
         break;
-- 
2.4.5

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

* [Qemu-devel] [PATCH v3 2/3] crypto: avoid undefined behavior in nettle calls
  2015-07-16 16:03 [Qemu-devel] [PATCH v3 0/3] crypto: nettle fixes Radim Krčmář
  2015-07-16 16:03 ` [Qemu-devel] [PATCH v3 1/3] crypto: fix build with nettle >= 3.0.0 Radim Krčmář
@ 2015-07-16 16:04 ` Radim Krčmář
  2015-07-17 10:55   ` Daniel P. Berrange
  2015-07-17 17:43   ` Kevin Wolf
  2015-07-16 16:04 ` [Qemu-devel] [PATCH v3 3/3] crypto: use CPP for wrapper definitions in nettle Radim Krčmář
  2 siblings, 2 replies; 11+ messages in thread
From: Radim Krčmář @ 2015-07-16 16:04 UTC (permalink / raw)
  To: qemu-devel; +Cc: Peter Maydell

Calling a function pointer that was cast from an incompatible function
results in undefined behavior.  'void *' isn't compatible with 'struct
XXX *', so we can't cast to nettle_cipher_func, but have to provide a
wrapper.  (Conversion from 'void *' to 'struct XXX *' might require
computation, which won't be done if we drop argument's true type and
pointers can have different sizes so passing arguments on stack would
bug.)

Having two different prototypes based on nettle version doesn't make
this solution any nicer.

Reported-by: Peter Maydell <peter.maydell@linaro.org>
Signed-off-by: Radim Krčmář <rkrcmar@redhat.com>
---
 v3: make wrappers 'static'
 v2: new [Peter]

 crypto/cipher-nettle.c | 43 +++++++++++++++++++++++++++++++++++++++----
 1 file changed, 39 insertions(+), 4 deletions(-)

diff --git a/crypto/cipher-nettle.c b/crypto/cipher-nettle.c
index e61aaa29f049..3e3c4c9cbf2f 100644
--- a/crypto/cipher-nettle.c
+++ b/crypto/cipher-nettle.c
@@ -25,8 +25,43 @@
 
 #if CONFIG_NETTLE_VERSION_MAJOR < 3
 typedef nettle_crypt_func nettle_cipher_func;
+
+typedef void *       cipher_ctx_t;
+typedef unsigned     cipher_length_t;
+#else
+typedef const void * cipher_ctx_t;
+typedef size_t       cipher_length_t;
 #endif
 
+static nettle_cipher_func aes_encrypt_wrapper;
+static nettle_cipher_func aes_decrypt_wrapper;
+static nettle_cipher_func des_encrypt_wrapper;
+static nettle_cipher_func des_decrypt_wrapper;
+
+static void aes_encrypt_wrapper(cipher_ctx_t ctx, cipher_length_t length,
+                         uint8_t *dst, const uint8_t *src)
+{
+    aes_encrypt(ctx, length, dst, src);
+}
+
+static void aes_decrypt_wrapper(cipher_ctx_t ctx, cipher_length_t length,
+                         uint8_t *dst, const uint8_t *src)
+{
+    aes_encrypt(ctx, length, dst, src);
+}
+
+static void des_encrypt_wrapper(cipher_ctx_t ctx, cipher_length_t length,
+                         uint8_t *dst, const uint8_t *src)
+{
+    des_encrypt(ctx, length, dst, src);
+}
+
+static void des_decrypt_wrapper(cipher_ctx_t ctx, cipher_length_t length,
+                         uint8_t *dst, const uint8_t *src)
+{
+    des_decrypt(ctx, length, dst, src);
+}
+
 typedef struct QCryptoCipherNettle QCryptoCipherNettle;
 struct QCryptoCipherNettle {
     void *ctx_encrypt;
@@ -87,8 +122,8 @@ QCryptoCipher *qcrypto_cipher_new(QCryptoCipherAlgorithm alg,
         des_set_key(ctx->ctx_encrypt, rfbkey);
         g_free(rfbkey);
 
-        ctx->alg_encrypt = (nettle_cipher_func *)des_encrypt;
-        ctx->alg_decrypt = (nettle_cipher_func *)des_decrypt;
+        ctx->alg_encrypt = des_encrypt_wrapper;
+        ctx->alg_decrypt = des_decrypt_wrapper;
 
         ctx->niv = DES_BLOCK_SIZE;
         break;
@@ -102,8 +137,8 @@ QCryptoCipher *qcrypto_cipher_new(QCryptoCipherAlgorithm alg,
         aes_set_encrypt_key(ctx->ctx_encrypt, nkey, key);
         aes_set_decrypt_key(ctx->ctx_decrypt, nkey, key);
 
-        ctx->alg_encrypt = (nettle_cipher_func *)aes_encrypt;
-        ctx->alg_decrypt = (nettle_cipher_func *)aes_decrypt;
+        ctx->alg_encrypt = aes_encrypt_wrapper;
+        ctx->alg_decrypt = aes_decrypt_wrapper;
 
         ctx->niv = AES_BLOCK_SIZE;
         break;
-- 
2.4.5

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

* [Qemu-devel] [PATCH v3 3/3] crypto: use CPP for wrapper definitions in nettle
  2015-07-16 16:03 [Qemu-devel] [PATCH v3 0/3] crypto: nettle fixes Radim Krčmář
  2015-07-16 16:03 ` [Qemu-devel] [PATCH v3 1/3] crypto: fix build with nettle >= 3.0.0 Radim Krčmář
  2015-07-16 16:04 ` [Qemu-devel] [PATCH v3 2/3] crypto: avoid undefined behavior in nettle calls Radim Krčmář
@ 2015-07-16 16:04 ` Radim Krčmář
  2015-07-16 16:11   ` Paolo Bonzini
  2015-07-17 10:58   ` Daniel P. Berrange
  2 siblings, 2 replies; 11+ messages in thread
From: Radim Krčmář @ 2015-07-16 16:04 UTC (permalink / raw)
  To: qemu-devel; +Cc: Peter Maydell

It's horrible both ways and I prefer this one.

Signed-off-by: Radim Krčmář <rkrcmar@redhat.com>
---
 v3: make wrappers 'static'
 v2: new

 crypto/cipher-nettle.c | 47 +++++++++++++++++------------------------------
 1 file changed, 17 insertions(+), 30 deletions(-)

diff --git a/crypto/cipher-nettle.c b/crypto/cipher-nettle.c
index 3e3c4c9cbf2f..5a9d506560ae 100644
--- a/crypto/cipher-nettle.c
+++ b/crypto/cipher-nettle.c
@@ -33,34 +33,21 @@ typedef const void * cipher_ctx_t;
 typedef size_t       cipher_length_t;
 #endif
 
-static nettle_cipher_func aes_encrypt_wrapper;
-static nettle_cipher_func aes_decrypt_wrapper;
-static nettle_cipher_func des_encrypt_wrapper;
-static nettle_cipher_func des_decrypt_wrapper;
+#define WRAP(cipher) \
+    static nettle_cipher_func cipher##_wrapper; \
+    static void cipher##_wrapper(cipher_ctx_t ctx, cipher_length_t length, \
+                          uint8_t *dst, const uint8_t *src) \
+    { \
+        cipher(ctx, length, dst, src); \
+    }
 
-static void aes_encrypt_wrapper(cipher_ctx_t ctx, cipher_length_t length,
-                         uint8_t *dst, const uint8_t *src)
-{
-    aes_encrypt(ctx, length, dst, src);
-}
+#define WRAPPED(cipher) \
+    cipher##_wrapper
 
-static void aes_decrypt_wrapper(cipher_ctx_t ctx, cipher_length_t length,
-                         uint8_t *dst, const uint8_t *src)
-{
-    aes_encrypt(ctx, length, dst, src);
-}
-
-static void des_encrypt_wrapper(cipher_ctx_t ctx, cipher_length_t length,
-                         uint8_t *dst, const uint8_t *src)
-{
-    des_encrypt(ctx, length, dst, src);
-}
-
-static void des_decrypt_wrapper(cipher_ctx_t ctx, cipher_length_t length,
-                         uint8_t *dst, const uint8_t *src)
-{
-    des_decrypt(ctx, length, dst, src);
-}
+WRAP(aes_encrypt)
+WRAP(aes_decrypt)
+WRAP(des_encrypt)
+WRAP(des_decrypt)
 
 typedef struct QCryptoCipherNettle QCryptoCipherNettle;
 struct QCryptoCipherNettle {
@@ -122,8 +109,8 @@ QCryptoCipher *qcrypto_cipher_new(QCryptoCipherAlgorithm alg,
         des_set_key(ctx->ctx_encrypt, rfbkey);
         g_free(rfbkey);
 
-        ctx->alg_encrypt = des_encrypt_wrapper;
-        ctx->alg_decrypt = des_decrypt_wrapper;
+        ctx->alg_encrypt = WRAPPED(des_encrypt);
+        ctx->alg_decrypt = WRAPPED(des_decrypt);
 
         ctx->niv = DES_BLOCK_SIZE;
         break;
@@ -137,8 +124,8 @@ QCryptoCipher *qcrypto_cipher_new(QCryptoCipherAlgorithm alg,
         aes_set_encrypt_key(ctx->ctx_encrypt, nkey, key);
         aes_set_decrypt_key(ctx->ctx_decrypt, nkey, key);
 
-        ctx->alg_encrypt = aes_encrypt_wrapper;
-        ctx->alg_decrypt = aes_decrypt_wrapper;
+        ctx->alg_encrypt = WRAPPED(aes_encrypt);
+        ctx->alg_decrypt = WRAPPED(aes_decrypt);
 
         ctx->niv = AES_BLOCK_SIZE;
         break;
-- 
2.4.5

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

* Re: [Qemu-devel] [PATCH v3 3/3] crypto: use CPP for wrapper definitions in nettle
  2015-07-16 16:04 ` [Qemu-devel] [PATCH v3 3/3] crypto: use CPP for wrapper definitions in nettle Radim Krčmář
@ 2015-07-16 16:11   ` Paolo Bonzini
  2015-07-17 11:00     ` Daniel P. Berrange
  2015-07-17 10:58   ` Daniel P. Berrange
  1 sibling, 1 reply; 11+ messages in thread
From: Paolo Bonzini @ 2015-07-16 16:11 UTC (permalink / raw)
  To: Radim Krčmář, qemu-devel; +Cc: Peter Maydell



On 16/07/2015 18:04, Radim Krčmář wrote:
> It's horrible both ways and I prefer this one.

Let's see what Dan thinks.  Since he's on vacation, for now I'm applying
patches 1 and 2 only.

Paolo

> Signed-off-by: Radim Krčmář <rkrcmar@redhat.com>
> ---
>  v3: make wrappers 'static'
>  v2: new
> 
>  crypto/cipher-nettle.c | 47 +++++++++++++++++------------------------------
>  1 file changed, 17 insertions(+), 30 deletions(-)

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

* Re: [Qemu-devel] [PATCH v3 1/3] crypto: fix build with nettle >= 3.0.0
  2015-07-16 16:03 ` [Qemu-devel] [PATCH v3 1/3] crypto: fix build with nettle >= 3.0.0 Radim Krčmář
@ 2015-07-17 10:54   ` Daniel P. Berrange
  0 siblings, 0 replies; 11+ messages in thread
From: Daniel P. Berrange @ 2015-07-17 10:54 UTC (permalink / raw)
  To: Radim Krčmář; +Cc: Peter Maydell, qemu-devel

On Thu, Jul 16, 2015 at 06:03:59PM +0200, Radim Krčmář wrote:
> In nettle 3, cbc_encrypt() accepts 'nettle_cipher_func' instead of
> 'nettle_crypt_func' and these two differ in 'const' qualifier of the
> first argument.  The build fails with:
> 
>   In file included from crypto/cipher.c:71:0:
>   ./crypto/cipher-nettle.c: In function ‘qcrypto_cipher_encrypt’:
>   ./crypto/cipher-nettle.c:154:38: error: passing argument 2 of
>   ‘nettle_cbc_encrypt’ from incompatible pointer type
>            cbc_encrypt(ctx->ctx_encrypt, ctx->alg_encrypt,
>                                                ^
>   In file included from ./crypto/cipher-nettle.c:24:0,
>                    from crypto/cipher.c:71:
>   /usr/include/nettle/cbc.h:48:1: note: expected
>   ‘void (*)(const void *, size_t, uint8_t *, const uint8_t *)
>   but argument is of type
>   ‘void (*)(      void *, size_t, uint8_t *, const uint8_t *)
> 
> To allow both versions, we switch to the new definition and #if typedef
> it for old versions.
> 
> Signed-off-by: Radim Krčmář <rkrcmar@redhat.com>
> ---
>  v3: no change
>  v2: fix typo in crypto
> 
>  configure              |  4 +++-
>  crypto/cipher-nettle.c | 16 ++++++++++------
>  2 files changed, 13 insertions(+), 7 deletions(-)

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

Regards,
Daniel
-- 
|: http://berrange.com      -o-    http://www.flickr.com/photos/dberrange/ :|
|: http://libvirt.org              -o-             http://virt-manager.org :|
|: http://autobuild.org       -o-         http://search.cpan.org/~danberr/ :|
|: http://entangle-photo.org       -o-       http://live.gnome.org/gtk-vnc :|

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

* Re: [Qemu-devel] [PATCH v3 2/3] crypto: avoid undefined behavior in nettle calls
  2015-07-16 16:04 ` [Qemu-devel] [PATCH v3 2/3] crypto: avoid undefined behavior in nettle calls Radim Krčmář
@ 2015-07-17 10:55   ` Daniel P. Berrange
  2015-07-17 17:43   ` Kevin Wolf
  1 sibling, 0 replies; 11+ messages in thread
From: Daniel P. Berrange @ 2015-07-17 10:55 UTC (permalink / raw)
  To: Radim Krčmář; +Cc: Peter Maydell, qemu-devel

On Thu, Jul 16, 2015 at 06:04:00PM +0200, Radim Krčmář wrote:
> Calling a function pointer that was cast from an incompatible function
> results in undefined behavior.  'void *' isn't compatible with 'struct
> XXX *', so we can't cast to nettle_cipher_func, but have to provide a
> wrapper.  (Conversion from 'void *' to 'struct XXX *' might require
> computation, which won't be done if we drop argument's true type and
> pointers can have different sizes so passing arguments on stack would
> bug.)
> 
> Having two different prototypes based on nettle version doesn't make
> this solution any nicer.
> 
> Reported-by: Peter Maydell <peter.maydell@linaro.org>
> Signed-off-by: Radim Krčmář <rkrcmar@redhat.com>
> ---
>  v3: make wrappers 'static'
>  v2: new [Peter]
> 
>  crypto/cipher-nettle.c | 43 +++++++++++++++++++++++++++++++++++++++----
>  1 file changed, 39 insertions(+), 4 deletions(-)

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

Regards,
Daniel
-- 
|: http://berrange.com      -o-    http://www.flickr.com/photos/dberrange/ :|
|: http://libvirt.org              -o-             http://virt-manager.org :|
|: http://autobuild.org       -o-         http://search.cpan.org/~danberr/ :|
|: http://entangle-photo.org       -o-       http://live.gnome.org/gtk-vnc :|

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

* Re: [Qemu-devel] [PATCH v3 3/3] crypto: use CPP for wrapper definitions in nettle
  2015-07-16 16:04 ` [Qemu-devel] [PATCH v3 3/3] crypto: use CPP for wrapper definitions in nettle Radim Krčmář
  2015-07-16 16:11   ` Paolo Bonzini
@ 2015-07-17 10:58   ` Daniel P. Berrange
  1 sibling, 0 replies; 11+ messages in thread
From: Daniel P. Berrange @ 2015-07-17 10:58 UTC (permalink / raw)
  To: Radim Krčmář; +Cc: Peter Maydell, qemu-devel

On Thu, Jul 16, 2015 at 06:04:01PM +0200, Radim Krčmář wrote:
> It's horrible both ways and I prefer this one.
> 
> Signed-off-by: Radim Krčmář <rkrcmar@redhat.com>

As you say its ugly either way, but I think the amount of duplicated
code pattern is not too bad, and IMHO it is slightly clearer to read
without the macro usage.

I won't object to merge if other people think this macro usage is worth
while, but I don't think there's compelling need for it.

> ---
>  v3: make wrappers 'static'
>  v2: new
> 
>  crypto/cipher-nettle.c | 47 +++++++++++++++++------------------------------
>  1 file changed, 17 insertions(+), 30 deletions(-)
> 
> diff --git a/crypto/cipher-nettle.c b/crypto/cipher-nettle.c
> index 3e3c4c9cbf2f..5a9d506560ae 100644
> --- a/crypto/cipher-nettle.c
> +++ b/crypto/cipher-nettle.c
> @@ -33,34 +33,21 @@ typedef const void * cipher_ctx_t;
>  typedef size_t       cipher_length_t;
>  #endif
>  
> -static nettle_cipher_func aes_encrypt_wrapper;
> -static nettle_cipher_func aes_decrypt_wrapper;
> -static nettle_cipher_func des_encrypt_wrapper;
> -static nettle_cipher_func des_decrypt_wrapper;
> +#define WRAP(cipher) \
> +    static nettle_cipher_func cipher##_wrapper; \
> +    static void cipher##_wrapper(cipher_ctx_t ctx, cipher_length_t length, \
> +                          uint8_t *dst, const uint8_t *src) \
> +    { \
> +        cipher(ctx, length, dst, src); \
> +    }
>  
> -static void aes_encrypt_wrapper(cipher_ctx_t ctx, cipher_length_t length,
> -                         uint8_t *dst, const uint8_t *src)
> -{
> -    aes_encrypt(ctx, length, dst, src);
> -}
> +#define WRAPPED(cipher) \
> +    cipher##_wrapper
>  
> -static void aes_decrypt_wrapper(cipher_ctx_t ctx, cipher_length_t length,
> -                         uint8_t *dst, const uint8_t *src)
> -{
> -    aes_encrypt(ctx, length, dst, src);
> -}
> -
> -static void des_encrypt_wrapper(cipher_ctx_t ctx, cipher_length_t length,
> -                         uint8_t *dst, const uint8_t *src)
> -{
> -    des_encrypt(ctx, length, dst, src);
> -}
> -
> -static void des_decrypt_wrapper(cipher_ctx_t ctx, cipher_length_t length,
> -                         uint8_t *dst, const uint8_t *src)
> -{
> -    des_decrypt(ctx, length, dst, src);
> -}
> +WRAP(aes_encrypt)
> +WRAP(aes_decrypt)
> +WRAP(des_encrypt)
> +WRAP(des_decrypt)
>  
>  typedef struct QCryptoCipherNettle QCryptoCipherNettle;
>  struct QCryptoCipherNettle {
> @@ -122,8 +109,8 @@ QCryptoCipher *qcrypto_cipher_new(QCryptoCipherAlgorithm alg,
>          des_set_key(ctx->ctx_encrypt, rfbkey);
>          g_free(rfbkey);
>  
> -        ctx->alg_encrypt = des_encrypt_wrapper;
> -        ctx->alg_decrypt = des_decrypt_wrapper;
> +        ctx->alg_encrypt = WRAPPED(des_encrypt);
> +        ctx->alg_decrypt = WRAPPED(des_decrypt);
>  
>          ctx->niv = DES_BLOCK_SIZE;
>          break;
> @@ -137,8 +124,8 @@ QCryptoCipher *qcrypto_cipher_new(QCryptoCipherAlgorithm alg,
>          aes_set_encrypt_key(ctx->ctx_encrypt, nkey, key);
>          aes_set_decrypt_key(ctx->ctx_decrypt, nkey, key);
>  
> -        ctx->alg_encrypt = aes_encrypt_wrapper;
> -        ctx->alg_decrypt = aes_decrypt_wrapper;
> +        ctx->alg_encrypt = WRAPPED(aes_encrypt);
> +        ctx->alg_decrypt = WRAPPED(aes_decrypt);
>  
>          ctx->niv = AES_BLOCK_SIZE;
>          break;

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

Regards,
Daniel
-- 
|: http://berrange.com      -o-    http://www.flickr.com/photos/dberrange/ :|
|: http://libvirt.org              -o-             http://virt-manager.org :|
|: http://autobuild.org       -o-         http://search.cpan.org/~danberr/ :|
|: http://entangle-photo.org       -o-       http://live.gnome.org/gtk-vnc :|

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

* Re: [Qemu-devel] [PATCH v3 3/3] crypto: use CPP for wrapper definitions in nettle
  2015-07-16 16:11   ` Paolo Bonzini
@ 2015-07-17 11:00     ` Daniel P. Berrange
  2015-07-17 11:00       ` Paolo Bonzini
  0 siblings, 1 reply; 11+ messages in thread
From: Daniel P. Berrange @ 2015-07-17 11:00 UTC (permalink / raw)
  To: Paolo Bonzini; +Cc: Peter Maydell, qemu-devel, Radim Krčmář

On Thu, Jul 16, 2015 at 06:11:12PM +0200, Paolo Bonzini wrote:
> 
> 
> On 16/07/2015 18:04, Radim Krčmář wrote:
> > It's horrible both ways and I prefer this one.
> 
> Let's see what Dan thinks.  Since he's on vacation, for now I'm applying
> patches 1 and 2 only.

I don't have a strong opinion on patch 3 - personally I don't think
it is much benefit, but if you / other maintainers like it, I'd be
fine with it being merged.

Regards,
Daniel
-- 
|: http://berrange.com      -o-    http://www.flickr.com/photos/dberrange/ :|
|: http://libvirt.org              -o-             http://virt-manager.org :|
|: http://autobuild.org       -o-         http://search.cpan.org/~danberr/ :|
|: http://entangle-photo.org       -o-       http://live.gnome.org/gtk-vnc :|

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

* Re: [Qemu-devel] [PATCH v3 3/3] crypto: use CPP for wrapper definitions in nettle
  2015-07-17 11:00     ` Daniel P. Berrange
@ 2015-07-17 11:00       ` Paolo Bonzini
  0 siblings, 0 replies; 11+ messages in thread
From: Paolo Bonzini @ 2015-07-17 11:00 UTC (permalink / raw)
  To: Daniel P. Berrange; +Cc: Peter Maydell, qemu-devel, Radim Krčmář



On 17/07/2015 13:00, Daniel P. Berrange wrote:
> > > On 16/07/2015 18:04, Radim Krčmář wrote:
> > > It's horrible both ways and I prefer this one.
> > 
> > Let's see what Dan thinks.  Since he's on vacation, for now I'm applying
> > patches 1 and 2 only.
>
> I don't have a strong opinion on patch 3 - personally I don't think
> it is much benefit, but if you / other maintainers like it, I'd be
> fine with it being merged.

I happen to agree with you and I was hoping that was the case. :P

Paolo

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

* Re: [Qemu-devel] [PATCH v3 2/3] crypto: avoid undefined behavior in nettle calls
  2015-07-16 16:04 ` [Qemu-devel] [PATCH v3 2/3] crypto: avoid undefined behavior in nettle calls Radim Krčmář
  2015-07-17 10:55   ` Daniel P. Berrange
@ 2015-07-17 17:43   ` Kevin Wolf
  1 sibling, 0 replies; 11+ messages in thread
From: Kevin Wolf @ 2015-07-17 17:43 UTC (permalink / raw)
  To: Radim Krčmář; +Cc: Peter Maydell, qemu-devel

Am 16.07.2015 um 18:04 hat Radim Krčmář geschrieben:
> Calling a function pointer that was cast from an incompatible function
> results in undefined behavior.  'void *' isn't compatible with 'struct
> XXX *', so we can't cast to nettle_cipher_func, but have to provide a
> wrapper.  (Conversion from 'void *' to 'struct XXX *' might require
> computation, which won't be done if we drop argument's true type and
> pointers can have different sizes so passing arguments on stack would
> bug.)
> 
> Having two different prototypes based on nettle version doesn't make
> this solution any nicer.
> 
> Reported-by: Peter Maydell <peter.maydell@linaro.org>
> Signed-off-by: Radim Krčmář <rkrcmar@redhat.com>

git bisect says that it's this commit which broke qemu-iotests 134.

> +static void aes_decrypt_wrapper(cipher_ctx_t ctx, cipher_length_t length,
> +                         uint8_t *dst, const uint8_t *src)
> +{
> +    aes_encrypt(ctx, length, dst, src);
> +}

And this is why (decrypt -> encrypt). I'll send a fix.

Kevin

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

end of thread, other threads:[~2015-07-17 17:43 UTC | newest]

Thread overview: 11+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2015-07-16 16:03 [Qemu-devel] [PATCH v3 0/3] crypto: nettle fixes Radim Krčmář
2015-07-16 16:03 ` [Qemu-devel] [PATCH v3 1/3] crypto: fix build with nettle >= 3.0.0 Radim Krčmář
2015-07-17 10:54   ` Daniel P. Berrange
2015-07-16 16:04 ` [Qemu-devel] [PATCH v3 2/3] crypto: avoid undefined behavior in nettle calls Radim Krčmář
2015-07-17 10:55   ` Daniel P. Berrange
2015-07-17 17:43   ` Kevin Wolf
2015-07-16 16:04 ` [Qemu-devel] [PATCH v3 3/3] crypto: use CPP for wrapper definitions in nettle Radim Krčmář
2015-07-16 16:11   ` Paolo Bonzini
2015-07-17 11:00     ` Daniel P. Berrange
2015-07-17 11:00       ` Paolo Bonzini
2015-07-17 10:58   ` Daniel P. Berrange

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.