Hi Daniel,
> -----Original Message-----
> From: Daniel P. Berrangé <berra...@redhat.com>
> Sent: Friday, April 17, 2020 6:44 PM
> To: Marc-André Lureau <marcandre.lur...@gmail.com>
> Cc: Sai Pavan Boddu <saip...@xilinx.com>; Paolo Bonzini
> <pbonz...@redhat.com>; Edgar Iglesias <edg...@xilinx.com>; QEMU <qemu-
> de...@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.bo...@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.bo...@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
> :|
