* 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-----

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>
This commit is contained in:
Peter Maydell 2024-03-20 12:01:22 +00:00
commit 9051995517
5 changed files with 145 additions and 71 deletions

View File

@ -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;
}
}

View File

@ -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);

View File

@ -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;

View File

@ -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),

View File

@ -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));
}
}