[Qemu-devel] [PATCH for-2.1 0/2] qemu-char: fixes to the new get_msgfds API

[Qemu-devel] [PATCH for-2.1 0/2] qemu-char: fixes to the new get_msgfds API

[Stefan Hajnoczi]

The recent changes to extend qemu-char get_msgfds to support multiple file
descriptors caused a qemu-iotests regression and also introduced a file
descriptor leak.  This patch series fixes the bugs.

Stefan Hajnoczi (2):
  qemu-char: fix qemu_chr_fe_get_msgfd()
  qemu-char: avoid leaking unused fds in tcp_get_msgfds()

 qemu-char.c | 9 ++++++++-
 1 file changed, 8 insertions(+), 1 deletion(-)

-- 
1.9.3

[Qemu-devel] [PATCH for-2.1 1/2] qemu-char: fix qemu_chr_fe_get_msgfd()

[Stefan Hajnoczi]

Commit c76bf6bb8fbbb233a7d3641e09229d23747d5ee3 ("Add chardev API
qemu_chr_fe_get_msgfds") broke qemu_chr_fe_get_msgfd() because it
changed the return value.

Callers expect -1 if no fd is available.  The commit changed the return
value to 0 (which is a valid file descriptor number) so callers always
detected a file descriptor even if none was available.

This patch fixes qemu-iotests 045:

  $ cd tests/qemu-iotests && ./check 045
  [...]
  +FAIL: test_add_fd_invalid_fd (__main__.TestFdSets)
  +----------------------------------------------------------------------
  +Traceback (most recent call last):
  +  File "./045", line 123, in test_add_fd_invalid_fd
  +    self.assert_qmp(result, 'error/class', 'GenericError')
  +  File "/home/stefanha/qemu/tests/qemu-iotests/iotests.py", line 232, in assert_qmp
  +    result = self.dictpath(d, path)
  +  File "/home/stefanha/qemu/tests/qemu-iotests/iotests.py", line 211, in dictpath
  +    self.fail('failed path traversal for "%s" in "%s"' % (path, str(d)))
  +AssertionError: failed path traversal for "error/class" in "{u'return': {u'fdset-id': 2, u'fd': 0}}"

Cc: Nikolay Nikolaev <n.nikolaev@virtualopensystems.com>
Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
---
 qemu-char.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/qemu-char.c b/qemu-char.c
index b3bd3b5..6ee2275 100644
--- a/qemu-char.c
+++ b/qemu-char.c
@@ -205,7 +205,7 @@ void qemu_chr_be_write(CharDriverState *s, uint8_t *buf, int len)
 int qemu_chr_fe_get_msgfd(CharDriverState *s)
 {
     int fd;
-    return (qemu_chr_fe_get_msgfds(s, &fd, 1) >= 0) ? fd : -1;
+    return (qemu_chr_fe_get_msgfds(s, &fd, 1) == 1) ? fd : -1;
 }
 
 int qemu_chr_fe_get_msgfds(CharDriverState *s, int *fds, int len)
-- 
1.9.3

[Qemu-devel] [PATCH for-2.1 2/2] qemu-char: avoid leaking unused fds in tcp_get_msgfds()

[Stefan Hajnoczi]

Commit c76bf6bb8fbbb233a7d3641e09229d23747d5ee3 ("Add chardev API
qemu_chr_fe_get_msgfds") extended the get_msgfds API from one to
multiple file descriptors.  It forgot to close unused file descriptors
before freeing the file descriptor array.

This patch prevents a file descriptor leak if the tcp_get_msgfds()
callers requests fewer file descriptors than are available.

Cc: Nikolay Nikolaev <n.nikolaev@virtualopensystems.com>
Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
---
 qemu-char.c | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/qemu-char.c b/qemu-char.c
index 6ee2275..a52613d 100644
--- a/qemu-char.c
+++ b/qemu-char.c
@@ -2482,8 +2482,15 @@ static int tcp_get_msgfds(CharDriverState *chr, int *fds, int num)
     int to_copy = (s->read_msgfds_num < num) ? s->read_msgfds_num : num;
 
     if (to_copy) {
+        int i;
+
         memcpy(fds, s->read_msgfds, to_copy * sizeof(int));
 
+        /* Close unused fds */
+        for (i = to_copy; i < s->read_msgfds_num; i++) {
+            close(s->read_msgfds[i]);
+        }
+
         g_free(s->read_msgfds);
         s->read_msgfds = 0;
         s->read_msgfds_num = 0;
-- 
1.9.3

Re: [Qemu-devel] [PATCH for-2.1 0/2] qemu-char: fixes to the new get_msgfds API

[Michael S. Tsirkin]

On Sun, Jun 22, 2014 at 10:38:35AM +0800, Stefan Hajnoczi wrote:
> The recent changes to extend qemu-char get_msgfds to support multiple file
> descriptors caused a qemu-iotests regression and also introduced a file
> descriptor leak.  This patch series fixes the bugs.

Applied, thanks!

> Stefan Hajnoczi (2):
>   qemu-char: fix qemu_chr_fe_get_msgfd()
>   qemu-char: avoid leaking unused fds in tcp_get_msgfds()
> 
>  qemu-char.c | 9 ++++++++-
>  1 file changed, 8 insertions(+), 1 deletion(-)
> 
> -- 
> 1.9.3