[PATCH not-so-trivial v2] treewide: use qemu_dup() instead of dup()

[PATCH not-so-trivial v2] treewide: use qemu_dup() instead of dup()

[Filip Hejsek]

Use qemu_dup() to ensure the cloexec flag is set on dup'd fds.

The following dup() instances were left unchanged:

    FILE                     REASON
    util/osdep.c             implementation of qemu_dup
    migration/cpr-exec.c     fd is intentionally kept across exec
    {linux,bsd}-user/*       guest syscalls
    tests/*                  left for future work

Signed-off-by: Filip Hejsek <filip.hejsek@gmail.com>
---
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

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

These uses shouldn't require keeping the fd open across exec.

Changes in v2:
- Replace the use in qemu-nbd.c too
- Changed commit message
- Link to v1: https://lore.kernel.org/qemu-devel/20261012-use-qemu-dup-v1-1-03d94852a6c8@gmail.com
---
 hw/virtio/virtio.c | 2 +-
 io/channel-file.c  | 2 +-
 qemu-nbd.c         | 2 +-
 system/vl.c        | 9 +--------
 ui/dmabuf.c        | 2 +-
 5 files changed, 5 insertions(+), 12 deletions(-)

diff --git a/hw/virtio/virtio.c b/hw/virtio/virtio.c
index c9f9e5b511..70779fbd56 100644
--- a/hw/virtio/virtio.c
+++ b/hw/virtio/virtio.c
@@ -3204,7 +3204,7 @@ VirtioSharedMemoryMapping *virtio_shared_memory_mapping_new(uint8_t shmid,
         return NULL;
     }
 
-    fd = dup(fd);
+    fd = qemu_dup(fd);
     if (fd < 0) {
         error_report("Failed to duplicate fd: %s", strerror(errno));
         return NULL;
diff --git a/io/channel-file.c b/io/channel-file.c
index 5cef75a67c..3f707e80c9 100644
--- a/io/channel-file.c
+++ b/io/channel-file.c
@@ -48,7 +48,7 @@ qio_channel_file_new_fd(int fd)
 QIOChannelFile *
 qio_channel_file_new_dupfd(int fd, Error **errp)
 {
-    int newfd = dup(fd);
+    int newfd = qemu_dup(fd);
 
     if (newfd < 0) {
         error_setg_errno(errp, errno, "Could not dup FD %d", fd);
diff --git a/qemu-nbd.c b/qemu-nbd.c
index ed5895861b..db1a70b008 100644
--- a/qemu-nbd.c
+++ b/qemu-nbd.c
@@ -975,7 +975,7 @@ int main(int argc, char **argv)
 
             /* Remember parent's stderr if we will be restoring it. */
             if (opts.verbose /* fork_process is set */) {
-                opts.old_stderr = dup(STDERR_FILENO);
+                opts.old_stderr = qemu_dup(STDERR_FILENO);
                 if (opts.old_stderr < 0) {
                     error_report("Could not dup original stderr: %s",
                                  strerror(errno));
diff --git a/system/vl.c b/system/vl.c
index 468a9fc247..9381e3d7f7 100644
--- a/system/vl.c
+++ b/system/vl.c
@@ -614,14 +614,7 @@ static int parse_add_fd(void *opaque, QemuOpts *opts, Error **errp)
         return -1;
     }
 
-#ifdef F_DUPFD_CLOEXEC
-    dupfd = fcntl(fd, F_DUPFD_CLOEXEC, 0);
-#else
-    dupfd = dup(fd);
-    if (dupfd != -1) {
-        qemu_set_cloexec(dupfd);
-    }
-#endif
+    dupfd = qemu_dup(fd);
     if (dupfd == -1) {
         error_setg_errno(errp, errno, "error duplicating fd");
         return -1;
diff --git a/ui/dmabuf.c b/ui/dmabuf.c
index 497f82cc61..6d5dffdf84 100644
--- a/ui/dmabuf.c
+++ b/ui/dmabuf.c
@@ -95,7 +95,7 @@ void qemu_dmabuf_dup_fds(QemuDmaBuf *dmabuf, int *fds, int nfds)
     assert(nfds >= dmabuf->num_planes);
 
     for (i = 0; i < dmabuf->num_planes; i++) {
-        fds[i] = dmabuf->fd[i] >= 0 ? dup(dmabuf->fd[i]) : -1;
+        fds[i] = dmabuf->fd[i] >= 0 ? qemu_dup(dmabuf->fd[i]) : -1;
     }
 }
 

---
base-commit: 615ece3c406b262996e569f29ea6eed0d81c4d8d
change-id: 20261012-use-qemu-dup-38628dbac3ee

Best regards,
--  
Filip Hejsek <filip.hejsek@gmail.com>

Re: [PATCH not-so-trivial v2] treewide: use qemu_dup() instead of dup()

[Marc-André Lureau]

On Tue, Oct 13, 2026 at 2:02 AM Filip Hejsek <filip.hejsek@gmail.com> wrote:
>
> Use qemu_dup() to ensure the cloexec flag is set on dup'd fds.
>
> The following dup() instances were left unchanged:
>
>     FILE                     REASON
>     util/osdep.c             implementation of qemu_dup
>     migration/cpr-exec.c     fd is intentionally kept across exec
>     {linux,bsd}-user/*       guest syscalls
>     tests/*                  left for future work
>
> Signed-off-by: Filip Hejsek <filip.hejsek@gmail.com>

Reviewed-by: Marc-André Lureau <marcandre.lureau@redhat.com>

> ---
> 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
>
> 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
>
> These uses shouldn't require keeping the fd open across exec.
>
> Changes in v2:
> - Replace the use in qemu-nbd.c too
> - Changed commit message
> - Link to v1: https://lore.kernel.org/qemu-devel/20261012-use-qemu-dup-v1-1-03d94852a6c8@gmail.com
> ---
>  hw/virtio/virtio.c | 2 +-
>  io/channel-file.c  | 2 +-
>  qemu-nbd.c         | 2 +-
>  system/vl.c        | 9 +--------
>  ui/dmabuf.c        | 2 +-
>  5 files changed, 5 insertions(+), 12 deletions(-)
>
> diff --git a/hw/virtio/virtio.c b/hw/virtio/virtio.c
> index c9f9e5b511..70779fbd56 100644
> --- a/hw/virtio/virtio.c
> +++ b/hw/virtio/virtio.c
> @@ -3204,7 +3204,7 @@ VirtioSharedMemoryMapping *virtio_shared_memory_mapping_new(uint8_t shmid,
>          return NULL;
>      }
>
> -    fd = dup(fd);
> +    fd = qemu_dup(fd);
>      if (fd < 0) {
>          error_report("Failed to duplicate fd: %s", strerror(errno));
>          return NULL;
> diff --git a/io/channel-file.c b/io/channel-file.c
> index 5cef75a67c..3f707e80c9 100644
> --- a/io/channel-file.c
> +++ b/io/channel-file.c
> @@ -48,7 +48,7 @@ qio_channel_file_new_fd(int fd)
>  QIOChannelFile *
>  qio_channel_file_new_dupfd(int fd, Error **errp)
>  {
> -    int newfd = dup(fd);
> +    int newfd = qemu_dup(fd);
>
>      if (newfd < 0) {
>          error_setg_errno(errp, errno, "Could not dup FD %d", fd);
> diff --git a/qemu-nbd.c b/qemu-nbd.c
> index ed5895861b..db1a70b008 100644
> --- a/qemu-nbd.c
> +++ b/qemu-nbd.c
> @@ -975,7 +975,7 @@ int main(int argc, char **argv)
>
>              /* Remember parent's stderr if we will be restoring it. */
>              if (opts.verbose /* fork_process is set */) {
> -                opts.old_stderr = dup(STDERR_FILENO);
> +                opts.old_stderr = qemu_dup(STDERR_FILENO);
>                  if (opts.old_stderr < 0) {
>                      error_report("Could not dup original stderr: %s",
>                                   strerror(errno));
> diff --git a/system/vl.c b/system/vl.c
> index 468a9fc247..9381e3d7f7 100644
> --- a/system/vl.c
> +++ b/system/vl.c
> @@ -614,14 +614,7 @@ static int parse_add_fd(void *opaque, QemuOpts *opts, Error **errp)
>          return -1;
>      }
>
> -#ifdef F_DUPFD_CLOEXEC
> -    dupfd = fcntl(fd, F_DUPFD_CLOEXEC, 0);
> -#else
> -    dupfd = dup(fd);
> -    if (dupfd != -1) {
> -        qemu_set_cloexec(dupfd);
> -    }
> -#endif
> +    dupfd = qemu_dup(fd);
>      if (dupfd == -1) {
>          error_setg_errno(errp, errno, "error duplicating fd");
>          return -1;
> diff --git a/ui/dmabuf.c b/ui/dmabuf.c
> index 497f82cc61..6d5dffdf84 100644
> --- a/ui/dmabuf.c
> +++ b/ui/dmabuf.c
> @@ -95,7 +95,7 @@ void qemu_dmabuf_dup_fds(QemuDmaBuf *dmabuf, int *fds, int nfds)
>      assert(nfds >= dmabuf->num_planes);
>
>      for (i = 0; i < dmabuf->num_planes; i++) {
> -        fds[i] = dmabuf->fd[i] >= 0 ? dup(dmabuf->fd[i]) : -1;
> +        fds[i] = dmabuf->fd[i] >= 0 ? qemu_dup(dmabuf->fd[i]) : -1;
>      }
>  }
>
>
> ---
> base-commit: 615ece3c406b262996e569f29ea6eed0d81c4d8d
> change-id: 20261012-use-qemu-dup-38628dbac3ee
>
> Best regards,
> --
> Filip Hejsek <filip.hejsek@gmail.com>
>

Re: [PATCH not-so-trivial v2] treewide: use qemu_dup() instead of dup()

[Daniel P. Berrangé]

On Tue, Oct 13, 2026 at 12:01:58AM +0200, Filip Hejsek wrote:
> Use qemu_dup() to ensure the cloexec flag is set on dup'd fds.
> 
> The following dup() instances were left unchanged:
> 
>     FILE                     REASON
>     util/osdep.c             implementation of qemu_dup
>     migration/cpr-exec.c     fd is intentionally kept across exec

IMHO that should use qemu_dup() too, and then explicitly
clear the O_CLOEXEC flag so intended behaviour is clear.
Right now it looks like a mistaken failure to use qemu_dup.

>     {linux,bsd}-user/*       guest syscalls

That's fine

>     tests/*                  left for future work

Why not fix them now ? There are only a few uses which look suitable
to change.

> 
> Signed-off-by: Filip Hejsek <filip.hejsek@gmail.com>
> ---
> 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
> 
> 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
> 
> These uses shouldn't require keeping the fd open across exec.
> 
> Changes in v2:
> - Replace the use in qemu-nbd.c too
> - Changed commit message
> - Link to v1: https://lore.kernel.org/qemu-devel/20261012-use-qemu-dup-v1-1-03d94852a6c8@gmail.com
> ---
>  hw/virtio/virtio.c | 2 +-
>  io/channel-file.c  | 2 +-
>  qemu-nbd.c         | 2 +-
>  system/vl.c        | 9 +--------
>  ui/dmabuf.c        | 2 +-
>  5 files changed, 5 insertions(+), 12 deletions(-)
> 
> diff --git a/hw/virtio/virtio.c b/hw/virtio/virtio.c
> index c9f9e5b511..70779fbd56 100644
> --- a/hw/virtio/virtio.c
> +++ b/hw/virtio/virtio.c
> @@ -3204,7 +3204,7 @@ VirtioSharedMemoryMapping *virtio_shared_memory_mapping_new(uint8_t shmid,
>          return NULL;
>      }
>  
> -    fd = dup(fd);
> +    fd = qemu_dup(fd);
>      if (fd < 0) {
>          error_report("Failed to duplicate fd: %s", strerror(errno));
>          return NULL;
> diff --git a/io/channel-file.c b/io/channel-file.c
> index 5cef75a67c..3f707e80c9 100644
> --- a/io/channel-file.c
> +++ b/io/channel-file.c
> @@ -48,7 +48,7 @@ qio_channel_file_new_fd(int fd)
>  QIOChannelFile *
>  qio_channel_file_new_dupfd(int fd, Error **errp)
>  {
> -    int newfd = dup(fd);
> +    int newfd = qemu_dup(fd);
>  
>      if (newfd < 0) {
>          error_setg_errno(errp, errno, "Could not dup FD %d", fd);
> diff --git a/qemu-nbd.c b/qemu-nbd.c
> index ed5895861b..db1a70b008 100644
> --- a/qemu-nbd.c
> +++ b/qemu-nbd.c
> @@ -975,7 +975,7 @@ int main(int argc, char **argv)
>  
>              /* Remember parent's stderr if we will be restoring it. */
>              if (opts.verbose /* fork_process is set */) {
> -                opts.old_stderr = dup(STDERR_FILENO);
> +                opts.old_stderr = qemu_dup(STDERR_FILENO);
>                  if (opts.old_stderr < 0) {
>                      error_report("Could not dup original stderr: %s",
>                                   strerror(errno));
> diff --git a/system/vl.c b/system/vl.c
> index 468a9fc247..9381e3d7f7 100644
> --- a/system/vl.c
> +++ b/system/vl.c
> @@ -614,14 +614,7 @@ static int parse_add_fd(void *opaque, QemuOpts *opts, Error **errp)
>          return -1;
>      }
>  
> -#ifdef F_DUPFD_CLOEXEC
> -    dupfd = fcntl(fd, F_DUPFD_CLOEXEC, 0);
> -#else
> -    dupfd = dup(fd);
> -    if (dupfd != -1) {
> -        qemu_set_cloexec(dupfd);
> -    }
> -#endif
> +    dupfd = qemu_dup(fd);
>      if (dupfd == -1) {
>          error_setg_errno(errp, errno, "error duplicating fd");
>          return -1;
> diff --git a/ui/dmabuf.c b/ui/dmabuf.c
> index 497f82cc61..6d5dffdf84 100644
> --- a/ui/dmabuf.c
> +++ b/ui/dmabuf.c
> @@ -95,7 +95,7 @@ void qemu_dmabuf_dup_fds(QemuDmaBuf *dmabuf, int *fds, int nfds)
>      assert(nfds >= dmabuf->num_planes);
>  
>      for (i = 0; i < dmabuf->num_planes; i++) {
> -        fds[i] = dmabuf->fd[i] >= 0 ? dup(dmabuf->fd[i]) : -1;
> +        fds[i] = dmabuf->fd[i] >= 0 ? qemu_dup(dmabuf->fd[i]) : -1;
>      }
>  }
>  
> 
> ---
> base-commit: 615ece3c406b262996e569f29ea6eed0d81c4d8d
> change-id: 20261012-use-qemu-dup-38628dbac3ee
> 
> Best regards,
> --  
> Filip Hejsek <filip.hejsek@gmail.com>
> 

With regards,
Daniel
-- 
|: https://berrange.com       ~~        https://hachyderm.io/@berrange :|
|: https://libvirt.org          ~~          https://entangle-photo.org :|
|: https://pixelfed.art/berrange   ~~    https://fstop138.berrange.com :|