From: "Daniel P. Berrangé" <berrange@redhat.com>
To: qemu-devel@nongnu.org
Cc: "Hyman Huang" <yong.huang@smartx.com>,
"Daniel P. Berrangé" <berrange@redhat.com>,
"Marc-André Lureau" <marcandre.lureau@redhat.com>,
"Eric Blake" <eblake@redhat.com>,
"Philippe Mathieu-Daudé" <philmd@linaro.org>,
"Hanna Reitz" <hreitz@redhat.com>,
qemu-block@nongnu.org, qemu-stable@nongnu.org,
"Laurent Vivier" <lvivier@redhat.com>,
"Thomas Huth" <thuth@redhat.com>,
"Paolo Bonzini" <pbonzini@redhat.com>,
"Kevin Wolf" <kwolf@redhat.com>,
"Markus Armbruster" <armbru@redhat.com>,
"Tiago Pasqualini" <tiago.pasqualini@canonical.com>
Subject: [PULL 02/10] crypto: run qcrypto_pbkdf2_count_iters in a new thread
Date: Mon, 9 Sep 2024 15:16:27 +0100 [thread overview]
Message-ID: <20240909141635.1459701-3-berrange@redhat.com> (raw)
In-Reply-To: <20240909141635.1459701-1-berrange@redhat.com>
From: Tiago Pasqualini <tiago.pasqualini@canonical.com>
CPU time accounting in the kernel has been demonstrated to have a
sawtooth pattern[1][2]. This can cause the getrusage system call to
not be as accurate as we are expecting, which can cause this calculation
to stall.
The kernel discussions shows that this inaccuracy happens when CPU time
gets big enough, so this patch changes qcrypto_pbkdf2_count_iters to run
in a fresh thread to avoid this inaccuracy. It also adds a sanity check
to fail the process if CPU time is not accounted.
[1] https://lore.kernel.org/lkml/159231011694.16989.16351419333851309713.tip-bot2@tip-bot2/
[2] https://lore.kernel.org/lkml/20221226031010.4079885-1-maxing.lan@bytedance.com/t/#m1c7f2fdc0ea742776a70fd1aa2a2e414c437f534
Resolves: #2398
Signed-off-by: Tiago Pasqualini <tiago.pasqualini@canonical.com>
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
---
crypto/pbkdf.c | 53 +++++++++++++++++++++++++++++++++++++++++++-------
1 file changed, 46 insertions(+), 7 deletions(-)
diff --git a/crypto/pbkdf.c b/crypto/pbkdf.c
index 8d198c152c..d1c06ef3ed 100644
--- a/crypto/pbkdf.c
+++ b/crypto/pbkdf.c
@@ -19,6 +19,7 @@
*/
#include "qemu/osdep.h"
+#include "qemu/thread.h"
#include "qapi/error.h"
#include "crypto/pbkdf.h"
#ifndef _WIN32
@@ -85,12 +86,28 @@ static int qcrypto_pbkdf2_get_thread_cpu(unsigned long long *val_ms,
#endif
}
-uint64_t qcrypto_pbkdf2_count_iters(QCryptoHashAlgorithm hash,
- const uint8_t *key, size_t nkey,
- const uint8_t *salt, size_t nsalt,
- size_t nout,
- Error **errp)
+typedef struct CountItersData {
+ QCryptoHashAlgorithm hash;
+ const uint8_t *key;
+ size_t nkey;
+ const uint8_t *salt;
+ size_t nsalt;
+ size_t nout;
+ uint64_t iterations;
+ Error **errp;
+} CountItersData;
+
+static void *threaded_qcrypto_pbkdf2_count_iters(void *data)
{
+ CountItersData *iters_data = (CountItersData *) data;
+ QCryptoHashAlgorithm hash = iters_data->hash;
+ const uint8_t *key = iters_data->key;
+ size_t nkey = iters_data->nkey;
+ const uint8_t *salt = iters_data->salt;
+ size_t nsalt = iters_data->nsalt;
+ size_t nout = iters_data->nout;
+ Error **errp = iters_data->errp;
+
uint64_t ret = -1;
g_autofree uint8_t *out = g_new(uint8_t, nout);
uint64_t iterations = (1 << 15);
@@ -114,7 +131,10 @@ uint64_t qcrypto_pbkdf2_count_iters(QCryptoHashAlgorithm hash,
delta_ms = end_ms - start_ms;
- if (delta_ms > 500) {
+ if (delta_ms == 0) { /* sanity check */
+ error_setg(errp, "Unable to get accurate CPU usage");
+ goto cleanup;
+ } else if (delta_ms > 500) {
break;
} else if (delta_ms < 100) {
iterations = iterations * 10;
@@ -129,5 +149,24 @@ uint64_t qcrypto_pbkdf2_count_iters(QCryptoHashAlgorithm hash,
cleanup:
memset(out, 0, nout);
- return ret;
+ iters_data->iterations = ret;
+ return NULL;
+}
+
+uint64_t qcrypto_pbkdf2_count_iters(QCryptoHashAlgorithm hash,
+ const uint8_t *key, size_t nkey,
+ const uint8_t *salt, size_t nsalt,
+ size_t nout,
+ Error **errp)
+{
+ CountItersData data = {
+ hash, key, nkey, salt, nsalt, nout, 0, errp
+ };
+ QemuThread thread;
+
+ qemu_thread_create(&thread, "pbkdf2", threaded_qcrypto_pbkdf2_count_iters,
+ &data, QEMU_THREAD_JOINABLE);
+ qemu_thread_join(&thread);
+
+ return data.iterations;
}
--
2.45.2
next prev parent reply other threads:[~2024-09-09 14:18 UTC|newest]
Thread overview: 15+ messages / expand[flat|nested] mbox.gz Atom feed top
2024-09-09 14:16 [PULL 00/10] Crypto fixes patches Daniel P. Berrangé
2024-09-09 14:16 ` [PULL 01/10] iotests: fix expected output from gnutls Daniel P. Berrangé
2024-09-09 14:16 ` Daniel P. Berrangé [this message]
2024-09-09 14:16 ` [PULL 03/10] crypto: check gnutls & gcrypt support the requested pbkdf hash Daniel P. Berrangé
2024-09-09 14:16 ` [PULL 04/10] tests/unit: always build the pbkdf crypto unit test Daniel P. Berrangé
2024-09-09 14:16 ` [PULL 05/10] tests/unit: build pbkdf test on macOS Daniel P. Berrangé
2024-09-09 14:16 ` [PULL 06/10] crypto: avoid leak of ctx when bad cipher mode is given Daniel P. Berrangé
2024-09-09 14:16 ` [PULL 07/10] crypto: use consistent error reporting pattern for unsupported cipher modes Daniel P. Berrangé
2024-09-09 14:16 ` [PULL 08/10] crypto: Define macros for hash algorithm digest lengths Daniel P. Berrangé
2024-09-09 14:16 ` [PULL 09/10] crypto: Support SHA384 hash when using glib Daniel P. Berrangé
2024-09-09 14:16 ` [PULL 10/10] crypto: Introduce x509 utils Daniel P. Berrangé
2025-03-18 16:44 ` Peter Maydell
2025-03-18 17:01 ` Dorjoy Chowdhury
2024-09-09 16:06 ` [PULL 00/10] Crypto fixes patches Peter Maydell
2024-09-11 5:59 ` Michael Tokarev
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20240909141635.1459701-3-berrange@redhat.com \
--to=berrange@redhat.com \
--cc=armbru@redhat.com \
--cc=eblake@redhat.com \
--cc=hreitz@redhat.com \
--cc=kwolf@redhat.com \
--cc=lvivier@redhat.com \
--cc=marcandre.lureau@redhat.com \
--cc=pbonzini@redhat.com \
--cc=philmd@linaro.org \
--cc=qemu-block@nongnu.org \
--cc=qemu-devel@nongnu.org \
--cc=qemu-stable@nongnu.org \
--cc=thuth@redhat.com \
--cc=tiago.pasqualini@canonical.com \
--cc=yong.huang@smartx.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
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).