You can not select more than 25 topics
Topics must start with a letter or number, can include dashes ('-') and can be up to 35 characters long.
106 lines
4.3 KiB
106 lines
4.3 KiB
8 months ago
|
From 95b2ffc5f01dc4309c2e747ed883d22cd1d26347 Mon Sep 17 00:00:00 2001
|
||
|
|
From: Thomas Huth <thuth@redhat.com>
|
||
|
|
Date: Sat, 2 Mar 2024 17:00:23 +0100
|
||
|
|
Subject: [PATCH 2/2] chardev/char-socket: Fix TLS io channels sending too much
|
||
|
|
data to the backend
|
||
|
|
MIME-Version: 1.0
|
||
|
|
Content-Type: text/plain; charset=UTF-8
|
||
|
|
Content-Transfer-Encoding: 8bit
|
||
|
|
|
||
|
|
RH-Author: Thomas Huth <thuth@redhat.com>
|
||
|
|
RH-MergeRequest: 227: Fix TLS io channels sending too much data to the backend
|
||
|
|
RH-Jira: RHEL-24614
|
||
|
|
RH-Acked-by: Cédric Le Goater <clg@redhat.com>
|
||
|
|
RH-Acked-by: Daniel P. Berrangé <berrange@redhat.com>
|
||
|
|
RH-Commit: [1/1] fce871914e0ce52e16a6edae0e007513f9fec1ae (thuth/qemu-kvm-cs9)
|
||
|
|
|
||
|
|
JIRA: https://issues.redhat.com/browse/RHEL-24614
|
||
|
|
|
||
|
|
commit 462945cd22d2bcd233401ed3aa167d83a8e35b05
|
||
|
|
Author: Thomas Huth <thuth@redhat.com>
|
||
|
|
Date: Thu Feb 29 11:43:37 2024 +0100
|
||
|
|
|
||
|
|
chardev/char-socket: Fix TLS io channels sending too much data to the backend
|
||
|
|
|
||
|
|
Commit ffda5db65a ("io/channel-tls: fix handling of bigger read buffers")
|
||
|
|
changed the behavior of the TLS io channels to schedule a second reading
|
||
|
|
attempt if there is still incoming data pending. This caused a regression
|
||
|
|
with backends like the sclpconsole that check in their read function that
|
||
|
|
the sender does not try to write more bytes to it than the device can
|
||
|
|
currently handle.
|
||
|
|
|
||
|
|
The problem can be reproduced like this:
|
||
|
|
|
||
|
|
1) In one terminal, do this:
|
||
|
|
|
||
|
|
mkdir qemu-pki
|
||
|
|
cd qemu-pki
|
||
|
|
openssl genrsa 2048 > ca-key.pem
|
||
|
|
openssl req -new -x509 -nodes -days 365000 -key ca-key.pem -out ca-cert.pem
|
||
|
|
# enter some dummy value for the cert
|
||
|
|
openssl genrsa 2048 > server-key.pem
|
||
|
|
openssl req -new -x509 -nodes -days 365000 -key server-key.pem \
|
||
|
|
-out server-cert.pem
|
||
|
|
# enter some other dummy values for the cert
|
||
|
|
|
||
|
|
gnutls-serv --echo --x509cafile ca-cert.pem --x509keyfile server-key.pem \
|
||
|
|
--x509certfile server-cert.pem -p 8338
|
||
|
|
|
||
|
|
2) In another terminal, do this:
|
||
|
|
|
||
|
|
wget https://download.fedoraproject.org/pub/fedora-secondary/releases/39/Cloud/s390x/images/Fedora-Cloud-Base-39-1.5.s390x.qcow2
|
||
|
|
|
||
|
|
qemu-system-s390x -nographic -nodefaults \
|
||
|
|
-hda Fedora-Cloud-Base-39-1.5.s390x.qcow2 \
|
||
|
|
-object tls-creds-x509,id=tls0,endpoint=client,verify-peer=false,dir=$PWD/qemu-pki \
|
||
|
|
-chardev socket,id=tls_chardev,host=localhost,port=8338,tls-creds=tls0 \
|
||
|
|
-device sclpconsole,chardev=tls_chardev,id=tls_serial
|
||
|
|
|
||
|
|
QEMU then aborts after a second or two with:
|
||
|
|
|
||
|
|
qemu-system-s390x: ../hw/char/sclpconsole.c:73: chr_read: Assertion
|
||
|
|
`size <= SIZE_BUFFER_VT220 - scon->iov_data_len' failed.
|
||
|
|
Aborted (core dumped)
|
||
|
|
|
||
|
|
It looks like the second read does not trigger the chr_can_read() function
|
||
|
|
to be called before the second read, which should normally always be done
|
||
|
|
before sending bytes to a character device to see how much it can handle,
|
||
|
|
so the s->max_size in tcp_chr_read() still contains the old value from the
|
||
|
|
previous read. Let's make sure that we use the up-to-date value by calling
|
||
|
|
tcp_chr_read_poll() again here.
|
||
|
|
|
||
|
|
Fixes: ffda5db65a ("io/channel-tls: fix handling of bigger read buffers")
|
||
|
|
Buglink: https://issues.redhat.com/browse/RHEL-24614
|
||
|
|
Reviewed-by: "Daniel P. Berrangé" <berrange@redhat.com>
|
||
|
|
Message-ID: <20240229104339.42574-1-thuth@redhat.com>
|
||
|
|
Reviewed-by: Antoine Damhet <antoine.damhet@blade-group.com>
|
||
|
|
Tested-by: Antoine Damhet <antoine.damhet@blade-group.com>
|
||
|
|
Reviewed-by: Marc-André Lureau <marcandre.lureau@redhat.com>
|
||
|
|
Signed-off-by: Thomas Huth <thuth@redhat.com>
|
||
|
|
|
||
|
|
Signed-off-by: Thomas Huth <thuth@redhat.com>
|
||
|
|
---
|
||
|
|
chardev/char-socket.c | 6 +++---
|
||
|
|
1 file changed, 3 insertions(+), 3 deletions(-)
|
||
|
|
|
||
|
|
diff --git a/chardev/char-socket.c b/chardev/char-socket.c
|
||
|
|
index 73947da188..034840593d 100644
|
||
|
|
--- a/chardev/char-socket.c
|
||
|
|
+++ b/chardev/char-socket.c
|
||
|
|
@@ -492,9 +492,9 @@ static gboolean tcp_chr_read(QIOChannel *chan, GIOCondition cond, void *opaque)
|
||
|
|
s->max_size <= 0) {
|
||
|
|
return TRUE;
|
||
|
|
}
|
||
|
|
- len = sizeof(buf);
|
||
|
|
- if (len > s->max_size) {
|
||
|
|
- len = s->max_size;
|
||
|
|
+ len = tcp_chr_read_poll(opaque);
|
||
|
|
+ if (len > sizeof(buf)) {
|
||
|
|
+ len = sizeof(buf);
|
||
|
|
}
|
||
|
|
size = tcp_chr_recv(chr, (void *)buf, len);
|
||
|
|
if (size == 0 || (size == -1 && errno != EAGAIN)) {
|
||
|
|
--
|
||
|
|
2.39.3
|
||
|
|
|