qemu-devel.nongnu.org archive mirror
 help / color / mirror / Atom feed
* [Qemu-devel] [PATCH 0/5] crypto threads
@ 2018-12-04 18:46 Vladimir Sementsov-Ogievskiy
  2018-12-04 18:46 ` [Qemu-devel] [PATCH for-3.1? 1/5] crypto/block-luks: fix memory leak in qcrypto_block_luks_create Vladimir Sementsov-Ogievskiy
                   ` (5 more replies)
  0 siblings, 6 replies; 8+ messages in thread
From: Vladimir Sementsov-Ogievskiy @ 2018-12-04 18:46 UTC (permalink / raw)
  To: qemu-devel, qemu-block; +Cc: berrange, mreitz, kwolf, den, vsementsov

Hi all.

These series are preliminary step before moving encryption code in
qcow2 to the threads. The first attempt of doing it is on list
([PATCH 00/11] qcow2: encryption threads : 
 https://lists.gnu.org/archive/html/qemu-block/2018-11/msg00729.html)
But it's approach with multiplying the whole QCryptoBlock per thread
was rejected. So, here is a solution to maintain multitasking inside
QCryptoBlock.

Patch 01 may be considered as 3.1-rc4 material, others are ofcourse
for 4.0.

Vladimir Sementsov-Ogievskiy (5):
  crypto/block-luks: fix memory leak in qcrypto_block_luks_create
  crypto/block: refactor qcrypto_block_*crypt_helper functions
  crypto/block: rename qcrypto_block_*crypt_helper
  crypto/block: introduce qcrypto_block_*crypt_helper functions
  crypto: support multiple threads accessing one QCryptoBlock

 crypto/blockpriv.h        |  42 ++++++--
 include/crypto/block.h    |  16 ++-
 block/crypto.c            |   1 +
 block/qcow.c              |   2 +-
 block/qcow2.c             |   4 +-
 crypto/block-luks.c       |  60 +++++------
 crypto/block-qcow.c       |  26 ++---
 crypto/block.c            | 210 ++++++++++++++++++++++++++++----------
 tests/test-crypto-block.c |   2 +
 9 files changed, 257 insertions(+), 106 deletions(-)

-- 
2.18.0

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

* [Qemu-devel] [PATCH for-3.1? 1/5] crypto/block-luks: fix memory leak in qcrypto_block_luks_create
  2018-12-04 18:46 [Qemu-devel] [PATCH 0/5] crypto threads Vladimir Sementsov-Ogievskiy
@ 2018-12-04 18:46 ` Vladimir Sementsov-Ogievskiy
  2018-12-04 18:53   ` Peter Maydell
  2018-12-04 18:46 ` [Qemu-devel] [PATCH 2/5] crypto/block: refactor qcrypto_block_*crypt_helper functions Vladimir Sementsov-Ogievskiy
                   ` (4 subsequent siblings)
  5 siblings, 1 reply; 8+ messages in thread
From: Vladimir Sementsov-Ogievskiy @ 2018-12-04 18:46 UTC (permalink / raw)
  To: qemu-devel, qemu-block; +Cc: berrange, mreitz, kwolf, den, vsementsov

Free block->cipher and block->ivgen on error path.

Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
---
 crypto/block-luks.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/crypto/block-luks.c b/crypto/block-luks.c
index 5738124773..51e24d23ca 100644
--- a/crypto/block-luks.c
+++ b/crypto/block-luks.c
@@ -1341,6 +1341,9 @@ qcrypto_block_luks_create(QCryptoBlock *block,
     qcrypto_ivgen_free(ivgen);
     qcrypto_cipher_free(cipher);
 
+    qcrypto_cipher_free(block->cipher);
+    qcrypto_ivgen_free(block->ivgen);
+
     g_free(luks);
     return -1;
 }
-- 
2.18.0

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

* [Qemu-devel] [PATCH 2/5] crypto/block: refactor qcrypto_block_*crypt_helper functions
  2018-12-04 18:46 [Qemu-devel] [PATCH 0/5] crypto threads Vladimir Sementsov-Ogievskiy
  2018-12-04 18:46 ` [Qemu-devel] [PATCH for-3.1? 1/5] crypto/block-luks: fix memory leak in qcrypto_block_luks_create Vladimir Sementsov-Ogievskiy
@ 2018-12-04 18:46 ` Vladimir Sementsov-Ogievskiy
  2018-12-04 18:46 ` [Qemu-devel] [PATCH 3/5] crypto/block: rename qcrypto_block_*crypt_helper Vladimir Sementsov-Ogievskiy
                   ` (3 subsequent siblings)
  5 siblings, 0 replies; 8+ messages in thread
From: Vladimir Sementsov-Ogievskiy @ 2018-12-04 18:46 UTC (permalink / raw)
  To: qemu-devel, qemu-block; +Cc: berrange, mreitz, kwolf, den, vsementsov

qcrypto_block_encrypt_helper and qcrypto_block_decrypt_helper are
almost identical, let's reduce code duplication and simplify further
improvements.

Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
---
 crypto/block.c | 81 +++++++++++++++++++-------------------------------
 1 file changed, 31 insertions(+), 50 deletions(-)

diff --git a/crypto/block.c b/crypto/block.c
index e59d1140fe..f4101f0841 100644
--- a/crypto/block.c
+++ b/crypto/block.c
@@ -190,14 +190,21 @@ void qcrypto_block_free(QCryptoBlock *block)
 }
 
 
