Merge tag 'misc-fixes-pull-request' of https://gitlab.com/berrange/qemu into staging

 * Use EPERM for seccomp filter instead of killing QEMU when
   an attempt to spawn child process is made
 * Reduce priority of POLLHUP handling for socket chardevs
   to increase likelihood of pending data being processed
 * Fix chardev I/O main loop integration when TLS is enabled
 * Fix broken crypto test suite when distro disables
   SM4 algorithm
 * Improve diagnosis of failed crypto tests

# -----BEGIN PGP SIGNATURE-----
#
# iQIzBAABCAAdFiEE2vOm/bJrYpEtDo4/vobrtBUQT98FAmX585EACgkQvobrtBUQ
# T98TIg//ekc/f0JrRs68hjmo/vfcHWGHDMbZagj48zZNIn8DhJmQdt+qrCjMrMGW
# 353nTawFuF3EO9ju/eRLO54T+p1+a3zX8TyO4tL1W+RY9HARPeqssmFemDPfkMfQ
# IFGv0M0vaxGZpBna7jlXfDK/hCbJexKoChyT4eSF9H1Tp9o6T2J9AWvB5WTYLoQ2
# GzusDqBLKTkKhxMTCqevkFD/yCkgIQKlX8mG188PoJnGMqpGzQLTyw9lo5Npi1nE
# nhXa2MrrSfusk0rtwEzT14sQ58U+MF4fLQxUC+knNX81FSv8Q6QDu4Stfhwc+az7
# ynO4b/3IzK+VCICb2QM1ZNoTZNLcLfw1jdFTIAt8wiE+BMSySNQtdneURZOynydy
# Qd0alPNb4zfVRIGVjoOj38HiOmIKp5riIsUsI03jjBAgJu47tYRi60Tq2t6KxVoP
# rpDd5Vmsd0AR+7acO29rp0aLB+x2/ANDY+1N1Xi4tQdblmKIziHPZzx6H49wbwev
# 8Jdghg10RpbdqIGOfZ9fn13iCDO+1/gy6g/jTe2tMZrZsyov904tDqyUCDCzAbTz
# B8lvnr0LfSX2DYBryGEHIa/eMN2TxPuzpvZP0JFO1QxJnOs9w3aHr1T6A1sCV4a3
# JjTu71LsomNMXj3t3ImBHzMlgQZoL5Bxoh7b7jbLO4cvnhRbiJk=
# =4HKW
# -----END PGP SIGNATURE-----
# gpg: Signature made Tue 19 Mar 2024 20:20:33 GMT
# gpg:                using RSA key DAF3A6FDB26B62912D0E8E3FBE86EBB415104FDF
# gpg: Good signature from "Daniel P. Berrange <dan@berrange.com>" [full]
# gpg:                 aka "Daniel P. Berrange <berrange@redhat.com>" [full]
# Primary key fingerprint: DAF3 A6FD B26B 6291 2D0E  8E3F BE86 EBB4 1510 4FDF

* tag 'misc-fixes-pull-request' of https://gitlab.com/berrange/qemu:
  crypto: report which ciphers are being skipped during tests
  crypto: use error_abort for unexpected failures
  crypto: query gcrypt for cipher availability
  crypto: factor out conversion of QAPI to gcrypt constants
  Revert "chardev: use a child source for qio input source"
  Revert "chardev/char-socket: Fix TLS io channels sending too much data to the backend"
  chardev: lower priority of the HUP GSource in socket chardev
  seccomp: report EPERM instead of killing process for spawn set

Signed-off-by: Peter Maydell <peter.maydell@linaro.org>

5 files changed, 145 insertions, 71 deletions

diff --git a/chardev/char-io.c b/chardev/char-io.c
index 4451128..dab77b1 100644
--- a/chardev/char-io.c
+++ b/chardev/char-io.c
@@ -33,6 +33,7 @@ typedef struct IOWatchPoll {
     IOCanReadHandler *fd_can_read;
     GSourceFunc fd_read;
     void *opaque;
+    GMainContext *context;
 } IOWatchPoll;
 
 static IOWatchPoll *io_watch_poll_from_source(GSource *source)
@@ -50,28 +51,59 @@ static gboolean io_watch_poll_prepare(GSource *source,
         return FALSE;
     }
 
