* [PATCH] chardev/char-socket: Properly make qio connections non blocking @ 2020-04-17 12:30 Sai Pavan Boddu 2020-04-17 13:01 ` Marc-André Lureau 0 siblings, 1 reply; 4+ messages in thread From: Sai Pavan Boddu @ 2020-04-17 12:30 UTC (permalink / raw) To: Marc-André Lureau, Paolo Bonzini; +Cc: edgari, qemu-devel In tcp_chr_sync_read function, there is a possibility of socket disconnection during read, then tcp_chr_hup function would clean up the qio channel pointers(i.e ioc, sioc). Signed-off-by: Sai Pavan Boddu <sai.pavan.boddu@xilinx.com> --- chardev/char-socket.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/chardev/char-socket.c b/chardev/char-socket.c index 185fe38..30f2b2b 100644 --- a/chardev/char-socket.c +++ b/chardev/char-socket.c @@ -549,11 +549,13 @@ static int tcp_chr_sync_read(Chardev *chr, const uint8_t *buf, int len) qio_channel_set_blocking(s->ioc, true, NULL); size = tcp_chr_recv(chr, (void *) buf, len); - qio_channel_set_blocking(s->ioc, false, NULL); if (size == 0) { /* connection closed */ tcp_chr_disconnect(chr); + return 0; } + /* Connection is good */ + qio_channel_set_blocking(s->ioc, false, NULL); return size; } -- 2.7.4 ^ permalink raw reply related [flat|nested] 4+ messages in thread
* Re: [PATCH] chardev/char-socket: Properly make qio connections non blocking 2020-04-17 12:30 [PATCH] chardev/char-socket: Properly make qio connections non blocking Sai Pavan Boddu @ 2020-04-17 13:01 ` Marc-André Lureau 2020-04-17 13:13 ` Daniel P. Berrangé 0 siblings, 1 reply; 4+ messages in thread From: Marc-André Lureau @ 2020-04-17 13:01 UTC (permalink / raw) To: Sai Pavan Boddu, Daniel P. Berrange; +Cc: Paolo Bonzini, edgari, QEMU Hi On Fri, Apr 17, 2020 at 2:38 PM Sai Pavan Boddu <sai.pavan.boddu@xilinx.com> wrote: > > In tcp_chr_sync_read function, there is a possibility of socket > disconnection during read, then tcp_chr_hup function would clean up > the qio channel pointers(i.e ioc, sioc). > > Signed-off-by: Sai Pavan Boddu <sai.pavan.boddu@xilinx.com> > --- > chardev/char-socket.c | 4 +++- > 1 file changed, 3 insertions(+), 1 deletion(-) > > diff --git a/chardev/char-socket.c b/chardev/char-socket.c > index 185fe38..30f2b2b 100644 > --- a/chardev/char-socket.c > +++ b/chardev/char-socket.c > @@ -549,11 +549,13 @@ static int tcp_chr_sync_read(Chardev *chr, const uint8_t *buf, int len) > > qio_channel_set_blocking(s->ioc, true, NULL); > size = tcp_chr_recv(chr, (void *) buf, len); > - qio_channel_set_blocking(s->ioc, false, NULL); But here it calls tcp_chr_recv(). And I can't find cleanup there. Nevertheless, I think this patch should be harmless. I'd ask Daniel to have a second look. > if (size == 0) { > /* connection closed */ > tcp_chr_disconnect(chr); > + return 0; > } > + /* Connection is good */ > + qio_channel_set_blocking(s->ioc, false, NULL); > > return size; > } > -- > 2.7.4 > > -- Marc-André Lureau ^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH] chardev/char-socket: Properly make qio connections non blocking 2020-04-17 13:01 ` Marc-André Lureau @ 2020-04-17 13:13 ` Daniel P. Berrangé 2020-04-17 13:37 ` Sai Pavan Boddu 0 siblings, 1 reply; 4+ messages in thread From: Daniel P. Berrangé @ 2020-04-17 13:13 UTC (permalink / raw) To: Marc-André Lureau; +Cc: Sai Pavan Boddu, Paolo Bonzini, edgari, QEMU On Fri, Apr 17, 2020 at 03:01:09PM +0200, Marc-André Lureau wrote: > Hi > > On Fri, Apr 17, 2020 at 2:38 PM Sai Pavan Boddu > <sai.pavan.boddu@xilinx.com> wrote: > > > > In tcp_chr_sync_read function, there is a possibility of socket > > disconnection during read, then tcp_chr_hup function would clean up > > the qio channel pointers(i.e ioc, sioc). > > > > Signed-off-by: Sai Pavan Boddu <sai.pavan.boddu@xilinx.com> > > --- > > chardev/char-socket.c | 4 +++- > > 1 file changed, 3 insertions(+), 1 deletion(-) > > > > diff --git a/chardev/char-socket.c b/chardev/char-socket.c > > index 185fe38..30f2b2b 100644 > > --- a/chardev/char-socket.c > > +++ b/chardev/char-socket.c > > @@ -549,11 +549,13 @@ static int tcp_chr_sync_read(Chardev *chr, const uint8_t *buf, int len) > > > > qio_channel_set_blocking(s->ioc, true, NULL); > > size = tcp_chr_recv(chr, (void *) buf, len); > > - qio_channel_set_blocking(s->ioc, false, NULL); > > But here it calls tcp_chr_recv(). And I can't find cleanup there. > Nevertheless, I think this patch should be harmless. > > I'd ask Daniel to have a second look. I don't see any bug that needs fixing here, and I prefer the current code as it gives confidence that nothing tcp_chr_disconnect does will accidentally block. > > if (size == 0) { > > /* connection closed */ > > tcp_chr_disconnect(chr); > > + return 0; > > } > > + /* Connection is good */ > > + qio_channel_set_blocking(s->ioc, false, NULL); > > > > return size; > > } > > -- > > 2.7.4 > > > > > > > -- > Marc-André Lureau > 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 :| ^ permalink raw reply [flat|nested] 4+ messages in thread
* RE: [PATCH] chardev/char-socket: Properly make qio connections non blocking 2020-04-17 13:13 ` Daniel P. Berrangé @ 2020-04-17 13:37 ` Sai Pavan Boddu 0 siblings, 0 replies; 4+ messages in thread From: Sai Pavan Boddu @ 2020-04-17 13:37 UTC (permalink / raw) To: Daniel P. Berrangé, Marc-André Lureau Cc: Paolo Bonzini, Edgar Iglesias, QEMU Hi Daniel, > -----Original Message----- > From: Daniel P. Berrangé <berrange@redhat.com> > Sent: Friday, April 17, 2020 6:44 PM > To: Marc-André Lureau <marcandre.lureau@gmail.com> > Cc: Sai Pavan Boddu <saipava@xilinx.com>; Paolo Bonzini > <pbonzini@redhat.com>; Edgar Iglesias <edgari@xilinx.com>; QEMU <qemu- > devel@nongnu.org> > Subject: Re: [PATCH] chardev/char-socket: Properly make qio connections > non blocking > > On Fri, Apr 17, 2020 at 03:01:09PM +0200, Marc-André Lureau wrote: > > Hi > > > > On Fri, Apr 17, 2020 at 2:38 PM Sai Pavan Boddu > > <sai.pavan.boddu@xilinx.com> wrote: > > > > > > In tcp_chr_sync_read function, there is a possibility of socket > > > disconnection during read, then tcp_chr_hup function would clean up > > > the qio channel pointers(i.e ioc, sioc). > > > > > > Signed-off-by: Sai Pavan Boddu <sai.pavan.boddu@xilinx.com> > > > --- > > > chardev/char-socket.c | 4 +++- > > > 1 file changed, 3 insertions(+), 1 deletion(-) > > > > > > diff --git a/chardev/char-socket.c b/chardev/char-socket.c index > > > 185fe38..30f2b2b 100644 > > > --- a/chardev/char-socket.c > > > +++ b/chardev/char-socket.c > > > @@ -549,11 +549,13 @@ static int tcp_chr_sync_read(Chardev *chr, > > > const uint8_t *buf, int len) > > > > > > qio_channel_set_blocking(s->ioc, true, NULL); > > > size = tcp_chr_recv(chr, (void *) buf, len); > > > - qio_channel_set_blocking(s->ioc, false, NULL); > > > > But here it calls tcp_chr_recv(). And I can't find cleanup there. > > Nevertheless, I think this patch should be harmless. > > > > I'd ask Daniel to have a second look. > > I don't see any bug that needs fixing here, and I prefer the current code as it > gives confidence that nothing tcp_chr_disconnect does will accidentally > block. [Sai Pavan Boddu] The issue is triggered when 'tcp_chr_hup' callback, is dispatched when socket hung up during a blocking read. Which also calls tcp_chr_disconnect and cleans up ioc, sioc pointers. Later below line segfaults due to null pointers " qio_channel_set_blocking(s->ioc, false, NULL); " Regards, Sai Pavan > > > > > if (size == 0) { > > > /* connection closed */ > > > tcp_chr_disconnect(chr); > > > + return 0; > > > } > > > + /* Connection is good */ > > > + qio_channel_set_blocking(s->ioc, false, NULL); > > > > > > return size; > > > } > > > -- > > > 2.7.4 > > > > > > > > > > > > -- > > Marc-André Lureau > > > > 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 > :| ^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2020-04-17 13:38 UTC | newest] Thread overview: 4+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2020-04-17 12:30 [PATCH] chardev/char-socket: Properly make qio connections non blocking Sai Pavan Boddu 2020-04-17 13:01 ` Marc-André Lureau 2020-04-17 13:13 ` Daniel P. Berrangé 2020-04-17 13:37 ` Sai Pavan Boddu
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.