From: Evgeny Kolmakov <randomjack94dev@gmail.com> Use g_autofree attribute to reduce the amount of manual g_free() calls and 'goto out' code Signed-off-by: Evgeny Kolmakov <randomjack94dev@gmail.com> Reviewed-by: Daniel P. Berrangé <berrange@redhat.com> Signed-off-by: Daniel P. Berrangé <berrange@redhat.com> --- crypto/block.c | 17 ++++++----------- crypto/hmac-gcrypt.c | 12 ++++-------- crypto/hmac-glib.c | 10 +++------- crypto/ivgen-essiv.c | 15 ++++----------- crypto/ivgen.c | 6 ++---- crypto/secret_keyring.c | 5 ++--- 6 files changed, 21 insertions(+), 44 deletions(-) diff --git a/crypto/block.c b/crypto/block.c index 96c83e60b9..42558f3caf 100644 --- a/crypto/block.c +++ b/crypto/block.c @@ -55,7 +55,7 @@ QCryptoBlock *qcrypto_block_open(QCryptoBlockOpenOptions *options, unsigned int flags, Error **errp) { - QCryptoBlock *block = g_new0(QCryptoBlock, 1); + g_autofree QCryptoBlock *block = g_new0(QCryptoBlock, 1); qemu_mutex_init(&block->mutex); @@ -65,7 +65,6 @@ QCryptoBlock *qcrypto_block_open(QCryptoBlockOpenOptions *options, !qcrypto_block_drivers[options->format]) { error_setg(errp, "Unsupported block driver %s", QCryptoBlockFormat_str(options->format)); - g_free(block); return NULL; } @@ -74,11 +73,10 @@ QCryptoBlock *qcrypto_block_open(QCryptoBlockOpenOptions *options, if (block->driver->open(block, options, optprefix, readfunc, opaque, flags, errp) < 0) { - g_free(block); return NULL; } - return block; + return g_steal_pointer(&block); } @@ -90,7 +88,7 @@ QCryptoBlock *qcrypto_block_create(QCryptoBlockCreateOptions *options, unsigned int flags, Error **errp) { - QCryptoBlock *block = g_new0(QCryptoBlock, 1); + g_autofree QCryptoBlock *block = g_new0(QCryptoBlock, 1); qemu_mutex_init(&block->mutex); @@ -100,7 +98,6 @@ QCryptoBlock *qcrypto_block_create(QCryptoBlockCreateOptions *options, !qcrypto_block_drivers[options->format]) { error_setg(errp, "Unsupported block driver %s", QCryptoBlockFormat_str(options->format)); - g_free(block); return NULL; } @@ -109,11 +106,10 @@ QCryptoBlock *qcrypto_block_create(QCryptoBlockCreateOptions *options, if (block->driver->create(block, options, optprefix, initfunc, writefunc, opaque, errp) < 0) { - g_free(block); return NULL; } - return block; + return g_steal_pointer(&block); } @@ -185,17 +181,16 @@ int qcrypto_block_amend_options(QCryptoBlock *block, QCryptoBlockInfo *qcrypto_block_get_info(QCryptoBlock *block, Error **errp) { - QCryptoBlockInfo *info = g_new0(QCryptoBlockInfo, 1); + g_autofree QCryptoBlockInfo *info = g_new0(QCryptoBlockInfo, 1); info->format = block->format; if (block->driver->get_info && block->driver->get_info(block, info, errp) < 0) { - g_free(info); return NULL; } - return info; + return g_steal_pointer(&info); } diff --git a/crypto/hmac-gcrypt.c b/crypto/hmac-gcrypt.c index e428d17479..44631fb348 100644 --- a/crypto/hmac-gcrypt.c +++ b/crypto/hmac-gcrypt.c @@ -50,7 +50,7 @@ void *qcrypto_hmac_ctx_new(QCryptoHashAlgo alg, const uint8_t *key, size_t nkey, Error **errp) { - QCryptoHmacGcrypt *ctx; + g_autofree QCryptoHmacGcrypt *ctx = NULL; gcry_error_t err; if (!qcrypto_hmac_supports(alg)) { @@ -66,7 +66,7 @@ void *qcrypto_hmac_ctx_new(QCryptoHashAlgo alg, if (err != 0) { error_setg(errp, "Cannot initialize hmac: %s", gcry_strerror(err)); - goto error; + return NULL; } err = gcry_mac_setkey(ctx->handle, (const void *)key, nkey); @@ -74,14 +74,10 @@ void *qcrypto_hmac_ctx_new(QCryptoHashAlgo alg, error_setg(errp, "Cannot set key: %s", gcry_strerror(err)); gcry_mac_close(ctx->handle); - goto error; + return NULL; } - return ctx; - -error: - g_free(ctx); - return NULL; + return g_steal_pointer(&ctx); } static void diff --git a/crypto/hmac-glib.c b/crypto/hmac-glib.c index b845133a05..1f17769c1c 100644 --- a/crypto/hmac-glib.c +++ b/crypto/hmac-glib.c @@ -46,7 +46,7 @@ void *qcrypto_hmac_ctx_new(QCryptoHashAlgo alg, const uint8_t *key, size_t nkey, Error **errp) { - QCryptoHmacGlib *ctx; + g_autofree QCryptoHmacGlib *ctx = NULL; if (!qcrypto_hmac_supports(alg)) { error_setg(errp, "Unsupported hmac algorithm %s", @@ -60,14 +60,10 @@ void *qcrypto_hmac_ctx_new(QCryptoHashAlgo alg, (const uint8_t *)key, nkey); if (!ctx->ghmac) { error_setg(errp, "Cannot initialize hmac and set key"); - goto error; + return NULL; } - return ctx; - -error: - g_free(ctx); - return NULL; + return g_steal_pointer(&ctx); } static void diff --git a/crypto/ivgen-essiv.c b/crypto/ivgen-essiv.c index 3d5a188795..d5fa269888 100644 --- a/crypto/ivgen-essiv.c +++ b/crypto/ivgen-essiv.c @@ -31,10 +31,10 @@ static int qcrypto_ivgen_essiv_init(QCryptoIVGen *ivgen, const uint8_t *key, size_t nkey, Error **errp) { - uint8_t *salt; + g_autofree uint8_t *salt = NULL; size_t nhash; size_t nsalt; - QCryptoIVGenESSIV *essiv = g_new0(QCryptoIVGenESSIV, 1); + g_autofree QCryptoIVGenESSIV *essiv = g_new0(QCryptoIVGenESSIV, 1); /* Not necessarily the same as nkey */ nsalt = qcrypto_cipher_get_key_len(ivgen->cipher); @@ -46,8 +46,6 @@ static int qcrypto_ivgen_essiv_init(QCryptoIVGen *ivgen, if (qcrypto_hash_bytes(ivgen->hash, (const gchar *)key, nkey, &salt, &nhash, errp) < 0) { - g_free(essiv); - g_free(salt); return -1; } @@ -57,13 +55,10 @@ static int qcrypto_ivgen_essiv_init(QCryptoIVGen *ivgen, salt, MIN(nhash, nsalt), errp); if (!essiv->cipher) { - g_free(essiv); - g_free(salt); return -1; } - g_free(salt); - ivgen->private = essiv; + ivgen->private = g_steal_pointer(&essiv); return 0; } @@ -75,7 +70,7 @@ static int qcrypto_ivgen_essiv_calculate(QCryptoIVGen *ivgen, { QCryptoIVGenESSIV *essiv = ivgen->private; size_t ndata = qcrypto_cipher_get_block_len(ivgen->cipher); - uint8_t *data = g_new(uint8_t, ndata); + g_autofree uint8_t *data = g_new(uint8_t, ndata); sector = cpu_to_le64(sector); memcpy(data, (uint8_t *)§or, MIN(sizeof(sector), ndata)); @@ -88,7 +83,6 @@ static int qcrypto_ivgen_essiv_calculate(QCryptoIVGen *ivgen, data, ndata, errp) < 0) { - g_free(data); return -1; } @@ -99,7 +93,6 @@ static int qcrypto_ivgen_essiv_calculate(QCryptoIVGen *ivgen, if (ndata < niv) { memset(iv + ndata, 0, niv - ndata); } - g_free(data); return 0; } diff --git a/crypto/ivgen.c b/crypto/ivgen.c index 6b7d24d889..9f1f7d7dca 100644 --- a/crypto/ivgen.c +++ b/crypto/ivgen.c @@ -33,7 +33,7 @@ QCryptoIVGen *qcrypto_ivgen_new(QCryptoIVGenAlgo alg, const uint8_t *key, size_t nkey, Error **errp) { - QCryptoIVGen *ivgen = g_new0(QCryptoIVGen, 1); + g_autofree QCryptoIVGen *ivgen = g_new0(QCryptoIVGen, 1); ivgen->algorithm = alg; ivgen->cipher = cipheralg; @@ -51,16 +51,14 @@ QCryptoIVGen *qcrypto_ivgen_new(QCryptoIVGenAlgo alg, break; default: error_setg(errp, "Unknown block IV generator algorithm %d", alg); - g_free(ivgen); return NULL; } if (ivgen->driver->init(ivgen, key, nkey, errp) < 0) { - g_free(ivgen); return NULL; } - return ivgen; + return g_steal_pointer(&ivgen); } diff --git a/crypto/secret_keyring.c b/crypto/secret_keyring.c index 78d7f09b3b..3b332276ef 100644 --- a/crypto/secret_keyring.c +++ b/crypto/secret_keyring.c @@ -41,7 +41,7 @@ qcrypto_secret_keyring_load_data(QCryptoSecretCommon *sec_common, Error **errp) { QCryptoSecretKeyring *secret = QCRYPTO_SECRET_KEYRING(sec_common); - uint8_t *buffer = NULL; + g_autofree uint8_t *buffer = NULL; long retcode; *output = NULL; @@ -61,12 +61,11 @@ qcrypto_secret_keyring_load_data(QCryptoSecretCommon *sec_common, retcode = keyctl_read(secret->serial, buffer, retcode); if (retcode < 0) { - g_free(buffer); goto keyctl_error; } *outputlen = retcode; - *output = buffer; + *output = g_steal_pointer(&buffer); return; keyctl_error: -- 2.55.0

From: Marc-André Lureau <marcandre.lureau@redhat.com> Propagate the error when qcrypto_x509_get_pk_algorithm fails, instead of falling through to return 0 (success) with *errp set. Fixes: e8317c4c9f68 ("crypto/x509-utils: Add helper functions for DIAG 320 subcode 2") Reviewed-by: Zhuoying Cai <zycai@linux.ibm.com> Reviewed-by: Daniel P. Berrangé <berrange@redhat.com> Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com> Signed-off-by: Daniel P. Berrangé <berrange@redhat.com> --- crypto/x509-utils.c | 3 +++ 1 file changed, 3 insertions(+) diff --git a/crypto/x509-utils.c b/crypto/x509-utils.c index 34cbfca26b..edcc44de80 100644 --- a/crypto/x509-utils.c +++ b/crypto/x509-utils.c @@ -319,6 +319,9 @@ int qcrypto_x509_check_ecc_curve_p521(uint8_t *cert, size_t size, Error **errp) int curve_id; algo = qcrypto_x509_get_pk_algorithm(cert, size, errp); + if (algo < 0) { + return -1; + } if (algo != GNUTLS_PK_ECDSA) { return 0; } -- 2.55.0

From: Denis V. Lunev <den@openvz.org> qio_channel_websock_extract_headers() returns 0 without queueing a response when the request line contains no space, unlike every sibling check which jumps to bad_request. encoutput stays empty, yet qio_channel_websock_handshake_read() still reports success and the caller arms a G_IO_OUT watch to flush nothing. Flushing that empty buffer is where QEMU crashes. Any client can trigger it before authentication on a VNC websocket port: printf 'stats\r\nx\r\n\r\n' | nc $host $port Fixes: 07e95cd529af ("io: fully parse & validate HTTP headers for websocket protocol handshake") Fixes: f69a8bde2935 ("io: send proper HTTP response for websocket errors") Fixes: CVE-2026-84788 Cc: qemu-stable@nongnu.org Cc: Daniel P. Berrangé <berrange@redhat.com> Cc: Marc-André Lureau <marcandre.lureau@redhat.com> Reviewed-by: Daniel P. Berrangé <berrange@redhat.com> Reviewed-by: Marc-André Lureau <marcandre.lureau@redhat.com> Signed-off-by: Denis V. Lunev <den@openvz.org> Signed-off-by: Daniel P. Berrangé <berrange@redhat.com> --- io/channel-websock.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/io/channel-websock.c b/io/channel-websock.c index 1929abf56a..66c91ed2a2 100644 --- a/io/channel-websock.c +++ b/io/channel-websock.c @@ -230,7 +230,7 @@ qio_channel_websock_extract_headers(QIOChannelWebsock *ioc, tmp = strchr(buffer, ' '); if (!tmp) { error_setg(errp, "Missing HTTP path delimiter"); - return 0; + goto bad_request; } *tmp = '\0'; -- 2.55.0

From: Denis V. Lunev <den@openvz.org> Check that malformed HTTP greetings are answered with an HTTP 400 rather than an empty response. The no-space case is the one which used to leave the response buffer empty. Fixes: CVE-2026-84788 Cc: Daniel P. Berrangé <berrange@redhat.com> Cc: Marc-André Lureau <marcandre.lureau@redhat.com> Reviewed-by: Daniel P. Berrangé <berrange@redhat.com> Reviewed-by: Marc-André Lureau <marcandre.lureau@redhat.com> Signed-off-by: Denis V. Lunev <den@openvz.org> Signed-off-by: Daniel P. Berrangé <berrange@redhat.com> --- tests/unit/meson.build | 1 + tests/unit/test-io-channel-websock.c | 105 +++++++++++++++++++++++++++ 2 files changed, 106 insertions(+) create mode 100644 tests/unit/test-io-channel-websock.c diff --git a/tests/unit/meson.build b/tests/unit/meson.build index 3a9866c1f2..6a11f07112 100644 --- a/tests/unit/meson.build +++ b/tests/unit/meson.build @@ -93,6 +93,7 @@ if have_block 'test-io-channel-command': ['io-channel-helpers.c', io], 'test-io-channel-buffer': ['io-channel-helpers.c', io], 'test-io-channel-null': [io], + 'test-io-channel-websock': [io], 'test-crypto-ivgen': [io], 'test-crypto-afsplit': [io], 'test-crypto-block': [io], diff --git a/tests/unit/test-io-channel-websock.c b/tests/unit/test-io-channel-websock.c new file mode 100644 index 0000000000..2a55a4bcdf --- /dev/null +++ b/tests/unit/test-io-channel-websock.c @@ -0,0 +1,105 @@ +/* + * SPDX-License-Identifier: GPL-2.0-or-later + * + * QEMU I/O channel websock test + * + * Copyright (c) 2026 Virtuozzo International GmbH + */ + +#include "qemu/osdep.h" +#include "io/channel-websock.h" +#include "io/channel-socket.h" +#include "qapi/error.h" +#include "qemu/module.h" +#include "qemu/sockets.h" + +typedef struct { + bool finished; + bool failed; +} QIOChannelWebsockHandshake; + +static void test_websock_handshake_done(QIOTask *task, gpointer opaque) +{ + QIOChannelWebsockHandshake *res = opaque; + + res->finished = true; + res->failed = qio_task_propagate_error(task, NULL); +} + +/* + * Drives a server-side handshake against @request and returns whatever + * the server wrote back, NUL terminated. The handshake is expected to + * fail; the point of the test is the HTTP response that goes with it. + */ +static char *test_websock_handshake_reply(const char *request) +{ + QIOChannelWebsockHandshake res = { false, false }; + QIOChannelSocket *cli, *srv; + QIOChannelWebsock *wioc; + GMainContext *mainloop; + int channel[2]; + char *reply; + ssize_t got; + + g_assert(qemu_socketpair(AF_UNIX, SOCK_STREAM, 0, channel) == 0); + + cli = qio_channel_socket_new_fd(channel[0], &error_abort); + srv = qio_channel_socket_new_fd(channel[1], &error_abort); + qio_channel_set_blocking(QIO_CHANNEL(srv), false, &error_abort); + qio_channel_set_blocking(QIO_CHANNEL(cli), false, &error_abort); + + wioc = qio_channel_websock_new_server(QIO_CHANNEL(srv)); + qio_channel_websock_handshake(wioc, test_websock_handshake_done, + &res, NULL); + + qio_channel_write_all(QIO_CHANNEL(cli), request, strlen(request), + &error_abort); + + mainloop = g_main_context_default(); + while (!res.finished) { + g_main_context_iteration(mainloop, TRUE); + } + g_assert(res.failed); + + reply = g_malloc0(1024); + got = qio_channel_read(QIO_CHANNEL(cli), reply, 1023, &error_abort); + if (got > 0) { + reply[got] = '\0'; + } + + object_unref(OBJECT(wioc)); + object_unref(OBJECT(srv)); + object_unref(OBJECT(cli)); + + return reply; +} + +static void test_websock_bad_request(const void *opaque) +{ + const char *request = opaque; + g_autofree char *reply = test_websock_handshake_reply(request); + + g_assert_true(g_str_has_prefix(reply, "HTTP/1.1 400 Bad Request\r\n")); +} + +int main(int argc, char **argv) +{ + module_call_init(MODULE_INIT_QOM); + g_test_init(&argc, &argv, NULL); + +#define TEST_BAD_REQUEST(name, request) \ + g_test_add_data_func("/io/channel/websock/bad-request/" name, \ + request, test_websock_bad_request) + + /* + * A greeting with no space at all used to leave the response buffer + * empty, which drove the handshake into a zero length write. + */ + TEST_BAD_REQUEST("no-space", "stats\r\nx\r\n\r\n"); + TEST_BAD_REQUEST("method-only", "GET\r\nx\r\n\r\n"); + TEST_BAD_REQUEST("no-version", "GET /\r\nx\r\n\r\n"); + TEST_BAD_REQUEST("bad-method", "POST / HTTP/1.1\r\nx: y\r\n\r\n"); + TEST_BAD_REQUEST("bad-version", "GET / HTTP/1.0\r\nx: y\r\n\r\n"); + + return g_test_run(); +} -- 2.55.0

From: Denis V. Lunev <den@openvz.org> Add a channel which reports QIO_CHANNEL_ERR_BLOCK on demand, the way a TLS channel does when a record arrives split across segments or when the socket cannot take the whole reply at once, and drive the server handshake through it in both directions. Without the fixes each direction dereferences a NULL Error and the test dies on SIGSEGV. Cc: Daniel P. Berrangé <berrange@redhat.com> Cc: Marc-André Lureau <marcandre.lureau@redhat.com> Reviewed-by: Daniel P. Berrangé <berrange@redhat.com> Reviewed-by: Marc-André Lureau <marcandre.lureau@redhat.com> Signed-off-by: Denis V. Lunev <den@openvz.org> Signed-off-by: Daniel P. Berrangé <berrange@redhat.com> --- tests/unit/test-io-channel-websock.c | 150 ++++++++++++++++++++++++++- 1 file changed, 147 insertions(+), 3 deletions(-) diff --git a/tests/unit/test-io-channel-websock.c b/tests/unit/test-io-channel-websock.c index 2a55a4bcdf..88da24f993 100644 --- a/tests/unit/test-io-channel-websock.c +++ b/tests/unit/test-io-channel-websock.c @@ -12,6 +12,123 @@ #include "qapi/error.h" #include "qemu/module.h" #include "qemu/sockets.h" +#include "qom/object.h" + +#define TYPE_QIO_CHANNEL_STALL "qio-channel-stall" +OBJECT_DECLARE_SIMPLE_TYPE(QIOChannelStall, QIO_CHANNEL_STALL) + +/* + * Reports QIO_CHANNEL_ERR_BLOCK for the first @rstalls reads and @wstalls + * writes, the way a TLS channel does when a record arrives split across TCP + * segments or the socket cannot take the whole reply at once. + */ +struct QIOChannelStall { + QIOChannel parent; + QIOChannel *master; + unsigned rstalls; + unsigned wstalls; +}; + +static ssize_t qio_channel_stall_readv(QIOChannel *ioc, + const struct iovec *iov, + size_t niov, + int **fds, + size_t *nfds, + int flags, + Error **errp) +{ + QIOChannelStall *sioc = QIO_CHANNEL_STALL(ioc); + + if (sioc->rstalls) { + sioc->rstalls--; + return QIO_CHANNEL_ERR_BLOCK; + } + return qio_channel_readv_full(sioc->master, iov, niov, fds, nfds, + flags, errp); +} + +static ssize_t qio_channel_stall_writev(QIOChannel *ioc, + const struct iovec *iov, + size_t niov, + int *fds, + size_t nfds, + int flags, + Error **errp) +{ + QIOChannelStall *sioc = QIO_CHANNEL_STALL(ioc); + + if (sioc->wstalls) { + sioc->wstalls--; + return QIO_CHANNEL_ERR_BLOCK; + } + return qio_channel_writev_full(sioc->master, iov, niov, fds, nfds, + flags, errp); +} + +static int qio_channel_stall_set_blocking(QIOChannel *ioc, bool enabled, + Error **errp) +{ + QIOChannelStall *sioc = QIO_CHANNEL_STALL(ioc); + + return qio_channel_set_blocking(sioc->master, enabled, errp) ? 0 : -1; +} + +static int qio_channel_stall_close(QIOChannel *ioc, Error **errp) +{ + QIOChannelStall *sioc = QIO_CHANNEL_STALL(ioc); + + return qio_channel_close(sioc->master, errp); +} + +static GSource *qio_channel_stall_create_watch(QIOChannel *ioc, + GIOCondition condition) +{ + QIOChannelStall *sioc = QIO_CHANNEL_STALL(ioc); + + return qio_channel_create_watch(sioc->master, condition); +} + +static void qio_channel_stall_finalize(Object *obj) +{ + QIOChannelStall *sioc = QIO_CHANNEL_STALL(obj); + + object_unref(OBJECT(sioc->master)); +} + +static void qio_channel_stall_class_init(ObjectClass *klass, + const void *class_data G_GNUC_UNUSED) +{ + QIOChannelClass *ioc_klass = QIO_CHANNEL_CLASS(klass); + + ioc_klass->io_writev = qio_channel_stall_writev; + ioc_klass->io_readv = qio_channel_stall_readv; + ioc_klass->io_set_blocking = qio_channel_stall_set_blocking; + ioc_klass->io_close = qio_channel_stall_close; + ioc_klass->io_create_watch = qio_channel_stall_create_watch; +} + +static const TypeInfo qio_channel_stall_info = { + .parent = TYPE_QIO_CHANNEL, + .name = TYPE_QIO_CHANNEL_STALL, + .instance_size = sizeof(QIOChannelStall), + .instance_finalize = qio_channel_stall_finalize, + .class_init = qio_channel_stall_class_init, +}; + +static QIOChannelStall *qio_channel_stall_new(QIOChannel *master, + unsigned rstalls, + unsigned wstalls) +{ + QIOChannelStall *sioc = QIO_CHANNEL_STALL( + object_new(TYPE_QIO_CHANNEL_STALL)); + + object_ref(OBJECT(master)); + sioc->master = master; + sioc->rstalls = rstalls; + sioc->wstalls = wstalls; + + return sioc; +} typedef struct { bool finished; @@ -31,10 +148,12 @@ static void test_websock_handshake_done(QIOTask *task, gpointer opaque) * the server wrote back, NUL terminated. The handshake is expected to * fail; the point of the test is the HTTP response that goes with it. */ -static char *test_websock_handshake_reply(const char *request) +static char *test_websock_handshake_reply(const char *request, + unsigned rstalls, unsigned wstalls) { QIOChannelWebsockHandshake res = { false, false }; QIOChannelSocket *cli, *srv; + QIOChannelStall *stall; QIOChannelWebsock *wioc; GMainContext *mainloop; int channel[2]; @@ -48,7 +167,8 @@ static char *test_websock_handshake_reply(const char *request) qio_channel_set_blocking(QIO_CHANNEL(srv), false, &error_abort); qio_channel_set_blocking(QIO_CHANNEL(cli), false, &error_abort); - wioc = qio_channel_websock_new_server(QIO_CHANNEL(srv)); + stall = qio_channel_stall_new(QIO_CHANNEL(srv), rstalls, wstalls); + wioc = qio_channel_websock_new_server(QIO_CHANNEL(stall)); qio_channel_websock_handshake(wioc, test_websock_handshake_done, &res, NULL); @@ -68,6 +188,7 @@ static char *test_websock_handshake_reply(const char *request) } object_unref(OBJECT(wioc)); + object_unref(OBJECT(stall)); object_unref(OBJECT(srv)); object_unref(OBJECT(cli)); @@ -77,7 +198,23 @@ static char *test_websock_handshake_reply(const char *request) static void test_websock_bad_request(const void *opaque) { const char *request = opaque; - g_autofree char *reply = test_websock_handshake_reply(request); + g_autofree char *reply = test_websock_handshake_reply(request, 0, 0); + + g_assert_true(g_str_has_prefix(reply, "HTTP/1.1 400 Bad Request\r\n")); +} + +static void test_websock_stalled_read(const void *opaque) +{ + const char *request = opaque; + g_autofree char *reply = test_websock_handshake_reply(request, 1, 0); + + g_assert_true(g_str_has_prefix(reply, "HTTP/1.1 400 Bad Request\r\n")); +} + +static void test_websock_stalled_write(const void *opaque) +{ + const char *request = opaque; + g_autofree char *reply = test_websock_handshake_reply(request, 0, 1); g_assert_true(g_str_has_prefix(reply, "HTTP/1.1 400 Bad Request\r\n")); } @@ -85,6 +222,7 @@ static void test_websock_bad_request(const void *opaque) int main(int argc, char **argv) { module_call_init(MODULE_INIT_QOM); + type_register_static(&qio_channel_stall_info); g_test_init(&argc, &argv, NULL); #define TEST_BAD_REQUEST(name, request) \ @@ -101,5 +239,11 @@ int main(int argc, char **argv) TEST_BAD_REQUEST("bad-method", "POST / HTTP/1.1\r\nx: y\r\n\r\n"); TEST_BAD_REQUEST("bad-version", "GET / HTTP/1.0\r\nx: y\r\n\r\n"); + /* A read which blocks before any header arrives is not a fatal error. */ + g_test_add_data_func("/io/channel/websock/stalled-read", + "stats\r\nx\r\n\r\n", test_websock_stalled_read); + g_test_add_data_func("/io/channel/websock/stalled-write", + "stats\r\nx\r\n\r\n", test_websock_stalled_write); + return g_test_run(); } -- 2.55.0

The default checkpatch.pl output includes a lot of "progress" information... NN/MM Checking commit HASH (SUBJECT) total: 0 errors, 0 warnings, 105 lines checked for large patch series, this results in alot of noise from clean patches (which are the common case), obscuring the info about the patch violations. Since the alerts from gitlab job failures only include the last few lines of log output, we want the checkpatch.pl output to be highly relevant to the failure. We previously addressed that by adding use of the --terse flag in 7025111a199b97ae806817788bec50f456c47d85, but that made the output a bit too terse. It no longer prints the offending line of code, so understanding the CI job failure now requires manually re-running checkpatch.pl locally. Using the '--quiet' flag gets rid of the "Check commit HASH.." messages which create noise, while adding '--emacs' causes the WARNING/ERROR message lines to include the commit hash. This is a more useful tradeoff for the CI job logs. The output is limited to only patches which include code violations, while still including the offending lines of code in the output. Reviewed-by: Alex Bennée <alex.bennee@linaro.org> Signed-off-by: Daniel P. Berrangé <berrange@redhat.com> --- .gitlab-ci.d/check-patch.py | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/.gitlab-ci.d/check-patch.py b/.gitlab-ci.d/check-patch.py index be13e6f77d..45be77295d 100755 --- a/.gitlab-ci.d/check-patch.py +++ b/.gitlab-ci.d/check-patch.py @@ -46,7 +46,11 @@ print("\nChecking all commits since %s...\n" % ancestor, flush=True) -ret = subprocess.run(["scripts/checkpatch.pl", "--terse", ancestor + "..."]) +# We don't want "noise" for clean patches, but do want to see +# the full commit hash for each violation, along with the +# offending patch content +ret = subprocess.run(["scripts/checkpatch.pl", "--emacs", "--quiet", + ancestor + "..."]) if ret.returncode != 0: print(" ❌ FAIL one or more commits failed scripts/checkpatch.pl") -- 2.55.0

The -ftrivial-auto-var-init=zero usage guarantees implicit zero initializers for all stack variables. Thus most bug reports relying on undefined behaviour from lack of variable initialization will not be security issues, or even bugs. Reviewed-by: Marc-André Lureau <marcandre.lureau@redhat.com> Signed-off-by: Daniel P. Berrangé <berrange@redhat.com> --- docs/system/security.rst | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/docs/system/security.rst b/docs/system/security.rst index af626a4230..8c42d1a6d8 100644 --- a/docs/system/security.rst +++ b/docs/system/security.rst @@ -143,6 +143,16 @@ an issue as a normal bug. which case plain manipulation of the stream is not considered as an attack vector. +* **uninitialized stack variables**. If the bug scenario relies on + undefined behaviour from stack variables that lack explicit + initialization, it will not usually be considered a security flaw. + The build system adds '-ftrivial-auto-var-init=zero', which is + available in both the supported compilers (GCC and CLang) and + ensures all stack variables have implicit zero-initializers. + This eliminates undefined behaviour and usually gives the + correct desired initialization value, eliminating most of the + bug scenarios wrt uninitialized stack variables. + * **low severity impact**. As a catch all rule, issues which are judged to have a "low" severity impact on the system will usually not justify handling as security bugs, nor assignment -- 2.55.0