+    /*
+     * We do not register the QIOChannel watch as a child GSource.
+     * The 'prepare' function on the parent GSource will be
+     * skipped if a child GSource's 'prepare' function indicates
+     * readiness. We need this prepare function be guaranteed
+     * to run on *every* iteration of the main loop, because
+     * it is critical to ensure we remove the QIOChannel watch
+     * if 'fd_can_read' indicates the frontend cannot receive
+     * more data.
+     */
     if (now_active) {
         iwp->src = qio_channel_create_watch(
             iwp->ioc, G_IO_IN | G_IO_ERR | G_IO_HUP | G_IO_NVAL);
         g_source_set_callback(iwp->src, iwp->fd_read, iwp->opaque, NULL);
-        g_source_add_child_source(source, iwp->src);
-        g_source_unref(iwp->src);
+        g_source_attach(iwp->src, iwp->context);
     } else {
-        g_source_remove_child_source(source, iwp->src);
+        g_source_destroy(iwp->src);
+        g_source_unref(iwp->src);
         iwp->src = NULL;
     }
     return FALSE;
 }
 
+static gboolean io_watch_poll_check(GSource *source)
+{
+    return FALSE;
+}
+
 static gboolean io_watch_poll_dispatch(GSource *source, GSourceFunc callback,
                                        gpointer user_data)
 {
-    return G_SOURCE_CONTINUE;
+    abort();
+}
+
+static void io_watch_poll_finalize(GSource *source)
+{
+    /*
+     * Due to a glib bug, removing the last reference to a source
+     * inside a finalize callback causes recursive locking (and a
+     * deadlock).  This is not a problem inside other callbacks,
+     * including dispatch callbacks, so we call io_remove_watch_poll
+     * to remove this source.  At this point, iwp->src must
+     * be NULL, or we would leak it.
+     */
+    IOWatchPoll *iwp = io_watch_poll_from_source(source);
+    assert(iwp->src == NULL);
 }
 
 static GSourceFuncs io_watch_poll_funcs = {
     .prepare = io_watch_poll_prepare,
+    .check = io_watch_poll_check,
     .dispatch = io_watch_poll_dispatch,
+    .finalize = io_watch_poll_finalize,
 };
 
 GSource *io_add_watch_poll(Chardev *chr,
@@ -91,6 +123,7 @@ GSource *io_add_watch_poll(Chardev *chr,
     iwp->ioc = ioc;
     iwp->fd_read = (GSourceFunc) fd_read;
     iwp->src = NULL;
+    iwp->context = context;
 
     name = g_strdup_printf("chardev-iowatch-%s", chr->label);
     g_source_set_name((GSource *)iwp, name);
@@ -101,10 +134,23 @@ GSource *io_add_watch_poll(Chardev *chr,
     return (GSource *)iwp;
 }
 
+static void io_remove_watch_poll(GSource *source)
+{
+    IOWatchPoll *iwp;
+
+    iwp = io_watch_poll_from_source(source);
+    if (iwp->src) {
+        g_source_destroy(iwp->src);
+        g_source_unref(iwp->src);
+        iwp->src = NULL;
+    }
+    g_source_destroy(&iwp->parent);
+}
+
 void remove_fd_in_watch(Chardev *chr)
 {
     if (chr->gsource) {
-        g_source_destroy(chr->gsource);
+        io_remove_watch_poll(chr->gsource);
         chr->gsource = NULL;
     }
 }
diff --git a/chardev/char-socket.c b/chardev/char-socket.c
index 8a0406c..812d7aa 100644
--- a/chardev/char-socket.c
+++ b/chardev/char-socket.c
@@ -496,9 +496,9 @@ static gboolean tcp_chr_read(QIOChannel *chan, GIOCondition cond, void *opaque)
         s->max_size <= 0) {
         return TRUE;
     }
-    len = tcp_chr_read_poll(opaque);
-    if (len > sizeof(buf)) {
-        len = sizeof(buf);
+    len = sizeof(buf);
+    if (len > s->max_size) {
+        len = s->max_size;
     }
     size = tcp_chr_recv(chr, (void *)buf, len);
     if (size == 0 || (size == -1 && errno != EAGAIN)) {
@@ -601,6 +601,22 @@ static void update_ioc_handlers(SocketChardev *s)
 
     remove_hup_source(s);
     s->hup_source = qio_channel_create_watch(s->ioc, G_IO_HUP);
+    /*
+     * poll() is liable to return POLLHUP even when there is
+     * still incoming data available to read on the FD. If
+     * we have the hup_source at the same priority as the
+     * main io_add_watch_poll GSource, then we might end up
+     * processing the POLLHUP event first, closing the FD,
+     * and as a result silently discard data we should have
+     * read.
+     *
+     * By setting the hup_source to G_PRIORITY_DEFAULT + 1,
+     * we ensure that io_add_watch_poll GSource will always
+     * be dispatched first, thus guaranteeing we will be
+     * able to process all incoming data before closing the
+     * FD
+     */
+    g_source_set_priority(s->hup_source, G_PRIORITY_DEFAULT + 1);
     g_source_set_callback(s->hup_source, (GSourceFunc)tcp_chr_hup,
                           chr, NULL);
     g_source_attach(s->hup_source, chr->gcontext);
diff --git a/crypto/cipher-gcrypt.c.inc b/crypto/cipher-gcrypt.c.inc
index 1377cba..4a83147 100644
--- a/crypto/cipher-gcrypt.c.inc
+++ b/crypto/cipher-gcrypt.c.inc
@@ -20,6 +20,56 @@
 
 #include <gcrypt.h>
 
+static int qcrypto_cipher_alg_to_gcry_alg(QCryptoCipherAlgorithm alg)
+{
+    switch (alg) {
+    case QCRYPTO_CIPHER_ALG_DES:
+        return GCRY_CIPHER_DES;
+    case QCRYPTO_CIPHER_ALG_3DES:
+        return GCRY_CIPHER_3DES;
+    case QCRYPTO_CIPHER_ALG_AES_128:
+        return GCRY_CIPHER_AES128;
+    case QCRYPTO_CIPHER_ALG_AES_192:
+        return GCRY_CIPHER_AES192;
+    case QCRYPTO_CIPHER_ALG_AES_256:
+        return GCRY_CIPHER_AES256;
+    case QCRYPTO_CIPHER_ALG_CAST5_128:
+        return GCRY_CIPHER_CAST5;
+    case QCRYPTO_CIPHER_ALG_SERPENT_128:
+        return GCRY_CIPHER_SERPENT128;
+    case QCRYPTO_CIPHER_ALG_SERPENT_192:
+        return GCRY_CIPHER_SERPENT192;
+    case QCRYPTO_CIPHER_ALG_SERPENT_256:
+        return GCRY_CIPHER_SERPENT256;
+    case QCRYPTO_CIPHER_ALG_TWOFISH_128:
+        return GCRY_CIPHER_TWOFISH128;
+    case QCRYPTO_CIPHER_ALG_TWOFISH_256:
+        return GCRY_CIPHER_TWOFISH;
+#ifdef CONFIG_CRYPTO_SM4
+    case QCRYPTO_CIPHER_ALG_SM4:
+        return GCRY_CIPHER_SM4;
+#endif
+    default:
+        return GCRY_CIPHER_NONE;
+    }
+}
+
+static int qcrypto_cipher_mode_to_gcry_mode(QCryptoCipherMode mode)
+{
+    switch (mode) {
+    case QCRYPTO_CIPHER_MODE_ECB:
+        return GCRY_CIPHER_MODE_ECB;
+    case QCRYPTO_CIPHER_MODE_XTS:
+        return GCRY_CIPHER_MODE_XTS;
+    case QCRYPTO_CIPHER_MODE_CBC:
+        return GCRY_CIPHER_MODE_CBC;
+    case QCRYPTO_CIPHER_MODE_CTR:
+        return GCRY_CIPHER_MODE_CTR;
+    default:
+        return GCRY_CIPHER_MODE_NONE;
+    }
+}
+
 bool qcrypto_cipher_supports(QCryptoCipherAlgorithm alg,
                              QCryptoCipherMode mode)
 {
@@ -43,6 +93,11 @@ bool qcrypto_cipher_supports(QCryptoCipherAlgorithm alg,
         return false;
     }
 
+    if (gcry_cipher_algo_info(qcrypto_cipher_alg_to_gcry_alg(alg),
+                              GCRYCTL_TEST_ALGO, NULL, NULL) != 0) {
+        return false;
+    }
+
     switch (mode) {
     case QCRYPTO_CIPHER_MODE_ECB:
     case QCRYPTO_CIPHER_MODE_CBC:
@@ -188,72 +243,26 @@ static QCryptoCipher *qcrypto_cipher_ctx_new(QCryptoCipherAlgorithm alg,
         return NULL;
     }
 
-    switch (alg) {
-    case QCRYPTO_CIPHER_ALG_DES:
-        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;
-#ifdef CONFIG_CRYPTO_SM4
-    case QCRYPTO_CIPHER_ALG_SM4:
-        gcryalg = GCRY_CIPHER_SM4;
-        break;
-#endif
-    default:
+    gcryalg = qcrypto_cipher_alg_to_gcry_alg(alg);
+    if (gcryalg == GCRY_CIPHER_NONE) {
         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:
-        gcrymode = GCRY_CIPHER_MODE_XTS;
-        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:
+    gcrymode = qcrypto_cipher_mode_to_gcry_mode(mode);
+    if (gcrymode == GCRY_CIPHER_MODE_NONE) {
         error_setg(errp, "Unsupported cipher mode %s",
                    QCryptoCipherMode_str(mode));
         return NULL;
     }
 
+    if (mode == QCRYPTO_CIPHER_MODE_CTR) {
+        drv = &qcrypto_gcrypt_ctr_driver;
+    } else {
+        drv = &qcrypto_gcrypt_driver;
+    }
+
     ctx = g_new0(QCryptoCipherGcrypt, 1);
     ctx->base.driver = drv;
 
diff --git a/system/qemu-seccomp.c b/system/qemu-seccomp.c
index 4d7439e..98ffce0 100644
--- a/system/qemu-seccomp.c
+++ b/system/qemu-seccomp.c
@@ -74,7 +74,7 @@ const struct scmp_arg_cmp sched_setscheduler_arg[] = {
 
 #define RULE_CLONE_FLAG(flag) \
     { SCMP_SYS(clone),                  QEMU_SECCOMP_SET_SPAWN, \
-      ARRAY_SIZE(clone_arg ## flag), clone_arg ## flag, SCMP_ACT_TRAP }
+      ARRAY_SIZE(clone_arg ## flag), clone_arg ## flag, SCMP_ACT_ERRNO(EPERM) }
 
 /* If no CLONE_* flags are set, except CSIGNAL, deny */
 const struct scmp_arg_cmp clone_arg_none[] = {
@@ -214,13 +214,13 @@ static const struct QemuSeccompSyscall denylist[] = {
       0, NULL, SCMP_ACT_TRAP },
     /* spawn */
     { SCMP_SYS(fork),                   QEMU_SECCOMP_SET_SPAWN,
-      0, NULL, SCMP_ACT_TRAP },
+      0, NULL, SCMP_ACT_ERRNO(EPERM) },
     { SCMP_SYS(vfork),                  QEMU_SECCOMP_SET_SPAWN,
-      0, NULL, SCMP_ACT_TRAP },
+      0, NULL, SCMP_ACT_ERRNO(EPERM) },
     { SCMP_SYS(execve),                 QEMU_SECCOMP_SET_SPAWN,
-      0, NULL, SCMP_ACT_TRAP },
+      0, NULL, SCMP_ACT_ERRNO(EPERM) },
     { SCMP_SYS(clone),                  QEMU_SECCOMP_SET_SPAWN,
-      ARRAY_SIZE(clone_arg_none), clone_arg_none, SCMP_ACT_TRAP },
+      ARRAY_SIZE(clone_arg_none), clone_arg_none, SCMP_ACT_ERRNO(EPERM) },
     RULE_CLONE_FLAG(CLONE_VM),
     RULE_CLONE_FLAG(CLONE_FS),
     RULE_CLONE_FLAG(CLONE_FILES),
diff --git a/tests/unit/test-crypto-cipher.c b/tests/unit/test-crypto-cipher.c
index 11ab1a5..f5152e5 100644
--- a/tests/unit/test-crypto-cipher.c
+++ b/tests/unit/test-crypto-cipher.c
@@ -676,9 +676,8 @@ static void test_cipher(const void *opaque)
     cipher = qcrypto_cipher_new(
         data->alg, data->mode,
         key, nkey,
-        &err);
+        data->plaintext ? &error_abort : &err);
     if (data->plaintext) {
-        g_assert(err == NULL);
         g_assert(cipher != NULL);
     } else {
         error_free_or_abort(&err);
@@ -822,6 +821,10 @@ int main(int argc, char **argv)
     for (i = 0; i < G_N_ELEMENTS(test_data); i++) {
         if (qcrypto_cipher_supports(test_data[i].alg, test_data[i].mode)) {
             g_test_add_data_func(test_data[i].path, &test_data[i], test_cipher);
+        } else {
+            g_printerr("# skip unsupported %s:%s\n",
+                       QCryptoCipherAlgorithm_str(test_data[i].alg),
+                       QCryptoCipherMode_str(test_data[i].mode));
         }
     }