From: "Daniel P. Berrangé" <berrange@redhat.com>
To: "Marc-André Lureau" <marcandre.lureau@redhat.com>
Cc: qemu-devel@nongnu.org, Paolo Bonzini <pbonzini@redhat.com>,
Thomas Huth <thuth@redhat.com>
Subject: Re: [PATCH 2/3 for 9.0] Revert "chardev/char-socket: Fix TLS io channels sending too much data to the backend"
Date: Tue, 19 Mar 2024 13:14:25 +0000 [thread overview]
Message-ID: <ZfmPse9su6yGdfm9@redhat.com> (raw)
In-Reply-To: <CAMxuvaz6_pRAaNX8zD-EGdTvhPhGcTEs8n+=tZOAYNLVAAc8hw@mail.gmail.com>
On Mon, Mar 18, 2024 at 11:09:23PM +0400, Marc-André Lureau wrote:
> Hi
>
> On Mon, Mar 18, 2024 at 10:23 PM Daniel P. Berrangé <berrange@redhat.com> wrote:
> >
> > This commit results in unexpected termination of the TLS connection.
> > When 'fd_can_read' returns 0, the code goes on to pass a zero length
> > buffer to qio_channel_read. The TLS impl calls into gnutls_recv()
> > with this zero length buffer, at which point GNUTLS returns an error
> > GNUTLS_E_INVALID_REQUEST. This is treated as fatal by QEMU's TLS code
> > resulting in the connection being torn down by the chardev.
> >
> > Simply skipping the qio_channel_read when the buffer length is zero
> > is also not satisfactory, as it results in a high CPU burn busy loop
> > massively slowing QEMU's functionality.
> >
> > The proper solution is to avoid tcp_chr_read being called at all
> > unless the frontend is able to accept more data. This will be done
> > in a followup commit.
> >
> > This reverts commit 1907f4d149c3589ade641423c6a33fd7598fa4d3.
>
> Actually 462945cd22d2bcd233401ed3aa167d83a8e35b05 upstream.
Opps, yes, will fix this before I send a pull.
>
> >
> > Signed-off-by: Daniel P. Berrangé <berrange@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 2c4dffc0e6..812d7aa38a 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)) {
> > --
> > 2.43.0
> >
>
With regards,
Daniel
--
|: https://berrange.com -o- https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org -o- https://fstop138.berrange.com :|
|: https://entangle-photo.org -o- https://www.instagram.com/dberrange :|
next prev parent reply other threads:[~2024-03-19 13:15 UTC|newest]
Thread overview: 13+ messages / expand[flat|nested] mbox.gz Atom feed top
2024-03-18 18:23 [PATCH 0/3 for 9.0] Fix TLS support for chardevs and incoming data loss on EOF Daniel P. Berrangé
2024-03-18 18:23 ` [PATCH 1/3 for 9.0] chardev: lower priority of the HUP GSource in socket chardev Daniel P. Berrangé
2024-03-18 19:17 ` Marc-André Lureau
2024-03-19 8:10 ` Thomas Huth
2024-03-18 18:23 ` [PATCH 2/3 for 9.0] Revert "chardev/char-socket: Fix TLS io channels sending too much data to the backend" Daniel P. Berrangé
2024-03-18 19:09 ` Marc-André Lureau
2024-03-19 13:14 ` Daniel P. Berrangé [this message]
2024-03-19 8:09 ` Thomas Huth
2024-03-18 18:23 ` [PATCH 3/3 for 9.0] Revert "chardev: use a child source for qio input source" Daniel P. Berrangé
2024-03-18 20:20 ` Marc-André Lureau
2024-03-19 11:07 ` Daniel P. Berrangé
2024-03-19 8:29 ` Thomas Huth
2024-03-19 9:32 ` [PATCH 0/3 for 9.0] Fix TLS support for chardevs and incoming data loss on EOF Thomas Huth
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=ZfmPse9su6yGdfm9@redhat.com \
--to=berrange@redhat.com \
--cc=marcandre.lureau@redhat.com \
--cc=pbonzini@redhat.com \
--cc=qemu-devel@nongnu.org \
--cc=thuth@redhat.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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.