Message ID | 4BC84683.7050003@redhat.com |
---|---|
State | New |
Headers | show |
On Fri, Apr 16, 2010 at 01:14:11PM +0200, Paolo Bonzini wrote: > > >The QEMU code appears to be written to assume that it will recvmsg() a > >complete monitor command in one go + process that, because it closes the > >FD the moment the data from any recvmsg() is dealt with. > > This is buggy anyway. This should fix it too: Yep, this makes it work too, but if a client is evil they could pass a FD to qemu with any other non-getfd command & it'd remain open for ever. Probably not important though. Daniel
On 04/16/2010 03:17 PM, Daniel P. Berrange wrote: > On Fri, Apr 16, 2010 at 01:14:11PM +0200, Paolo Bonzini wrote: >> >>> The QEMU code appears to be written to assume that it will recvmsg() a >>> complete monitor command in one go + process that, because it closes the >>> FD the moment the data from any recvmsg() is dealt with. >> >> This is buggy anyway. This should fix it too: > > Yep, this makes it work too, but if a client is evil they could > pass a FD to qemu with any other non-getfd command& it'd remain > open for ever. Probably not important though. No, it wouldn't: outside the part that I patched there is this: if (s->msgfd != -1) close(s->msgfd); s->msgfd = fd; Only one file descriptor could "leak". Paolo
diff --git a/monitor.c b/monitor.c index 5659991..225a922 100644 --- a/monitor.c +++ b/monitor.c @@ -2408,15 +2408,6 @@ return -1; } - fd = dup(fd); - if (fd == -1) { - if (errno == EMFILE) - qerror_report(QERR_TOO_MANY_FILES); - else - qerror_report(QERR_UNDEFINED_ERROR); - return -1; - } - QLIST_FOREACH(monfd, &mon->fds, next) { if (strcmp(monfd->name, fdname) != 0) { continue; diff --git a/qemu-char.c b/qemu-char.c index 05df971..ac65a1c 100644 --- a/qemu-char.c +++ b/qemu-char.c @@ -2000,8 +2000,9 @@ static int tcp_get_msgfd(CharDriverState *chr) { TCPCharDriver *s = chr->opaque; - - return s->msgfd; + int fd = s->msgfd; + s->msgfd = -1; + return fd; } #ifndef _WIN32 @@ -2089,10 +2090,6 @@ static void tcp_chr_read(void *opaque) tcp_chr_process_IAC_bytes(chr, s, buf, &size); if (size > 0) qemu_chr_read(chr, buf, size); - if (s->msgfd != -1) { - close(s->msgfd); - s->msgfd = -1; - } } }