All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH v3 0/2] io: make sure we don't close stdio fds
@ 2026-10-16 23:09 Filip Hejsek
  2026-10-16 23:09 ` [PATCH v3 1/2] char-stdio: avoid closing " Filip Hejsek
                   ` (2 more replies)
  0 siblings, 3 replies; 4+ messages in thread
From: Filip Hejsek @ 2026-10-16 23:09 UTC (permalink / raw)
  To: qemu-devel; +Cc: Daniel P. Berrangé, Marc-André Lureau, Paolo Bonzini

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 and modifies the documentation of that function to say that passing
stdio descriptors is incorrect.

Changes in v3:
- Add documentation changes
- Link to v2: https://lore.kernel.org/qemu-devel/20261015-fix-stdio-close-v2-0-0f01e6580e24@gmail.com

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 +++++++++++++++-
 include/io/channel-file.h | 12 ++++++++----
 io/channel-file.c         |  6 ++++++
 3 files changed, 29 insertions(+), 5 deletions(-)
---
base-commit: 7e8d3c7fc460fe7cd28a3e423d7466fa292bc269
change-id: 20261012-fix-stdio-close-98864055d473

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



^ permalink raw reply	[flat|nested] 4+ messages in thread

* [PATCH v3 1/2] char-stdio: avoid closing stdio fds
  2026-10-16 23:09 [PATCH v3 0/2] io: make sure we don't close stdio fds Filip Hejsek
@ 2026-10-16 23:09 ` Filip Hejsek
  2026-10-16 23:09 ` [PATCH v3 2/2] io: report a warning when a file channel is created from a stdio fd Filip Hejsek
  2026-10-20 13:44 ` [PATCH v3 0/2] io: make sure we don't close stdio fds marcandre.lureau
  2 siblings, 0 replies; 4+ messages in thread
From: Filip Hejsek @ 2026-10-16 23:09 UTC (permalink / raw)
  To: qemu-devel; +Cc: Daniel P. Berrangé, Marc-André Lureau, Paolo Bonzini

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 f63bf413c7..16b49fd924 100644
--- a/chardev/char-stdio.c
+++ b/chardev/char-stdio.c
@@ -114,6 +114,7 @@ static bool stdio_chr_open(Chardev *chr, ChardevBackend *backend, Error **errp)
     StdioChardev *s = STDIO_CHARDEV(chr);
     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");
@@ -133,7 +134,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



^ permalink raw reply related	[flat|nested] 4+ messages in thread

* [PATCH v3 2/2] io: report a warning when a file channel is created from a stdio fd
  2026-10-16 23:09 [PATCH v3 0/2] io: make sure we don't close stdio fds Filip Hejsek
  2026-10-16 23:09 ` [PATCH v3 1/2] char-stdio: avoid closing " Filip Hejsek
@ 2026-10-16 23:09 ` Filip Hejsek
  2026-10-20 13:44 ` [PATCH v3 0/2] io: make sure we don't close stdio fds marcandre.lureau
  2 siblings, 0 replies; 4+ messages in thread
From: Filip Hejsek @ 2026-10-16 23:09 UTC (permalink / raw)
  To: qemu-devel; +Cc: Daniel P. Berrangé, Marc-André Lureau, Paolo Bonzini

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().

Change the documentation of qio_channel_file_new_fd() to explicitly say
that passing stdio descriptors is incorrect.

Signed-off-by: Filip Hejsek <filip.hejsek@gmail.com>
---
 include/io/channel-file.h | 12 ++++++++----
 io/channel-file.c         |  6 ++++++
 2 files changed, 14 insertions(+), 4 deletions(-)

diff --git a/include/io/channel-file.h b/include/io/channel-file.h
index d373a4e44d..6a6776ff8e 100644
--- a/include/io/channel-file.h
+++ b/include/io/channel-file.h
@@ -58,10 +58,14 @@ struct QIOChannelFile {
  * should be used instead, as this provides greater
  * functionality and cross platform portability.
  *
- * The channel will own the passed in file descriptor
- * and will take responsibility for closing it, so the
- * caller must not close it. If appropriate the caller
- * should dup() its FD before opening the channel.
+ * The channel will own the file descriptor that was
+ * passed to it and will take responsibility for closing
+ * it. This means the caller must not close it, and must
+ * not pass in descriptors that will remain in use after
+ * the channel is destroyed. In particular, @fd must
+ * not be one of the stdin/stdout/stderr descriptors. If
+ * requiring a channel to wrap a descriptor that should
+ * remain open, use qio_channel_file_new_dupfd() instead.
  *
  * Returns: the new channel object
  */
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



^ permalink raw reply related	[flat|nested] 4+ messages in thread

* Re: [PATCH v3 0/2] io: make sure we don't close stdio fds
  2026-10-16 23:09 [PATCH v3 0/2] io: make sure we don't close stdio fds Filip Hejsek
  2026-10-16 23:09 ` [PATCH v3 1/2] char-stdio: avoid closing " Filip Hejsek
  2026-10-16 23:09 ` [PATCH v3 2/2] io: report a warning when a file channel is created from a stdio fd Filip Hejsek
@ 2026-10-20 13:44 ` marcandre.lureau
  2 siblings, 0 replies; 4+ messages in thread
From: marcandre.lureau @ 2026-10-20 13:44 UTC (permalink / raw)
  To: Filip Hejsek
  Cc: qemu-devel, Daniel P. Berrangé, Marc-André Lureau,
	Paolo Bonzini

On Sat, 17 Oct 2026 01:09:07 +0200, Filip Hejsek <filip.hejsek@gmail.com> wrote:
> io: make sure we don't close stdio fds
> 
> 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
> 
> [...]

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

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



^ permalink raw reply	[flat|nested] 4+ messages in thread

end of thread, other threads:[~2026-10-20 13:44 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-10-16 23:09 [PATCH v3 0/2] io: make sure we don't close stdio fds Filip Hejsek
2026-10-16 23:09 ` [PATCH v3 1/2] char-stdio: avoid closing " Filip Hejsek
2026-10-16 23:09 ` [PATCH v3 2/2] io: report a warning when a file channel is created from a stdio fd Filip Hejsek
2026-10-20 13:44 ` [PATCH v3 0/2] io: make sure we don't close stdio fds marcandre.lureau

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.