[PATCH v2 0/2] io: make sure we don't close stdio fds

[PATCH v2 0/2] io: make sure we don't close stdio fds

[Filip Hejsek]

The stdio chardev currently closes the stdin and stdout fds when it is
destroyed. This means that, assuming char0 is a stdio chardev, invoking
the HMP command

    chardev-change char0 null

will cause QEMU to run with descriptors 0 and 1 closed. That's not a
great situation, as:
  - the stdio chardev cannot be recreated
  - code assuming fds 0 and 1 are open will misbehave
  - the fds 0 and 1 will get assigned to newly opened files, possibly
    causing them to be confused for actual stdin/stdout

The fist patch fixes the problem by duplicating the fds before they get
passed to qio_channel_file_new_fd(), the second patch adds a warning
that is printed when qio_channel_file_new_fd() is called with a stdio
fd.

Changes in v2:
- Code style fixes
- Set *errp when dup fails
- Link to v1: https://lore.kernel.org/qemu-devel/20261013-fix-stdio-close-v1-0-4ecfa416c7bc@gmail.com

Signed-off-by: Filip Hejsek <filip.hejsek@gmail.com>
---
Filip Hejsek (2):
      char-stdio: avoid closing stdio fds
      io: report a warning when a file channel is created from a stdio fd

 chardev/char-stdio.c | 16 +++++++++++++++-
 io/channel-file.c    |  6 ++++++
 2 files changed, 21 insertions(+), 1 deletion(-)
---
base-commit: 615ece3c406b262996e569f29ea6eed0d81c4d8d
change-id: 20261012-fix-stdio-close-98864055d473

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

[PATCH v2 1/2] char-stdio: avoid closing stdio fds

[Filip Hejsek]

Use a duplicate of stdin and stdout fds to avoid closing the standard
descriptors when the stdio chardev is destroyed.

Signed-off-by: Filip Hejsek <filip.hejsek@gmail.com>
---
 chardev/char-stdio.c | 16 +++++++++++++++-
 1 file changed, 15 insertions(+), 1 deletion(-)

