On Thu, 2026-10-15 at 09:04 +0100, Daniel P. Berrangé wrote:
> On Thu, Oct 15, 2026 at 03:34:43AM +0200, Filip Hejsek wrote:
> > Use qemu_dup() to ensure the cloexec flag is set on dup'd fds.
> >
> > The only direct users of dup() are now guest syscalls in the user
> > emulators and the implementation of qemu_dup() itself.
> >
> > Signed-off-by: Filip Hejsek <[email protected]>
> > ---
> > For each call I replaced, I checked how the resulting fd is used
> > afterwards. In summary:
> >
> > hw/virtio/virtio.c:
> > the fd is passed to memory_region_init_ram_from_fd, used for mmaping
> >
> > io/channel-file.c:
> > the fd is passed to qio_channel_file_new_fd, used for reading/writing
> >
> > migration/cpr-exec.c:
> > the cloexec flag is cleared
> >
> > qemu-nbd.c:
> > the fd is stored into old_stderr, then dup2'd back to STDERR_FILENO
> >
> > system/vl.c:
> > no change in behavior, the code was already setting the cloexec flag
> >
> > ui/dmabuf.c:
> > the function is called by qemu_spice_gl_update, which passes the fds
> > to SPICE server
> >
> > tests/qtest/bios-tables-test.c:
> > temp dup of stdout, dup2'd back to STDOUT_FILENO
> >
> > tests/unit/test-util-sockets.c:
> > one fd is dup'd again and returned from mock monitor_get_fd; another
> > is converted to string and given as address to socket_connect
> >
> > tests/unit/test-vmstate.c:
> > passed to qio_channel_file_new_fd to create a channel
> >
> > Changes in v3:
> > - Also replace the use in migration/cpr-exec.c and the tests
> > - Link to v2:
> > https://lore.kernel.org/qemu-devel/[email protected]
> >
> > Changes in v2:
> > - Replace the use in qemu-nbd.c too
> > - Changed commit message
> > - Link to v1:
> > https://lore.kernel.org/qemu-devel/[email protected]
> > ---
> > hw/virtio/virtio.c | 2 +-
> > io/channel-file.c | 2 +-
> > migration/cpr-exec.c | 2 +-
> > qemu-nbd.c | 2 +-
> > system/vl.c | 9 +--------
> > tests/qtest/bios-tables-test.c | 2 +-
> > tests/unit/test-util-sockets.c | 6 +++---
> > tests/unit/test-vmstate.c | 2 +-
> > ui/dmabuf.c | 2 +-
> > 9 files changed, 11 insertions(+), 18 deletions(-)
>
>
> > diff --git a/migration/cpr-exec.c b/migration/cpr-exec.c
> > index daa50916d2..2f38db30bb 100644
> > --- a/migration/cpr-exec.c
> > +++ b/migration/cpr-exec.c
> > @@ -44,7 +44,7 @@ bool cpr_exec_persist_state(QEMUFile *f, Error **errp)
> > {
> > QIOChannelFile *fioc = QIO_CHANNEL_FILE(qemu_file_get_ioc(f));
> > /* coverity[leaked_storage] - mfd intentionally kept open across
> > exec() */
> > - int mfd = dup(fioc->fd);
> > + int mfd = qemu_dup(fioc->fd);
> > char val[16];
>
> This change needed to be:
>
> int mfd = qemu_dup(fioc->fd);
> qemu_clear_cloexec(fioc->fd);
>
There was a call to qemu_clear_cloexec(mfd) already, even before the
change, just outside the visible hunk:
bool cpr_exec_persist_state(QEMUFile *f, Error **errp)
{
QIOChannelFile *fioc = QIO_CHANNEL_FILE(qemu_file_get_ioc(f));
/* coverity[leaked_storage] - mfd intentionally kept open across exec()
*/
int mfd = qemu_dup(fioc->fd);
char val[16];
/* Remember mfd in environment for post-exec load */
qemu_clear_cloexec(mfd);
snprintf(val, sizeof(val), "%d", mfd);
if (!g_setenv(CPR_EXEC_STATE_NAME, val, 1)) {
error_setg(errp, "Setting env %s = %s failed", CPR_EXEC_STATE_NAME,
val);
return false;
}
return true;
}
>
> If that is added:
>
> Reviewed-by: Daniel P. Berrangé <[email protected]>
>
>
> With regards,
> Daniel
