[Top][All Lists]
[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH 1/2] qemu-char: BUGFIX, don't call FD_ISSET with
From: |
Anthony Liguori |
Subject: |
Re: [Qemu-devel] [PATCH 1/2] qemu-char: BUGFIX, don't call FD_ISSET with negative fd |
Date: |
Mon, 17 Sep 2012 13:24:51 -0500 |
User-agent: |
Notmuch/0.13.2+93~ged93d79 (http://notmuchmail.org) Emacs/23.3.1 (x86_64-pc-linux-gnu) |
David Gibson <address@hidden> writes:
> tcp_chr_connect(), unlike for example udp_chr_update_read_handler() does
> not check if the fd it is using is valid (>= 0) before passing it to
> qemu_set_fd_handler2(). If using e.g. a TCP serial port, which is not
> initially connected, this can result in -1 being passed to FD_ISSET, which
> has undefined behaviour. On x86 it seems to harmlessly return 0, but on
> PowerPC, it causes a fortify buffer overflow error to be thrown.
>
> This patch fixes this by putting an extra test in tcp_chr_connect(), and
> also adds an assert qemu_set_fd_handler2() to catch other such errors on
> all platforms, rather than just some.
>
> Signed-off-by: David Gibson <address@hidden>
Applied. Thanks.
Regards,
Anthony Liguori
> ---
> iohandler.c | 2 ++
> qemu-char.c | 6 ++++--
> 2 files changed, 6 insertions(+), 2 deletions(-)
>
> diff --git a/iohandler.c b/iohandler.c
> index dea4355..a2d871b 100644
> --- a/iohandler.c
> +++ b/iohandler.c
> @@ -56,6 +56,8 @@ int qemu_set_fd_handler2(int fd,
> {
> IOHandlerRecord *ioh;
>
> + assert(fd >= 0);
> +
> if (!fd_read && !fd_write) {
> QLIST_FOREACH(ioh, &io_handlers, next) {
> if (ioh->fd == fd) {
> diff --git a/qemu-char.c b/qemu-char.c
> index 398baf1..73e48ff 100644
> --- a/qemu-char.c
> +++ b/qemu-char.c
> @@ -2332,8 +2332,10 @@ static void tcp_chr_connect(void *opaque)
> TCPCharDriver *s = chr->opaque;
>
> s->connected = 1;
> - qemu_set_fd_handler2(s->fd, tcp_chr_read_poll,
> - tcp_chr_read, NULL, chr);
> + if (s->fd >= 0) {
> + qemu_set_fd_handler2(s->fd, tcp_chr_read_poll,
> + tcp_chr_read, NULL, chr);
> + }
> qemu_chr_generic_open(chr);
> }
>
> --
> 1.7.10.4