mirrors/qemu
1
0
Fork 0
Code Issues Pull Requests Packages Projects Releases Wiki Activity

vhost-user: Fix double-close on slave_read() error path

Browse Source 
Some message types, e.g. VHOST_USER_SLAVE_VRING_HOST_NOTIFIER_MSG,
can convey file descriptors. These must be closed before returning
from slave_read() to avoid being leaked. This can currently be done
in two different places:

[1] just after the request has been processed

[2] on the error path, under the goto label err:

These path are supposed to be mutually exclusive but they are not
actually. If the VHOST_USER_NEED_REPLY_MASK flag was passed and the
sending of the reply fails, both [1] and [2] are performed with the
same descriptor values. This can potentially cause subtle bugs if one
of the descriptor was recycled by some other thread in the meantime.

This code duplication complicates rollback for no real good benefit.
Do the closing in a unique place, under a new fdcleanup: goto label
at the end of the function.

Signed-off-by: Greg Kurz <groug@kaod.org>
Message-Id: <20210312092212.782255-3-groug@kaod.org>
Reviewed-by: Michael S. Tsirkin <mst@redhat.com>
Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>
This commit is contained in:
Greg Kurz 2021-03-12 10:22:07 +01:00 committed by Michael S. Tsirkin
parent a890557d5a
commit 9e06080bed
1 changed files with 3 additions and 8 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

11
hw/virtio/vhost-user.c
View File

@ -1475,13 +1475,6 @@ static void slave_read(void *opaque)
ret = -EINVAL;
}
/* Close the remaining file descriptors. */
for (i = 0; i < fdsize; i++) {
if (fd[i] != -1) {
close(fd[i]);
}
}
/*
* REPLY_ACK feature handling. Other reply types has to be managed
* directly in their request handlers.
@ -1511,12 +1504,14 @@ static void slave_read(void *opaque)
}
}
return;
goto fdcleanup;
err:
qemu_set_fd_handler(u->slave_fd, NULL, NULL, NULL);
close(u->slave_fd);
u->slave_fd = -1;
fdcleanup:
for (i = 0; i < fdsize; i++) {
if (fd[i] != -1) {
close(fd[i]);