[PATCH TRIVIAL] treewide: use qemu_dup() instead of dup()

[PATCH TRIVIAL] treewide: use qemu_dup() instead of dup()

[Filip Hejsek]

Make the use of qemu_dup() more consistent and eliminate the possibility
of accidentally creating non-cloexec fds.

Signed-off-by: Filip Hejsek <filip.hejsek@gmail.com>
---
 hw/virtio/virtio.c | 2 +-
 io/channel-file.c  | 2 +-
 system/vl.c        | 9 +--------
 ui/dmabuf.c        | 2 +-
 4 files changed, 4 insertions(+), 11 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/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 TRIVIAL] treewide: use qemu_dup() instead of dup()

[Marc-André Lureau]

Hi

On Mon, Oct 12, 2026 at 11:44 PM Filip Hejsek <filip.hejsek@gmail.com> wrote:
>
> Make the use of qemu_dup() more consistent and eliminate the possibility
> of accidentally creating non-cloexec fds.
>

Not so trivial, in my humble opinion, as there might have been some
reason it was avoided.

> Signed-off-by: Filip Hejsek <filip.hejsek@gmail.com>
> ---
>  hw/virtio/virtio.c | 2 +-
>  io/channel-file.c  | 2 +-
>  system/vl.c        | 9 +--------
>  ui/dmabuf.c        | 2 +-
>  4 files changed, 4 insertions(+), 11 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/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;
>      }
>  }
>

After review I think these changes are valid. These changes shouldn't
impact the exec() code paths in qemu.

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


> ---
> base-commit: 615ece3c406b262996e569f29ea6eed0d81c4d8d
> change-id: 20261012-use-qemu-dup-38628dbac3ee
>
> Best regards,
> --
> Filip Hejsek <filip.hejsek@gmail.com>
>

Re: [PATCH TRIVIAL] treewide: use qemu_dup() instead of dup()

[Filip Hejsek]

Hello,

On Mon, 2026-10-12 at 23:54 +0400, Marc-André Lureau wrote:
> Hi
> 
> On Mon, Oct 12, 2026 at 11:44 PM Filip Hejsek <filip.hejsek@gmail.com> wrote:
> > 
> > Make the use of qemu_dup() more consistent and eliminate the possibility
> > of accidentally creating non-cloexec fds.
> > 
> 
> Not so trivial, in my humble opinion, as there might have been some
> reason it was avoided.
> 

You're right, I shouldn't have tagged this as TRIVIAL.

I've actually intentionally left some dup()s unchanged for exactly this
reason. They are the ones in qemu-nbd.c, migration/cpr-exec.c, and
under linux-user/ and bsd-user/. I also didn't touch the tests, as I
didn't consider evaluating them to be worth the effort.

I should have put this info in the commit message...

> > [...]
> 
> After review I think these changes are valid. These changes shouldn't
> impact the exec() code paths in qemu.
> 
> Reviewed-by: Marc-André Lureau <marcandre.lureau@redhat.com>
> 

Thanks for reviewing this.

Filip

Re: [PATCH TRIVIAL] treewide: use qemu_dup() instead of dup()

[Filip Hejsek]

On Mon, 2026-10-12 at 22:12 +0200, Filip Hejsek wrote:
> I've actually intentionally left some dup()s unchanged for exactly this
> reason. They are the ones in qemu-nbd.c, migration/cpr-exec.c, and
> under linux-user/ and bsd-user/. I also didn't touch the tests, as I
> didn't consider evaluating them to be worth the effort.

Now that I think about it again, the use in qemu-nbd.c should probably
be replaced too... I'll probably send a v2.

Filip

Re: [PATCH TRIVIAL] treewide: use qemu_dup() instead of dup()

[Daniel P. Berrangé]

On Mon, Oct 12, 2026 at 10:12:34PM +0200, Filip Hejsek wrote:
> Hello,
> 
> On Mon, 2026-10-12 at 23:54 +0400, Marc-André Lureau wrote:
> > Hi
> > 
> > On Mon, Oct 12, 2026 at 11:44 PM Filip Hejsek <filip.hejsek@gmail.com> wrote:
> > > 
> > > Make the use of qemu_dup() more consistent and eliminate the possibility
> > > of accidentally creating non-cloexec fds.
> > > 
> > 
> > Not so trivial, in my humble opinion, as there might have been some
> > reason it was avoided.
> > 
> 
> You're right, I shouldn't have tagged this as TRIVIAL.
> 
> I've actually intentionally left some dup()s unchanged for exactly this
> reason. They are the ones in qemu-nbd.c, migration/cpr-exec.c, and
> under linux-user/ and bsd-user/. I also didn't touch the tests, as I
> didn't consider evaluating them to be worth the effort.

IMHO, everything must be switched to use qemu_dup.  If a few callers
actually want the FD to remain open across execve, then they should
explicitly call qemu_clear_cloexec().  THis makes the intended
behaviour clear, as opposed to looking like a potential mistake.

> 
> I should have put this info in the commit message...
> 
> > > [...]
> > 
> > After review I think these changes are valid. These changes shouldn't
> > impact the exec() code paths in qemu.
> > 
> > Reviewed-by: Marc-André Lureau <marcandre.lureau@redhat.com>
> > 
> 
> Thanks for reviewing this.
> 
> Filip
> 

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 :|