-int qcrypto_block_decrypt_helper(QCryptoCipher *cipher,
-                                 size_t niv,
-                                 QCryptoIVGen *ivgen,
-                                 int sectorsize,
-                                 uint64_t offset,
-                                 uint8_t *buf,
-                                 size_t len,
-                                 Error **errp)
+typedef int (*QCryptoCipherEncryptFunc)(QCryptoCipher *cipher,
+                                        const void *in,
+                                        void *out,
+                                        size_t len,
+                                        Error **errp);
+
+static int do_qcrypto_block_encrypt(QCryptoCipher *cipher,
+                                    size_t niv,
+                                    QCryptoIVGen *ivgen,
+                                    int sectorsize,
+                                    uint64_t offset,
+                                    uint8_t *buf,
+                                    size_t len,
+                                    QCryptoCipherEncryptFunc func,
+                                    Error **errp)
 {
     uint8_t *iv;
     int ret = -1;
@@ -226,8 +233,7 @@ int qcrypto_block_decrypt_helper(QCryptoCipher *cipher,
         }
 
         nbytes = len > sectorsize ? sectorsize : len;
-        if (qcrypto_cipher_decrypt(cipher, buf, buf,
-                                   nbytes, errp) < 0) {
+        if (func(cipher, buf, buf, nbytes, errp) < 0) {
             goto cleanup;
         }
 
@@ -243,7 +249,7 @@ int qcrypto_block_decrypt_helper(QCryptoCipher *cipher,
 }
 
 
-int qcrypto_block_encrypt_helper(QCryptoCipher *cipher,
+int qcrypto_block_decrypt_helper(QCryptoCipher *cipher,
                                  size_t niv,
                                  QCryptoIVGen *ivgen,
                                  int sectorsize,
@@ -252,45 +258,20 @@ int qcrypto_block_encrypt_helper(QCryptoCipher *cipher,
                                  size_t len,
                                  Error **errp)
 {
-    uint8_t *iv;
-    int ret = -1;
-    uint64_t startsector = offset / sectorsize;
-
-    assert(QEMU_IS_ALIGNED(offset, sectorsize));
-    assert(QEMU_IS_ALIGNED(len, sectorsize));
-
-    iv = niv ? g_new0(uint8_t, niv) : NULL;
-
-    while (len > 0) {
-        size_t nbytes;
-        if (niv) {
-            if (qcrypto_ivgen_calculate(ivgen,
-                                        startsector,
-                                        iv, niv,
-                                        errp) < 0) {
-                goto cleanup;
-            }
-
-            if (qcrypto_cipher_setiv(cipher,
-                                     iv, niv,
-                                     errp) < 0) {
-                goto cleanup;
-            }
-        }
-
-        nbytes = len > sectorsize ? sectorsize : len;
-        if (qcrypto_cipher_encrypt(cipher, buf, buf,
-                                   nbytes, errp) < 0) {
-            goto cleanup;
-        }
+    return do_qcrypto_block_encrypt(cipher, niv, ivgen, sectorsize, offset,
+                                    buf, len, qcrypto_cipher_decrypt, errp);
+}
 
-        startsector++;
-        buf += nbytes;
-        len -= nbytes;
-    }
 
-    ret = 0;
- cleanup:
-    g_free(iv);
-    return ret;
+int qcrypto_block_encrypt_helper(QCryptoCipher *cipher,
+                                 size_t niv,
+                                 QCryptoIVGen *ivgen,
+                                 int sectorsize,
+                                 uint64_t offset,
+                                 uint8_t *buf,
+                                 size_t len,
+                                 Error **errp)
+{
+    return do_qcrypto_block_encrypt(cipher, niv, ivgen, sectorsize, offset,
+                                    buf, len, qcrypto_cipher_encrypt, errp);
 }
-- 
2.18.0

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

* [Qemu-devel] [PATCH 3/5] crypto/block: rename qcrypto_block_*crypt_helper
  2018-12-04 18:46 [Qemu-devel] [PATCH 0/5] crypto threads Vladimir Sementsov-Ogievskiy
  2018-12-04 18:46 ` [Qemu-devel] [PATCH for-3.1? 1/5] crypto/block-luks: fix memory leak in qcrypto_block_luks_create Vladimir Sementsov-Ogievskiy
  2018-12-04 18:46 ` [Qemu-devel] [PATCH 2/5] crypto/block: refactor qcrypto_block_*crypt_helper functions Vladimir Sementsov-Ogievskiy
@ 2018-12-04 18:46 ` Vladimir Sementsov-Ogievskiy
  2018-12-04 18:46 ` [Qemu-devel] [PATCH 4/5] crypto/block: introduce qcrypto_block_*crypt_helper functions Vladimir Sementsov-Ogievskiy
                   ` (2 subsequent siblings)
  5 siblings, 0 replies; 8+ messages in thread
From: Vladimir Sementsov-Ogievskiy @ 2018-12-04 18:46 UTC (permalink / raw)
  To: qemu-devel, qemu-block; +Cc: berrange, mreitz, kwolf, den, vsementsov

Rename qcrypto_block_*crypt_helper to qcrypto_cipher_*crypt_helper, as
it's not about QCryptoBlock. This is needed to introduce
qcrypto_block_*crypt_helper in the next commit, which will have
QCryptoBlock pointer and than will be able to use additional fields of
it, which in turn will be used to implement thread-safe QCryptoBlock
operations.

Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
---
 crypto/blockpriv.h  | 34 +++++++++++++-------------
 crypto/block-luks.c | 44 +++++++++++++++++-----------------
 crypto/block-qcow.c | 16 ++++++-------
 crypto/block.c      | 58 ++++++++++++++++++++++-----------------------
 4 files changed, 76 insertions(+), 76 deletions(-)

diff --git a/crypto/blockpriv.h b/crypto/blockpriv.h
index 41840abcec..347a7c010a 100644
--- a/crypto/blockpriv.h
+++ b/crypto/blockpriv.h
@@ -78,22 +78,22 @@ struct QCryptoBlockDriver {
 };
 
 
-int qcrypto_block_decrypt_helper(QCryptoCipher *cipher,
-                                 size_t niv,
-                                 QCryptoIVGen *ivgen,
-                                 int sectorsize,
-                                 uint64_t offset,
-                                 uint8_t *buf,
-                                 size_t len,
-                                 Error **errp);
-
-int qcrypto_block_encrypt_helper(QCryptoCipher *cipher,
-                                 size_t niv,
-                                 QCryptoIVGen *ivgen,
-                                 int sectorsize,
-                                 uint64_t offset,
-                                 uint8_t *buf,
-                                 size_t len,
-                                 Error **errp);
+int qcrypto_cipher_decrypt_helper(QCryptoCipher *cipher,
+                                  size_t niv,
+                                  QCryptoIVGen *ivgen,
+                                  int sectorsize,
+                                  uint64_t offset,
+                                  uint8_t *buf,
+                                  size_t len,
+                                  Error **errp);
+
+int qcrypto_cipher_encrypt_helper(QCryptoCipher *cipher,
+                                  size_t niv,
+                                  QCryptoIVGen *ivgen,
+                                  int sectorsize,
+                                  uint64_t offset,
+                                  uint8_t *buf,
+                                  size_t len,
+                                  Error **errp);
 
 #endif /* QCRYPTO_BLOCKPRIV_H */
diff --git a/crypto/block-luks.c b/crypto/block-luks.c
index 51e24d23ca..72dd51051d 100644
--- a/crypto/block-luks.c
+++ b/crypto/block-luks.c
@@ -504,14 +504,14 @@ qcrypto_block_luks_load_key(QCryptoBlock *block,
      * to reset the encryption cipher every time the master
      * key crosses a sector boundary.
      */
-    if (qcrypto_block_decrypt_helper(cipher,
-                                     niv,
-                                     ivgen,
-                                     QCRYPTO_BLOCK_LUKS_SECTOR_SIZE,
-                                     0,
-                                     splitkey,
-                                     splitkeylen,
-                                     errp) < 0) {
+    if (qcrypto_cipher_decrypt_helper(cipher,
+                                      niv,
+                                      ivgen,
+                                      QCRYPTO_BLOCK_LUKS_SECTOR_SIZE,
+                                      0,
+                                      splitkey,
+                                      splitkeylen,
+                                      errp) < 0) {
         goto cleanup;
     }
 
@@ -1219,12 +1219,12 @@ qcrypto_block_luks_create(QCryptoBlock *block,
 
     /* Now we encrypt the split master key with the key generated
      * from the user's password, before storing it */
-    if (qcrypto_block_encrypt_helper(cipher, block->niv, ivgen,
-                                     QCRYPTO_BLOCK_LUKS_SECTOR_SIZE,
-                                     0,
-                                     splitkey,
-                                     splitkeylen,
-                                     errp) < 0) {
+    if (qcrypto_cipher_encrypt_helper(cipher, block->niv, ivgen,
+                                      QCRYPTO_BLOCK_LUKS_SECTOR_SIZE,
+                                      0,
+                                      splitkey,
+                                      splitkeylen,
+                                      errp) < 0) {
         goto error;
     }
 
@@ -1409,10 +1409,10 @@ qcrypto_block_luks_decrypt(QCryptoBlock *block,
 {
     assert(QEMU_IS_ALIGNED(offset, QCRYPTO_BLOCK_LUKS_SECTOR_SIZE));
     assert(QEMU_IS_ALIGNED(len, QCRYPTO_BLOCK_LUKS_SECTOR_SIZE));
-    return qcrypto_block_decrypt_helper(block->cipher,
-                                        block->niv, block->ivgen,
-                                        QCRYPTO_BLOCK_LUKS_SECTOR_SIZE,
-                                        offset, buf, len, errp);
+    return qcrypto_cipher_decrypt_helper(block->cipher,
+                                         block->niv, block->ivgen,
+                                         QCRYPTO_BLOCK_LUKS_SECTOR_SIZE,
+                                         offset, buf, len, errp);
 }
 
 
@@ -1425,10 +1425,10 @@ qcrypto_block_luks_encrypt(QCryptoBlock *block,
 {
     assert(QEMU_IS_ALIGNED(offset, QCRYPTO_BLOCK_LUKS_SECTOR_SIZE));
     assert(QEMU_IS_ALIGNED(len, QCRYPTO_BLOCK_LUKS_SECTOR_SIZE));
-    return qcrypto_block_encrypt_helper(block->cipher,
-                                        block->niv, block->ivgen,
-                                        QCRYPTO_BLOCK_LUKS_SECTOR_SIZE,
-                                        offset, buf, len, errp);
+    return qcrypto_cipher_encrypt_helper(block->cipher,
+                                         block->niv, block->ivgen,
+                                         QCRYPTO_BLOCK_LUKS_SECTOR_SIZE,
+                                         offset, buf, len, errp);
 }
 
 
diff --git a/crypto/block-qcow.c b/crypto/block-qcow.c
index 7606231e79..536ef4ee98 100644
--- a/crypto/block-qcow.c
+++ b/crypto/block-qcow.c
@@ -152,10 +152,10 @@ qcrypto_block_qcow_decrypt(QCryptoBlock *block,
 {
     assert(QEMU_IS_ALIGNED(offset, QCRYPTO_BLOCK_QCOW_SECTOR_SIZE));
     assert(QEMU_IS_ALIGNED(len, QCRYPTO_BLOCK_QCOW_SECTOR_SIZE));
-    return qcrypto_block_decrypt_helper(block->cipher,
-                                        block->niv, block->ivgen,
-                                        QCRYPTO_BLOCK_QCOW_SECTOR_SIZE,
-                                        offset, buf, len, errp);
+    return qcrypto_cipher_decrypt_helper(block->cipher,
+                                         block->niv, block->ivgen,
+                                         QCRYPTO_BLOCK_QCOW_SECTOR_SIZE,
+                                         offset, buf, len, errp);
 }
 
 
@@ -168,10 +168,10 @@ qcrypto_block_qcow_encrypt(QCryptoBlock *block,
 {
     assert(QEMU_IS_ALIGNED(offset, QCRYPTO_BLOCK_QCOW_SECTOR_SIZE));
     assert(QEMU_IS_ALIGNED(len, QCRYPTO_BLOCK_QCOW_SECTOR_SIZE));
-    return qcrypto_block_encrypt_helper(block->cipher,
-                                        block->niv, block->ivgen,
-                                        QCRYPTO_BLOCK_QCOW_SECTOR_SIZE,
-                                        offset, buf, len, errp);
+    return qcrypto_cipher_encrypt_helper(block->cipher,
+                                         block->niv, block->ivgen,
+                                         QCRYPTO_BLOCK_QCOW_SECTOR_SIZE,
+                                         offset, buf, len, errp);
 }
 
 
diff --git a/crypto/block.c b/crypto/block.c
index f4101f0841..540b27e581 100644
--- a/crypto/block.c
+++ b/crypto/block.c
@@ -196,15 +196,15 @@ typedef int (*QCryptoCipherEncryptFunc)(QCryptoCipher *cipher,
                                         size_t len,
                                         Error **errp);
 
-static int do_qcrypto_block_encrypt(QCryptoCipher *cipher,
-                                    size_t niv,
-                                    QCryptoIVGen *ivgen,
-                                    int sectorsize,
-                                    uint64_t offset,
-                                    uint8_t *buf,
-                                    size_t len,
-                                    QCryptoCipherEncryptFunc func,
-                                    Error **errp)
+static int do_qcrypto_cipher_encrypt(QCryptoCipher *cipher,
+                                     size_t niv,
+                                     QCryptoIVGen *ivgen,
+                                     int sectorsize,
+                                     uint64_t offset,
+                                     uint8_t *buf,
+                                     size_t len,
+                                     QCryptoCipherEncryptFunc func,
+                                     Error **errp)
 {
     uint8_t *iv;
     int ret = -1;
@@ -249,29 +249,29 @@ static int do_qcrypto_block_encrypt(QCryptoCipher *cipher,
 }
 
 
-int qcrypto_block_decrypt_helper(QCryptoCipher *cipher,
-                                 size_t niv,
-                                 QCryptoIVGen *ivgen,
-                                 int sectorsize,
-                                 uint64_t offset,
-                                 uint8_t *buf,
-                                 size_t len,
-                                 Error **errp)
+int qcrypto_cipher_decrypt_helper(QCryptoCipher *cipher,
+                                  size_t niv,
+                                  QCryptoIVGen *ivgen,
+                                  int sectorsize,
+                                  uint64_t offset,
+                                  uint8_t *buf,
+                                  size_t len,
+                                  Error **errp)
 {
-    return do_qcrypto_block_encrypt(cipher, niv, ivgen, sectorsize, offset,
-                                    buf, len, qcrypto_cipher_decrypt, errp);
+    return do_qcrypto_cipher_encrypt(cipher, niv, ivgen, sectorsize, offset,
+                                     buf, len, qcrypto_cipher_decrypt, errp);
 }
 
 
-int qcrypto_block_encrypt_helper(QCryptoCipher *cipher,
-                                 size_t niv,
-                                 QCryptoIVGen *ivgen,
-                                 int sectorsize,
-                                 uint64_t offset,
-                                 uint8_t *buf,
-                                 size_t len,
-                                 Error **errp)
+int qcrypto_cipher_encrypt_helper(QCryptoCipher *cipher,
+                                  size_t niv,
+                                  QCryptoIVGen *ivgen,
+                                  int sectorsize,
+                                  uint64_t offset,
+                                  uint8_t *buf,
+                                  size_t len,
+                                  Error **errp)
 {
-    return do_qcrypto_block_encrypt(cipher, niv, ivgen, sectorsize, offset,
-                                    buf, len, qcrypto_cipher_encrypt, errp);
+    return do_qcrypto_cipher_encrypt(cipher, niv, ivgen, sectorsize, offset,
+                                     buf, len, qcrypto_cipher_encrypt, errp);
 }
-- 
2.18.0

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

* [Qemu-devel] [PATCH 4/5] crypto/block: introduce qcrypto_block_*crypt_helper functions
  2018-12-04 18:46 [Qemu-devel] [PATCH 0/5] crypto threads Vladimir Sementsov-Ogievskiy
                   ` (2 preceding siblings ...)
  2018-12-04 18:46 ` [Qemu-devel] [PATCH 3/5] crypto/block: rename qcrypto_block_*crypt_helper Vladimir Sementsov-Ogievskiy
@ 2018-12-04 18:46 ` Vladimir Sementsov-Ogievskiy
  2018-12-04 18:46 ` [Qemu-devel] [PATCH 5/5] crypto: support multiple threads accessing one QCryptoBlock Vladimir Sementsov-Ogievskiy
  2018-12-04 23:23 ` [Qemu-devel] [PATCH 0/5] crypto threads no-reply
  5 siblings, 0 replies; 8+ messages in thread
From: Vladimir Sementsov-Ogievskiy @ 2018-12-04 18:46 UTC (permalink / raw)
  To: qemu-devel, qemu-block; +Cc: berrange, mreitz, kwolf, den, vsementsov

Introduce QCryptoBlock-based functions and use them where possible.
This is needed to implement thread-safe encrypt/decrypt operations.

Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
---
 crypto/blockpriv.h  | 14 ++++++++++++++
 crypto/block-luks.c | 14 ++++++--------
 crypto/block-qcow.c | 14 ++++++--------
 crypto/block.c      | 26 ++++++++++++++++++++++++++
 4 files changed, 52 insertions(+), 16 deletions(-)

diff --git a/crypto/blockpriv.h b/crypto/blockpriv.h
index 347a7c010a..e9fe3e5687 100644
--- a/crypto/blockpriv.h
+++ b/crypto/blockpriv.h
@@ -96,4 +96,18 @@ int qcrypto_cipher_encrypt_helper(QCryptoCipher *cipher,
                                   size_t len,
                                   Error **errp);
 
+int qcrypto_block_decrypt_helper(QCryptoBlock *block,
+                                 int sectorsize,
+                                 uint64_t offset,
+                                 uint8_t *buf,
+                                 size_t len,
+                                 Error **errp);
+
+int qcrypto_block_encrypt_helper(QCryptoBlock *block,
+                                 int sectorsize,
+                                 uint64_t offset,
+                                 uint8_t *buf,
+                                 size_t len,
+                                 Error **errp);
+
 #endif /* QCRYPTO_BLOCKPRIV_H */
diff --git a/crypto/block-luks.c b/crypto/block-luks.c
index 72dd51051d..bbffd80fff 100644
--- a/crypto/block-luks.c
+++ b/crypto/block-luks.c
@@ -1409,10 +1409,9 @@ qcrypto_block_luks_decrypt(QCryptoBlock *block,
 {
     assert(QEMU_IS_ALIGNED(offset, QCRYPTO_BLOCK_LUKS_SECTOR_SIZE));
     assert(QEMU_IS_ALIGNED(len, QCRYPTO_BLOCK_LUKS_SECTOR_SIZE));
-    return qcrypto_cipher_decrypt_helper(block->cipher,
-                                         block->niv, block->ivgen,
-                                         QCRYPTO_BLOCK_LUKS_SECTOR_SIZE,
-                                         offset, buf, len, errp);
+    return qcrypto_block_decrypt_helper(block,
+                                        QCRYPTO_BLOCK_LUKS_SECTOR_SIZE,
+                                        offset, buf, len, errp);
 }
 
 
@@ -1425,10 +1424,9 @@ qcrypto_block_luks_encrypt(QCryptoBlock *block,
 {
     assert(QEMU_IS_ALIGNED(offset, QCRYPTO_BLOCK_LUKS_SECTOR_SIZE));
     assert(QEMU_IS_ALIGNED(len, QCRYPTO_BLOCK_LUKS_SECTOR_SIZE));
-    return qcrypto_cipher_encrypt_helper(block->cipher,
-                                         block->niv, block->ivgen,
-                                         QCRYPTO_BLOCK_LUKS_SECTOR_SIZE,
-                                         offset, buf, len, errp);
+    return qcrypto_block_encrypt_helper(block,
+                                        QCRYPTO_BLOCK_LUKS_SECTOR_SIZE,
+                                        offset, buf, len, errp);
 }
 
 
diff --git a/crypto/block-qcow.c b/crypto/block-qcow.c
index 536ef4ee98..36bf5f09b7 100644
--- a/crypto/block-qcow.c
+++ b/crypto/block-qcow.c
@@ -152,10 +152,9 @@ qcrypto_block_qcow_decrypt(QCryptoBlock *block,
 {
     assert(QEMU_IS_ALIGNED(offset, QCRYPTO_BLOCK_QCOW_SECTOR_SIZE));
     assert(QEMU_IS_ALIGNED(len, QCRYPTO_BLOCK_QCOW_SECTOR_SIZE));
-    return qcrypto_cipher_decrypt_helper(block->cipher,
-                                         block->niv, block->ivgen,
-                                         QCRYPTO_BLOCK_QCOW_SECTOR_SIZE,
-                                         offset, buf, len, errp);
+    return qcrypto_block_decrypt_helper(block,
+                                        QCRYPTO_BLOCK_QCOW_SECTOR_SIZE,
+                                        offset, buf, len, errp);
 }
 
 
@@ -168,10 +167,9 @@ qcrypto_block_qcow_encrypt(QCryptoBlock *block,
 {
     assert(QEMU_IS_ALIGNED(offset, QCRYPTO_BLOCK_QCOW_SECTOR_SIZE));
     assert(QEMU_IS_ALIGNED(len, QCRYPTO_BLOCK_QCOW_SECTOR_SIZE));
-    return qcrypto_cipher_encrypt_helper(block->cipher,
-                                         block->niv, block->ivgen,
-                                         QCRYPTO_BLOCK_QCOW_SECTOR_SIZE,
-                                         offset, buf, len, errp);
+    return qcrypto_block_encrypt_helper(block,
+                                        QCRYPTO_BLOCK_QCOW_SECTOR_SIZE,
+                                        offset, buf, len, errp);
 }
 
 
diff --git a/crypto/block.c b/crypto/block.c
index 540b27e581..3edd9ec251 100644
--- a/crypto/block.c
+++ b/crypto/block.c
@@ -275,3 +275,29 @@ int qcrypto_cipher_encrypt_helper(QCryptoCipher *cipher,
     return do_qcrypto_cipher_encrypt(cipher, niv, ivgen, sectorsize, offset,
                                      buf, len, qcrypto_cipher_encrypt, errp);
 }
+
+
+int qcrypto_block_decrypt_helper(QCryptoBlock *block,
+                                 int sectorsize,
+                                 uint64_t offset,
+                                 uint8_t *buf,
+                                 size_t len,
+                                 Error **errp)
+{
+    return do_qcrypto_cipher_encrypt(block->cipher, block->niv, block->ivgen,
+                                     sectorsize, offset, buf, len,
+                                     qcrypto_cipher_decrypt, errp);
+}
+
+
+int qcrypto_block_encrypt_helper(QCryptoBlock *block,
+                                 int sectorsize,
+                                 uint64_t offset,
+                                 uint8_t *buf,
+                                 size_t len,
+                                 Error **errp)
+{
+    return do_qcrypto_cipher_encrypt(block->cipher, block->niv, block->ivgen,
+                                     sectorsize, offset, buf, len,
+                                     qcrypto_cipher_encrypt, errp);
+}
-- 
2.18.0

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

* [Qemu-devel] [PATCH 5/5] crypto: support multiple threads accessing one QCryptoBlock
  2018-12-04 18:46 [Qemu-devel] [PATCH 0/5] crypto threads Vladimir Sementsov-Ogievskiy
                   ` (3 preceding siblings ...)
  2018-12-04 18:46 ` [Qemu-devel] [PATCH 4/5] crypto/block: introduce qcrypto_block_*crypt_helper functions Vladimir Sementsov-Ogievskiy
@ 2018-12-04 18:46 ` Vladimir Sementsov-Ogievskiy
  2018-12-04 23:23 ` [Qemu-devel] [PATCH 0/5] crypto threads no-reply
  5 siblings, 0 replies; 8+ messages in thread
From: Vladimir Sementsov-Ogievskiy @ 2018-12-04 18:46 UTC (permalink / raw)
  To: qemu-devel, qemu-block; +Cc: berrange, mreitz, kwolf, den, vsementsov

The two thing that should be handled are cipher and ivgen. For ivgen
the solution is just mutex, as iv calculations should not be long in
comparison with encryption/decryption. And for cipher let's just keep
per-thread ciphers.

Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
---
 crypto/blockpriv.h        |  16 ++++-
 include/crypto/block.h    |  16 ++++-
 block/crypto.c            |   1 +
 block/qcow.c              |   2 +-
 block/qcow2.c             |   4 +-
 crypto/block-luks.c       |  25 +++----
 crypto/block-qcow.c       |  20 +++---
 crypto/block.c            | 135 ++++++++++++++++++++++++++++++++------
 tests/test-crypto-block.c |   2 +
 9 files changed, 176 insertions(+), 45 deletions(-)

diff --git a/crypto/blockpriv.h b/crypto/blockpriv.h
index e9fe3e5687..86dae49210 100644
--- a/crypto/blockpriv.h
+++ b/crypto/blockpriv.h
@@ -22,6 +22,7 @@
 #define QCRYPTO_BLOCKPRIV_H
 
 #include "crypto/block.h"
+#include "qemu/thread.h"
 
 typedef struct QCryptoBlockDriver QCryptoBlockDriver;
 
@@ -31,8 +32,12 @@ struct QCryptoBlock {
     const QCryptoBlockDriver *driver;
     void *opaque;
 
-    QCryptoCipher *cipher;
+    QCryptoCipher **ciphers;
+    int n_ciphers;
+    int n_free_ciphers;
     QCryptoIVGen *ivgen;
+    QemuMutex mutex;
+
     QCryptoHashAlgorithm kdfhash;
     size_t niv;
     uint64_t payload_offset; /* In bytes */
@@ -46,6 +51,7 @@ struct QCryptoBlockDriver {
                 QCryptoBlockReadFunc readfunc,
                 void *opaque,
                 unsigned int flags,
+                int n_threads,
                 Error **errp);
 
     int (*create)(QCryptoBlock *block,
@@ -110,4 +116,12 @@ int qcrypto_block_encrypt_helper(QCryptoBlock *block,
                                  size_t len,
                                  Error **errp);
 
+int qcrypto_block_init_cipher(QCryptoBlock *block,
+                              QCryptoCipherAlgorithm alg,
+                              QCryptoCipherMode mode,
+                              const uint8_t *key, size_t nkey,
+                              int n_threads, Error **errp);
+
+void qcrypto_block_free_cipher(QCryptoBlock *block);
+
 #endif /* QCRYPTO_BLOCKPRIV_H */
diff --git a/include/crypto/block.h b/include/crypto/block.h
index cd18f46d56..d54044323a 100644
--- a/include/crypto/block.h
+++ b/include/crypto/block.h
@@ -75,6 +75,8 @@ typedef enum {
  * @readfunc: callback for reading data from the volume
  * @opaque: data to pass to @readfunc
  * @flags: bitmask of QCryptoBlockOpenFlags values
+ * @n_threads: prepare block to multi tasking with up to
+ *             @n_threads threads
  * @errp: pointer to a NULL-initialized error object
  *
  * Create a new block encryption object for an existing
@@ -107,6 +109,7 @@ QCryptoBlock *qcrypto_block_open(QCryptoBlockOpenOptions *options,
                                  QCryptoBlockReadFunc readfunc,
                                  void *opaque,
                                  unsigned int flags,
+                                 int n_threads,
                                  Error **errp);
 
 /**
@@ -202,12 +205,23 @@ int qcrypto_block_encrypt(QCryptoBlock *block,
  * qcrypto_block_get_cipher:
  * @block: the block encryption object
  *
- * Get the cipher to use for payload encryption
+ * Get the cipher to use for payload encryption. Must be paired with
+ * qcrypto_block_put_cipher.
  *
  * Returns: the cipher object
  */
 QCryptoCipher *qcrypto_block_get_cipher(QCryptoBlock *block);
 
+/**
+ * qcrypto_block_put_cipher:
+ * @block: the block encryption object
+ *
+ * Put cipher back to the block. Must follow qcrypto_block_get_cipher.
+ *
+ * Returns: the cipher object
+ */
+void qcrypto_block_put_cipher(QCryptoBlock *block, QCryptoCipher *cipher);
+
 /**
  * qcrypto_block_get_ivgen:
  * @block: the block encryption object
diff --git a/block/crypto.c b/block/crypto.c
index 33ee01bebd..f0a5f6b987 100644
--- a/block/crypto.c
+++ b/block/crypto.c
@@ -229,6 +229,7 @@ static int block_crypto_open_generic(QCryptoBlockFormat format,
                                        block_crypto_read_func,
                                        bs,
                                        cflags,
+                                       1,
                                        errp);
 
     if (!crypto->block) {
diff --git a/block/qcow.c b/block/qcow.c
index 4518cb4c35..0a235bf393 100644
--- a/block/qcow.c
+++ b/block/qcow.c
@@ -213,7 +213,7 @@ static int qcow_open(BlockDriverState *bs, QDict *options, int flags,
                 cflags |= QCRYPTO_BLOCK_OPEN_NO_IO;
             }
             s->crypto = qcrypto_block_open(crypto_opts, "encrypt.",
-                                           NULL, NULL, cflags, errp);
+                                           NULL, NULL, cflags, 1, errp);
             if (!s->crypto) {
                 ret = -EINVAL;
                 goto fail;
diff --git a/block/qcow2.c b/block/qcow2.c
index 991d6ac91b..bc8868c36a 100644
--- a/block/qcow2.c
+++ b/block/qcow2.c
@@ -294,7 +294,7 @@ static int qcow2_read_extensions(BlockDriverState *bs, uint64_t start_offset,
             }
             s->crypto = qcrypto_block_open(s->crypto_opts, "encrypt.",
                                            qcow2_crypto_hdr_read_func,
-                                           bs, cflags, errp);
+                                           bs, cflags, 1, errp);
             if (!s->crypto) {
                 return -EINVAL;
             }
@@ -1445,7 +1445,7 @@ static int coroutine_fn qcow2_do_open(BlockDriverState *bs, QDict *options,
                 cflags |= QCRYPTO_BLOCK_OPEN_NO_IO;
             }
             s->crypto = qcrypto_block_open(s->crypto_opts, "encrypt.",
-                                           NULL, NULL, cflags, errp);
+                                           NULL, NULL, cflags, 1, errp);
             if (!s->crypto) {
                 ret = -EINVAL;
                 goto fail;
diff --git a/crypto/block-luks.c b/crypto/block-luks.c
index bbffd80fff..41fb0aa2e7 100644
--- a/crypto/block-luks.c
+++ b/crypto/block-luks.c
@@ -636,6 +636,7 @@ qcrypto_block_luks_open(QCryptoBlock *block,
                         QCryptoBlockReadFunc readfunc,
                         void *opaque,
                         unsigned int flags,
+                        int n_threads,
                         Error **errp)
 {
     QCryptoBlockLUKS *luks;
@@ -836,11 +837,10 @@ qcrypto_block_luks_open(QCryptoBlock *block,
             goto fail;
         }
 
-        block->cipher = qcrypto_cipher_new(cipheralg,
-                                           ciphermode,
-                                           masterkey, masterkeylen,
-                                           errp);
-        if (!block->cipher) {
+        ret = qcrypto_block_init_cipher(block, cipheralg, ciphermode,
+                                        masterkey, masterkeylen, n_threads,
+                                        errp);
+        if (ret < 0) {
             ret = -ENOTSUP;
             goto fail;
         }
@@ -863,7 +863,7 @@ qcrypto_block_luks_open(QCryptoBlock *block,
 
  fail:
     g_free(masterkey);
-    qcrypto_cipher_free(block->cipher);
+    qcrypto_block_free_cipher(block);
     qcrypto_ivgen_free(block->ivgen);
     g_free(luks);
     g_free(password);
@@ -888,6 +888,7 @@ qcrypto_block_luks_create(QCryptoBlock *block,
                           void *opaque,
                           Error **errp)
 {
+    int ret;
     QCryptoBlockLUKS *luks;
     QCryptoBlockCreateOptionsLUKS luks_opts;
     Error *local_err = NULL;
@@ -1030,11 +1031,11 @@ qcrypto_block_luks_create(QCryptoBlock *block,
 
 
     /* Setup the block device payload encryption objects */
-    block->cipher = qcrypto_cipher_new(luks_opts.cipher_alg,
-                                       luks_opts.cipher_mode,
-                                       masterkey, luks->header.key_bytes,
-                                       errp);
-    if (!block->cipher) {
+    ret = qcrypto_block_init_cipher(block, luks_opts.cipher_alg,
+                                    luks_opts.cipher_mode,
+                                    masterkey, luks->header.key_bytes,
+                                    1, errp);
+    if (ret < 0) {
         goto error;
     }
 
@@ -1341,7 +1342,7 @@ qcrypto_block_luks_create(QCryptoBlock *block,
     qcrypto_ivgen_free(ivgen);
     qcrypto_cipher_free(cipher);
 
-    qcrypto_cipher_free(block->cipher);
+    qcrypto_block_free_cipher(block);
     qcrypto_ivgen_free(block->ivgen);
 
     g_free(luks);
diff --git a/crypto/block-qcow.c b/crypto/block-qcow.c
index 36bf5f09b7..e668145561 100644
--- a/crypto/block-qcow.c
+++ b/crypto/block-qcow.c
@@ -44,6 +44,7 @@ qcrypto_block_qcow_has_format(const uint8_t *buf G_GNUC_UNUSED,
 static int
 qcrypto_block_qcow_init(QCryptoBlock *block,
                         const char *keysecret,
+                        int n_threads,
                         Error **errp)
 {
     char *password;
@@ -71,11 +72,11 @@ qcrypto_block_qcow_init(QCryptoBlock *block,
         goto fail;
     }
 
-    block->cipher = qcrypto_cipher_new(QCRYPTO_CIPHER_ALG_AES_128,
-                                       QCRYPTO_CIPHER_MODE_CBC,
-                                       keybuf, G_N_ELEMENTS(keybuf),
-                                       errp);
-    if (!block->cipher) {
+    ret = qcrypto_block_init_cipher(block, QCRYPTO_CIPHER_ALG_AES_128,
+                                    QCRYPTO_CIPHER_MODE_CBC,
+                                    keybuf, G_N_ELEMENTS(keybuf),
+                                    n_threads, errp);
+    if (ret < 0) {
         ret = -ENOTSUP;
         goto fail;
     }
@@ -86,7 +87,7 @@ qcrypto_block_qcow_init(QCryptoBlock *block,
     return 0;
 
  fail:
-    qcrypto_cipher_free(block->cipher);
+    qcrypto_block_free_cipher(block);
     qcrypto_ivgen_free(block->ivgen);
     return ret;
 }
@@ -99,6 +100,7 @@ qcrypto_block_qcow_open(QCryptoBlock *block,
                         QCryptoBlockReadFunc readfunc G_GNUC_UNUSED,
                         void *opaque G_GNUC_UNUSED,
                         unsigned int flags,
+                        int n_threads,
                         Error **errp)
 {
     if (flags & QCRYPTO_BLOCK_OPEN_NO_IO) {
@@ -112,8 +114,8 @@ qcrypto_block_qcow_open(QCryptoBlock *block,
                        optprefix ? optprefix : "");
             return -1;
         }
-        return qcrypto_block_qcow_init(block,
-                                       options->u.qcow.key_secret, errp);
+        return qcrypto_block_qcow_init(block, options->u.qcow.key_secret,
+                                       n_threads, errp);
     }
 }
 
@@ -133,7 +135,7 @@ qcrypto_block_qcow_create(QCryptoBlock *block,
         return -1;
     }
     /* QCow2 has no special header, since everything is hardwired */
-    return qcrypto_block_qcow_init(block, options->u.qcow.key_secret, errp);
+    return qcrypto_block_qcow_init(block, options->u.qcow.key_secret, 1, errp);
 }
 
 
diff --git a/crypto/block.c b/crypto/block.c
index 3edd9ec251..58c2ba1987 100644
--- a/crypto/block.c
+++ b/crypto/block.c
@@ -52,10 +52,13 @@ QCryptoBlock *qcrypto_block_open(QCryptoBlockOpenOptions *options,
                                  QCryptoBlockReadFunc readfunc,
                                  void *opaque,
                                  unsigned int flags,
+                                 int n_threads,
                                  Error **errp)
 {
     QCryptoBlock *block = g_new0(QCryptoBlock, 1);
 
+    qemu_mutex_init(&block->mutex);
+
     block->format = options->format;
 
     if (options->format >= G_N_ELEMENTS(qcrypto_block_drivers) ||
@@ -69,7 +72,8 @@ QCryptoBlock *qcrypto_block_open(QCryptoBlockOpenOptions *options,
     block->driver = qcrypto_block_drivers[options->format];
 
     if (block->driver->open(block, options, optprefix,
-                            readfunc, opaque, flags, errp) < 0) {
+                            readfunc, opaque, flags, n_threads, errp) < 0)
+    {
         g_free(block);
         return NULL;
     }
@@ -87,6 +91,8 @@ QCryptoBlock *qcrypto_block_create(QCryptoBlockCreateOptions *options,
 {
     QCryptoBlock *block = g_new0(QCryptoBlock, 1);
 
+    qemu_mutex_init(&block->mutex);
+
     block->format = options->format;
 
     if (options->format >= G_N_ELEMENTS(qcrypto_block_drivers) ||
@@ -148,12 +154,83 @@ int qcrypto_block_encrypt(QCryptoBlock *block,
 
 QCryptoCipher *qcrypto_block_get_cipher(QCryptoBlock *block)
 {
-    return block->cipher;
+    QCryptoCipher *cipher;
+
+    qemu_mutex_lock(&block->mutex);
+
+    assert(block->n_free_ciphers > 0);
+    block->n_free_ciphers--;
+    cipher = block->ciphers[block->n_free_ciphers];
+
+    qemu_mutex_unlock(&block->mutex);
+
+    return cipher;
+}
+
+
+void qcrypto_block_put_cipher(QCryptoBlock *block, QCryptoCipher *cipher)
+{
+    qemu_mutex_lock(&block->mutex);
+
+    assert(block->n_free_ciphers < block->n_ciphers);
+    block->ciphers[block->n_free_ciphers] = cipher;
+    block->n_free_ciphers++;
+
+    qemu_mutex_unlock(&block->mutex);
 }
 
 
+int qcrypto_block_init_cipher(QCryptoBlock *block,
+                              QCryptoCipherAlgorithm alg,
+                              QCryptoCipherMode mode,
+                              const uint8_t *key, size_t nkey,
+                              int n_threads, Error **errp)
+{
+    int i;
+
+    assert(!block->ciphers && !block->n_ciphers && !block->n_free_ciphers);
+
+    block->ciphers = g_new0(QCryptoCipher *, n_threads);
+
+    for (i = 0; i < n_threads; i++) {
+        block->ciphers[i] = qcrypto_cipher_new(alg, mode, key, nkey, errp);
+        if (!block->ciphers[i]) {
+            qcrypto_block_free_cipher(block);
+            return -1;
+        }
+        block->n_ciphers++;
+        block->n_free_ciphers++;
+    }
+
+    return 0;
+}
+
+
+void qcrypto_block_free_cipher(QCryptoBlock *block)
+{
+    int i;
+
+    if (!block->ciphers) {
+        return;
+    }
+
+    assert(block->n_ciphers == block->n_free_ciphers);
+
+    for (i = 0; i < block->n_ciphers; i++) {
+        qcrypto_cipher_free(block->ciphers[i]);
+    }
+
+    g_free(block->ciphers);
+    block->ciphers = NULL;
+    block->n_ciphers = block->n_free_ciphers = 0;
+}
+
 QCryptoIVGen *qcrypto_block_get_ivgen(QCryptoBlock *block)
 {
+    /* ivgen should be accessed under mutex. However, this function is used only
+     * in test with one thread, so it's enough to assert it here:
+     */
+    assert(block->n_ciphers == 1);
     return block->ivgen;
 }
 
@@ -184,7 +261,7 @@ void qcrypto_block_free(QCryptoBlock *block)
 
     block->driver->cleanup(block);
 
-    qcrypto_cipher_free(block->cipher);
+    qcrypto_block_free_cipher(block);
     qcrypto_ivgen_free(block->ivgen);
     g_free(block);
 }
@@ -199,6 +276,7 @@ typedef int (*QCryptoCipherEncryptFunc)(QCryptoCipher *cipher,
 static int do_qcrypto_cipher_encrypt(QCryptoCipher *cipher,
                                      size_t niv,
                                      QCryptoIVGen *ivgen,
+                                     QemuMutex *ivgen_mutex,
                                      int sectorsize,
                                      uint64_t offset,
                                      uint8_t *buf,
@@ -218,10 +296,15 @@ static int do_qcrypto_cipher_encrypt(QCryptoCipher *cipher,
     while (len > 0) {
         size_t nbytes;
         if (niv) {
-            if (qcrypto_ivgen_calculate(ivgen,
-                                        startsector,
-                                        iv, niv,
-                                        errp) < 0) {
+            if (ivgen_mutex) {
+                qemu_mutex_lock(ivgen_mutex);
+            }
+            ret = qcrypto_ivgen_calculate(ivgen, startsector, iv, niv, errp);
+            if (ivgen_mutex) {
+                qemu_mutex_unlock(ivgen_mutex);
+            }
+
+            if (ret < 0) {
                 goto cleanup;
             }
 
@@ -258,8 +341,9 @@ int qcrypto_cipher_decrypt_helper(QCryptoCipher *cipher,
                                   size_t len,
                                   Error **errp)
 {
-    return do_qcrypto_cipher_encrypt(cipher, niv, ivgen, sectorsize, offset,
-                                     buf, len, qcrypto_cipher_decrypt, errp);
+    return do_qcrypto_cipher_encrypt(cipher, niv, ivgen, NULL, sectorsize,
+                                     offset, buf, len, qcrypto_cipher_decrypt,
+                                     errp);
 }
 
 
@@ -272,11 +356,11 @@ int qcrypto_cipher_encrypt_helper(QCryptoCipher *cipher,
                                   size_t len,
                                   Error **errp)
 {
-    return do_qcrypto_cipher_encrypt(cipher, niv, ivgen, sectorsize, offset,
-                                     buf, len, qcrypto_cipher_encrypt, errp);
+    return do_qcrypto_cipher_encrypt(cipher, niv, ivgen, NULL, sectorsize,
+                                     offset, buf, len, qcrypto_cipher_encrypt,
+                                     errp);
 }
 
-
 int qcrypto_block_decrypt_helper(QCryptoBlock *block,
                                  int sectorsize,
                                  uint64_t offset,
@@ -284,11 +368,17 @@ int qcrypto_block_decrypt_helper(QCryptoBlock *block,
                                  size_t len,
                                  Error **errp)
 {
-    return do_qcrypto_cipher_encrypt(block->cipher, block->niv, block->ivgen,
-                                     sectorsize, offset, buf, len,
-                                     qcrypto_cipher_decrypt, errp);
-}
+    int ret;
+    QCryptoCipher *cipher = qcrypto_block_get_cipher(block);
 
+    ret = do_qcrypto_cipher_encrypt(cipher, block->niv, block->ivgen,
+                                    &block->mutex, sectorsize, offset, buf, len,
+                                    qcrypto_cipher_decrypt, errp);
+
+    qcrypto_block_put_cipher(block, cipher);
+
+    return ret;
+}
 
 int qcrypto_block_encrypt_helper(QCryptoBlock *block,
                                  int sectorsize,
@@ -297,7 +387,14 @@ int qcrypto_block_encrypt_helper(QCryptoBlock *block,
                                  size_t len,
                                  Error **errp)
 {
-    return do_qcrypto_cipher_encrypt(block->cipher, block->niv, block->ivgen,
-                                     sectorsize, offset, buf, len,
-                                     qcrypto_cipher_encrypt, errp);
+    int ret;
+    QCryptoCipher *cipher = qcrypto_block_get_cipher(block);
+
+    ret = do_qcrypto_cipher_encrypt(cipher, block->niv, block->ivgen,
+                                    &block->mutex, sectorsize, offset, buf, len,
+                                    qcrypto_cipher_encrypt, errp);
+
+    qcrypto_block_put_cipher(block, cipher);
+
+    return ret;
 }
diff --git a/tests/test-crypto-block.c b/tests/test-crypto-block.c
index fae4ffc453..097497b5bf 100644
--- a/tests/test-crypto-block.c
+++ b/tests/test-crypto-block.c
@@ -269,6 +269,8 @@ static void test_block_assert_setup(const struct QCryptoBlockTestData *data,
                     qcrypto_ivgen_get_algorithm(ivgen));
     g_assert_cmpint(data->ivgen_hash, ==,
                     qcrypto_ivgen_get_hash(ivgen));
+
+    qcrypto_block_put_cipher(blk, cipher);
 }
 
 
-- 
2.18.0

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

* Re: [Qemu-devel] [PATCH for-3.1? 1/5] crypto/block-luks: fix memory leak in qcrypto_block_luks_create
  2018-12-04 18:46 ` [Qemu-devel] [PATCH for-3.1? 1/5] crypto/block-luks: fix memory leak in qcrypto_block_luks_create Vladimir Sementsov-Ogievskiy
@ 2018-12-04 18:53   ` Peter Maydell
  0 siblings, 0 replies; 8+ messages in thread
From: Peter Maydell @ 2018-12-04 18:53 UTC (permalink / raw)
  To: Vladimir Sementsov-Ogievskiy
  Cc: QEMU Developers, Qemu-block, Kevin Wolf, Denis V. Lunev,
	Max Reitz

On Tue, 4 Dec 2018 at 18:50, Vladimir Sementsov-Ogievskiy
<vsementsov@virtuozzo.com> wrote:
>
> Free block->cipher and block->ivgen on error path.
>
> Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
> ---
>  crypto/block-luks.c | 3 +++
>  1 file changed, 3 insertions(+)
>
> diff --git a/crypto/block-luks.c b/crypto/block-luks.c
> index 5738124773..51e24d23ca 100644
> --- a/crypto/block-luks.c
> +++ b/crypto/block-luks.c
> @@ -1341,6 +1341,9 @@ qcrypto_block_luks_create(QCryptoBlock *block,
>      qcrypto_ivgen_free(ivgen);
>      qcrypto_cipher_free(cipher);
>
> +    qcrypto_cipher_free(block->cipher);
> +    qcrypto_ivgen_free(block->ivgen);
> +
>      g_free(luks);
>      return -1;
>  }
> --

Too late for 3.1, I'm afraid. You should probably add the
cc tag for stable.

thanks
-- PMM

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

* Re: [Qemu-devel] [PATCH 0/5] crypto threads
  2018-12-04 18:46 [Qemu-devel] [PATCH 0/5] crypto threads Vladimir Sementsov-Ogievskiy
                   ` (4 preceding siblings ...)
  2018-12-04 18:46 ` [Qemu-devel] [PATCH 5/5] crypto: support multiple threads accessing one QCryptoBlock Vladimir Sementsov-Ogievskiy
@ 2018-12-04 23:23 ` no-reply
  5 siblings, 0 replies; 8+ messages in thread
From: no-reply @ 2018-12-04 23:23 UTC (permalink / raw)
  To: vsementsov; +Cc: famz, qemu-devel, qemu-block, kwolf, den, mreitz

Patchew URL: https://patchew.org/QEMU/20181204184657.78028-1-vsementsov@virtuozzo.com/



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
time make docker-test-quick@centos7 SHOW_ENV=1 J=8
=== TEST SCRIPT END ===

libpmem support   no
libudev           no

WARNING: Use of SDL 1.2 is deprecated and will be removed in
WARNING: future releases. Please switch to using SDL 2.0

NOTE: cross-compilers enabled:  'cc'
  GEN     x86_64-softmmu/config-devices.mak.tmp
---
  CC      tests/usb-hcd-uhci-test.o
  CC      tests/usb-hcd-xhci-test.o
/tmp/qemu-test/src/tests/test-crypto-block.c: In function 'test_block':
/tmp/qemu-test/src/tests/test-crypto-block.c:310:30: error: passing argument 6 of 'qcrypto_block_open' makes integer from pointer without a cast [-Werror]
                              NULL);
                              ^
In file included from /tmp/qemu-test/src/tests/test-crypto-block.c:24:0:
/tmp/qemu-test/src/include/crypto/block.h:107:15: note: expected 'int' but argument is of type 'void *'
 QCryptoBlock *qcrypto_block_open(QCryptoBlockOpenOptions *options,
               ^
/tmp/qemu-test/src/tests/test-crypto-block.c:310:30: error: too few arguments to function 'qcrypto_block_open'
                              NULL);
                              ^
In file included from /tmp/qemu-test/src/tests/test-crypto-block.c:24:0:
/tmp/qemu-test/src/include/crypto/block.h:107:15: note: declared here
 QCryptoBlock *qcrypto_block_open(QCryptoBlockOpenOptions *options,
               ^
/tmp/qemu-test/src/tests/test-crypto-block.c:318:30: error: passing argument 6 of 'qcrypto_block_open' makes integer from pointer without a cast [-Werror]
                              &error_abort);
                              ^
In file included from /tmp/qemu-test/src/tests/test-crypto-block.c:24:0:
/tmp/qemu-test/src/include/crypto/block.h:107:15: note: expected 'int' but argument is of type 'struct Error **'
 QCryptoBlock *qcrypto_block_open(QCryptoBlockOpenOptions *options,
               ^
/tmp/qemu-test/src/tests/test-crypto-block.c:318:30: error: too few arguments to function 'qcrypto_block_open'
                              &error_abort);
                              ^
In file included from /tmp/qemu-test/src/tests/test-crypto-block.c:24:0:
/tmp/qemu-test/src/include/crypto/block.h:107:15: note: declared here
 QCryptoBlock *qcrypto_block_open(QCryptoBlockOpenOptions *options,
               ^
/tmp/qemu-test/src/tests/test-crypto-block.c:332:30: error: passing argument 6 of 'qcrypto_block_open' makes integer from pointer without a cast [-Werror]
                              &error_abort);
                              ^
In file included from /tmp/qemu-test/src/tests/test-crypto-block.c:24:0:
/tmp/qemu-test/src/include/crypto/block.h:107:15: note: expected 'int' but argument is of type 'struct Error **'
 QCryptoBlock *qcrypto_block_open(QCryptoBlockOpenOptions *options,
               ^
/tmp/qemu-test/src/tests/test-crypto-block.c:332:30: error: too few arguments to function 'qcrypto_block_open'
                              &error_abort);
                              ^
In file included from /tmp/qemu-test/src/tests/test-crypto-block.c:24:0:


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

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

end of thread, other threads:[~2018-12-04 23:39 UTC | newest]

Thread overview: 8+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2018-12-04 18:46 [Qemu-devel] [PATCH 0/5] crypto threads Vladimir Sementsov-Ogievskiy
2018-12-04 18:46 ` [Qemu-devel] [PATCH for-3.1? 1/5] crypto/block-luks: fix memory leak in qcrypto_block_luks_create Vladimir Sementsov-Ogievskiy
2018-12-04 18:53   ` Peter Maydell
2018-12-04 18:46 ` [Qemu-devel] [PATCH 2/5] crypto/block: refactor qcrypto_block_*crypt_helper functions Vladimir Sementsov-Ogievskiy
2018-12-04 18:46 ` [Qemu-devel] [PATCH 3/5] crypto/block: rename qcrypto_block_*crypt_helper Vladimir Sementsov-Ogievskiy
2018-12-04 18:46 ` [Qemu-devel] [PATCH 4/5] crypto/block: introduce qcrypto_block_*crypt_helper functions Vladimir Sementsov-Ogievskiy
2018-12-04 18:46 ` [Qemu-devel] [PATCH 5/5] crypto: support multiple threads accessing one QCryptoBlock Vladimir Sementsov-Ogievskiy
2018-12-04 23:23 ` [Qemu-devel] [PATCH 0/5] crypto threads no-reply

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).