Message ID | 1448965656-27220-1-git-send-email-pbonzini@redhat.com |
---|---|
State | New |
Headers | show |
Paolo Bonzini <pbonzini@redhat.com> writes: > This is a case where pty_chr_update_read_handler_locked's lack > of error checking can produce incorrect values. We are not using > SIGUSR1 anymore, so this is quite theoretical, but easy to fix. > > Reported-by: Markus Armbruster <armbru@redhat.com> > Signed-off-by: Paolo Bonzini <pbonzini@redhat.com> > --- > qemu-char.c | 7 ++++++- > 1 file changed, 6 insertions(+), 1 deletion(-) > > diff --git a/qemu-char.c b/qemu-char.c > index 5448b0f..2969c44 100644 > --- a/qemu-char.c > +++ b/qemu-char.c > @@ -1241,11 +1241,16 @@ static void pty_chr_update_read_handler_locked(CharDriverState *chr) > { > PtyCharDriver *s = chr->opaque; > GPollFD pfd; > + int rc; > > pfd.fd = g_io_channel_unix_get_fd(s->fd); > pfd.events = G_IO_OUT; > pfd.revents = 0; > - g_poll(&pfd, 1, 0); > + do { > + rc = g_poll(&pfd, 1, 0); > + } while (rc == -1 && errno == EINTR); Could use TFR(), but that's a matter of taste. Both ways aleady exist in this file. > + assert(rc >= 0); Documented errors other than EINTR: EFAULT The array given as argument was not contained in the calling program's address space. EINVAL The nfds value exceeds the RLIMIT_NOFILE value. ENOMEM There was no space to allocate file descriptor tables. Aborting feels okay to me. > + > if (pfd.revents & G_IO_HUP) { > pty_chr_state(chr, 0); > } else { Reviewed-by: Markus Armbruster <armbru@redhat.com>
diff --git a/qemu-char.c b/qemu-char.c index 5448b0f..2969c44 100644 --- a/qemu-char.c +++ b/qemu-char.c @@ -1241,11 +1241,16 @@ static void pty_chr_update_read_handler_locked(CharDriverState *chr) { PtyCharDriver *s = chr->opaque; GPollFD pfd; + int rc; pfd.fd = g_io_channel_unix_get_fd(s->fd); pfd.events = G_IO_OUT; pfd.revents = 0; - g_poll(&pfd, 1, 0); + do { + rc = g_poll(&pfd, 1, 0); + } while (rc == -1 && errno == EINTR); + assert(rc >= 0); + if (pfd.revents & G_IO_HUP) { pty_chr_state(chr, 0); } else {
This is a case where pty_chr_update_read_handler_locked's lack of error checking can produce incorrect values. We are not using SIGUSR1 anymore, so this is quite theoretical, but easy to fix. Reported-by: Markus Armbruster <armbru@redhat.com> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com> --- qemu-char.c | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-)