diff --git a/chardev/char-stdio.c b/chardev/char-stdio.c
index fe1cbb2d88..19860e960d 100644
--- a/chardev/char-stdio.c
+++ b/chardev/char-stdio.c
@@ -89,6 +89,7 @@ static bool stdio_chr_open(Chardev *chr, ChardevBackend *backend, Error **errp)
 {
     ChardevStdio *opts = backend->u.stdio.data;
     struct sigaction act;
+    int fd_in, fd_out;
 
     if (is_daemonized()) {
         error_setg(errp, "cannot use stdio with -daemonize");
@@ -108,7 +109,20 @@ static bool stdio_chr_open(Chardev *chr, ChardevBackend *backend, Error **errp)
         return false;
     }
 
-    if (!qemu_chr_open_fd(chr, 0, 1, errp)) {
+    fd_in = qemu_dup(0);
+    if (fd_in < 0) {
+        error_setg_errno(errp, errno, "Failed to dup stdin fd");
+        return false;
+    }
+    fd_out = qemu_dup(1);
+    if (fd_out < 0) {
+        error_setg_errno(errp, errno, "Failed to dup stdout fd");
+        qemu_close(fd_in);
+        return false;
+    }
+    if (!qemu_chr_open_fd(chr, fd_in, fd_out, errp)) {
+        qemu_close(fd_in);
+        qemu_close(fd_out);
         return false;
     }
 

-- 
2.56.0

Re: [PATCH v2 1/2] char-stdio: avoid closing stdio fds

[Marc-André Lureau]

On Thu, Oct 15, 2026 at 5:59 AM Filip Hejsek <filip.hejsek@gmail.com> wrote:
>
> Use a duplicate of stdin and stdout fds to avoid closing the standard
> descriptors when the stdio chardev is destroyed.
>
> Signed-off-by: Filip Hejsek <filip.hejsek@gmail.com>

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

> ---
>  chardev/char-stdio.c | 16 +++++++++++++++-
>  1 file changed, 15 insertions(+), 1 deletion(-)
>
> diff --git a/chardev/char-stdio.c b/chardev/char-stdio.c
> index fe1cbb2d88..19860e960d 100644
> --- a/chardev/char-stdio.c
> +++ b/chardev/char-stdio.c
> @@ -89,6 +89,7 @@ static bool stdio_chr_open(Chardev *chr, ChardevBackend *backend, Error **errp)
>  {
>      ChardevStdio *opts = backend->u.stdio.data;
>      struct sigaction act;
> +    int fd_in, fd_out;
>
>      if (is_daemonized()) {
>          error_setg(errp, "cannot use stdio with -daemonize");
> @@ -108,7 +109,20 @@ static bool stdio_chr_open(Chardev *chr, ChardevBackend *backend, Error **errp)
>          return false;
>      }
>
> -    if (!qemu_chr_open_fd(chr, 0, 1, errp)) {
> +    fd_in = qemu_dup(0);
> +    if (fd_in < 0) {
> +        error_setg_errno(errp, errno, "Failed to dup stdin fd");
> +        return false;
> +    }
> +    fd_out = qemu_dup(1);
> +    if (fd_out < 0) {
> +        error_setg_errno(errp, errno, "Failed to dup stdout fd");
> +        qemu_close(fd_in);
> +        return false;
> +    }
> +    if (!qemu_chr_open_fd(chr, fd_in, fd_out, errp)) {
> +        qemu_close(fd_in);
> +        qemu_close(fd_out);
>          return false;
>      }
>
>
> --
> 2.56.0
>

[PATCH v2 2/2] io: report a warning when a file channel is created from a stdio fd

[Filip Hejsek]

A channel closes its fd when the last reference to it is dropped.
Closing stdio fds is typically unwanted, so warn when they are passed to
qio_channel_file_new_fd().

Signed-off-by: Filip Hejsek <filip.hejsek@gmail.com>
---
 io/channel-file.c | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/io/channel-file.c b/io/channel-file.c
index 5cef75a67c..4c9da6619b 100644
--- a/io/channel-file.c
+++ b/io/channel-file.c
@@ -23,6 +23,7 @@
 #include "io/channel-util.h"
 #include "io/channel-watch.h"
 #include "qapi/error.h"
+#include "qemu/error-report.h"
 #include "qemu/module.h"
 #include "qemu/sockets.h"
 #include "trace.h"
@@ -32,6 +33,11 @@ qio_channel_file_new_fd(int fd)
 {
     QIOChannelFile *ioc;
 
+    if (fd <= STDERR_FILENO) {
+        warn_report("potential bug: creating a channel from stdio fd %d; "
+                    "the fd will be closed when the channel is destroyed", fd);
+    }
+
     ioc = QIO_CHANNEL_FILE(object_new(TYPE_QIO_CHANNEL_FILE));
 
     ioc->fd = fd;

-- 
2.56.0

Re: [PATCH v2 2/2] io: report a warning when a file channel is created from a stdio fd

[Daniel P. Berrangé]

On Thu, Oct 15, 2026 at 03:58:25AM +0200, Filip Hejsek wrote:
> A channel closes its fd when the last reference to it is dropped.
> Closing stdio fds is typically unwanted, so warn when they are passed to
> qio_channel_file_new_fd().
> 
> Signed-off-by: Filip Hejsek <filip.hejsek@gmail.com>
> ---
>  io/channel-file.c | 6 ++++++
>  1 file changed, 6 insertions(+)
> 
> diff --git a/io/channel-file.c b/io/channel-file.c
> index 5cef75a67c..4c9da6619b 100644
> --- a/io/channel-file.c
> +++ b/io/channel-file.c
> @@ -23,6 +23,7 @@
>  #include "io/channel-util.h"
>  #include "io/channel-watch.h"
>  #include "qapi/error.h"
> +#include "qemu/error-report.h"
>  #include "qemu/module.h"
>  #include "qemu/sockets.h"
>  #include "trace.h"
> @@ -32,6 +33,11 @@ qio_channel_file_new_fd(int fd)
>  {
>      QIOChannelFile *ioc;
>  
> +    if (fd <= STDERR_FILENO) {
> +        warn_report("potential bug: creating a channel from stdio fd %d; "
> +                    "the fd will be closed when the channel is destroyed", fd);
> +    }
> +
>      ioc = QIO_CHANNEL_FILE(object_new(TYPE_QIO_CHANNEL_FILE));
>  
>      ioc->fd = fd;

Can we also update the API docs for qio_channel_file_new_fd, to
say

   @fd must not point to stdin/stdout/stderr, since the FD
   will be closed by the channel on destruction. If requiring
   a channel to wrap stdio, use qio_channel_file_new_dupfd()
   instead.


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