[PATCH 0/4] um: Assorted console related fixes

[PATCH 0/4] um: Assorted console related fixes

[Richard Weinberger]

When UML terminates (gracefully or panic) it does not always
cleanup on the host side.
Especially when fd:0,fd:1 is used as console it can leave a misconfigured
stdin/out on the host side.
Also stale processes such as port-helper can left behind.

This series fixes the console exit code to make sure UML
undoes everything it changed on the host side.

Richard Weinberger (4):
  um: Run console exit code also upon kernel panic
  um: os_set_fd_block: Return old blocking mode
  um: chan: Restore blocking mode upon exit
  um: irq: Restore O_ASYNC upon exit

 arch/um/drivers/chan.h               |  1 +
 arch/um/drivers/chan_kern.c          | 11 +++++++----
 arch/um/drivers/chan_user.c          |  2 +-
 arch/um/drivers/mconsole_kern.c      |  2 +-
 arch/um/drivers/stdio_console.c      |  4 ++--
 arch/um/drivers/ubd_user.c           |  2 +-
 arch/um/drivers/xterm.c              |  2 +-
 arch/um/include/shared/uml_console.h | 11 +++++++++++
 arch/um/kernel/irq.c                 |  5 ++++-
 arch/um/kernel/um_arch.c             |  8 +++++---
 arch/um/os-Linux/file.c              |  5 ++++-
 11 files changed, 38 insertions(+), 15 deletions(-)
 create mode 100644 arch/um/include/shared/uml_console.h

-- 
2.26.2

[PATCH 1/4] um: Run console exit code also upon kernel panic

[Richard Weinberger]

Make sure that UML's console exit handler run also when the kernel
crashes.
That way settings and resources can get restored.

Signed-off-by: Richard Weinberger <richard@nod.at>
---
 arch/um/drivers/stdio_console.c      |  4 ++--
 arch/um/include/shared/uml_console.h | 11 +++++++++++
 arch/um/kernel/um_arch.c             |  8 +++++---
 3 files changed, 18 insertions(+), 5 deletions(-)
 create mode 100644 arch/um/include/shared/uml_console.h

diff --git a/arch/um/drivers/stdio_console.c b/arch/um/drivers/stdio_console.c
index e8b762f4d8c2..06612c3b6240 100644
--- a/arch/um/drivers/stdio_console.c
+++ b/arch/um/drivers/stdio_console.c
@@ -180,13 +180,13 @@ static int stdio_init(void)
 }
 late_initcall(stdio_init);
 
-static void console_exit(void)
+void uml_console_exit(void)
 {
 	if (!con_init_done)
 		return;
 	close_lines(vts, ARRAY_SIZE(vts));
 }
-__uml_exitcall(console_exit);
+__uml_exitcall(uml_console_exit);
 
 static int console_chan_setup(char *str)
 {
diff --git a/arch/um/include/shared/uml_console.h b/arch/um/include/shared/uml_console.h
new file mode 100644
index 000000000000..aa8d02fe5e90
--- /dev/null
+++ b/arch/um/include/shared/uml_console.h
@@ -0,0 +1,11 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+/*
+ * Copyright (C) 2022 Richard Weinberger <richard@nod.at>
+ */
+
+#ifndef UML_CONSOLE_H
+#define UML_CONSOLE_H
+
+extern void uml_console_exit(void);
+
+#endif /* UML_CONSOLE_H */
diff --git a/arch/um/kernel/um_arch.c b/arch/um/kernel/um_arch.c
index abceeabe29b9..0bdae61929ef 100644
--- a/arch/um/kernel/um_arch.c
+++ b/arch/um/kernel/um_arch.c
@@ -17,17 +17,18 @@
 #include <linux/kmsg_dump.h>
 #include <linux/suspend.h>
 
-#include <asm/processor.h>
+#include <arch.h>
+#include <as-layout.h>
 #include <asm/cpufeature.h>
+#include <asm/processor.h>
 #include <asm/sections.h>
 #include <asm/setup.h>
-#include <as-layout.h>
-#include <arch.h>
 #include <init.h>
 #include <kern.h>
 #include <kern_util.h>
 #include <mem_user.h>
 #include <os.h>
+#include <uml_console.h>
 
 #include "um_arch.h"
 
@@ -245,6 +246,7 @@ static int panic_exit(struct notifier_block *self, unsigned long unused1,
 	bust_spinlocks(1);
 	bust_spinlocks(0);
 	uml_exitcode = 1;
+	uml_console_exit();
 	os_dump_core();
 	return 0;
 }
-- 
2.26.2

Re: [PATCH 1/4] um: Run console exit code also upon kernel panic

[Johannes Berg]

On Sat, 2022-01-01 at 22:58 +0100, Richard Weinberger wrote:
> 
> diff --git a/arch/um/include/shared/uml_console.h b/arch/um/include/shared/uml_console.h
> new file mode 100644
> index 000000000000..aa8d02fe5e90
> --- /dev/null
> +++ b/arch/um/include/shared/uml_console.h
> @@ -0,0 +1,11 @@
> +/* SPDX-License-Identifier: GPL-2.0 */
> +/*
> + * Copyright (C) 2022 Richard Weinberger <richard@nod.at>
> + */
> +
> +#ifndef UML_CONSOLE_H
> +#define UML_CONSOLE_H
> +
> +extern void uml_console_exit(void);
> 

Not sure it's worth adding a whole new file just for this, but also
doesn't really matter :)

johannes

[PATCH 2/4] um: os_set_fd_block: Return old blocking mode

[Richard Weinberger]

This will be needed when we restore the old mode upon exit.
The function now returns < 0 in case of an error, 1 if O_NONBLOCK
was set, 0 otherwise.

Signed-off-by: Richard Weinberger <richard@nod.at>
---
 arch/um/drivers/chan_kern.c     | 8 ++++----
 arch/um/drivers/chan_user.c     | 2 +-
 arch/um/drivers/mconsole_kern.c | 2 +-
 arch/um/drivers/ubd_user.c      | 2 +-
 arch/um/drivers/xterm.c         | 2 +-
 arch/um/os-Linux/file.c         | 5 ++++-
 6 files changed, 12 insertions(+), 9 deletions(-)

diff --git a/arch/um/drivers/chan_kern.c b/arch/um/drivers/chan_kern.c
index 62997055c454..86be115b5890 100644
--- a/arch/um/drivers/chan_kern.c
+++ b/arch/um/drivers/chan_kern.c
@@ -83,7 +83,7 @@ static const struct chan_ops not_configged_ops = {
 
 static int open_one_chan(struct chan *chan)
 {
-	int fd, err;
+	int fd, ret;
 
 	if (chan->opened)
 		return 0;
@@ -95,10 +95,10 @@ static int open_one_chan(struct chan *chan)
 	if (fd < 0)
 		return fd;
 
-	err = os_set_fd_block(fd, 0);
-	if (err) {
+	ret = os_set_fd_block(fd, 0);
+	if (ret < 0) {
 		(*chan->ops->close)(fd, chan->data);
-		return err;
+		return ret;
 	}
 
 	chan->fd = fd;
diff --git a/arch/um/drivers/chan_user.c b/arch/um/drivers/chan_user.c
index 6040817c036f..b449656cd5f8 100644
--- a/arch/um/drivers/chan_user.c
+++ b/arch/um/drivers/chan_user.c
@@ -257,7 +257,7 @@ static int winch_tramp(int fd, struct tty_port *port, int *fd_out,
 	}
 
 	err = os_set_fd_block(*fd_out, 0);
-	if (err) {
+	if (err < 0) {
 		printk(UM_KERN_ERR "winch_tramp: failed to set thread_fd "
 		       "non-blocking.\n");
 		goto out_close;
diff --git a/arch/um/drivers/mconsole_kern.c b/arch/um/drivers/mconsole_kern.c
index 6ead1e240457..8000914f9cd4 100644
--- a/arch/um/drivers/mconsole_kern.c
+++ b/arch/um/drivers/mconsole_kern.c
@@ -732,7 +732,7 @@ static int __init mconsole_init(void)
 		printk(KERN_ERR "Failed to initialize management console\n");
 		return 1;
 	}
-	if (os_set_fd_block(sock, 0))
+	if (os_set_fd_block(sock, 0) < 0)
 		goto out;
 
 	register_reboot_notifier(&reboot_notifier);
diff --git a/arch/um/drivers/ubd_user.c b/arch/um/drivers/ubd_user.c
index a1afe414ce48..a930a254826b 100644
--- a/arch/um/drivers/ubd_user.c
+++ b/arch/um/drivers/ubd_user.c
@@ -42,7 +42,7 @@ int start_io_thread(unsigned long sp, int *fd_out)
 
 	err = os_set_fd_block(*fd_out, 0);
 	err = os_set_fd_block(kernel_fd, 0);
-	if (err) {
+	if (err < 0) {
 		printk("start_io_thread - failed to set nonblocking I/O.\n");
 		goto out_close;
 	}
diff --git a/arch/um/drivers/xterm.c b/arch/um/drivers/xterm.c
index 87ca4a47cd66..01be81c25266 100644
--- a/arch/um/drivers/xterm.c
+++ b/arch/um/drivers/xterm.c
@@ -161,7 +161,7 @@ static int xterm_open(int input, int output, int primary, void *d,
 	}
 
 	err = os_set_fd_block(new, 0);
-	if (err) {
+	if (err < 0) {
 		printk(UM_KERN_ERR "xterm_open : failed to set xterm "
 		       "descriptor non-blocking, err = %d\n", -err);
 		goto out_close2;
diff --git a/arch/um/os-Linux/file.c b/arch/um/os-Linux/file.c
index e4421dbc4c36..af2b3cba2c93 100644
--- a/arch/um/os-Linux/file.c
+++ b/arch/um/os-Linux/file.c
@@ -445,11 +445,14 @@ int os_clear_fd_async(int fd)
 int os_set_fd_block(int fd, int blocking)
 {
 	int flags;
+	int oldmode;
 
 	flags = fcntl(fd, F_GETFL);
 	if (flags < 0)
 		return -errno;
 
+	oldmode = !!(flags & O_NONBLOCK);
+
 	if (blocking)
 		flags &= ~O_NONBLOCK;
 	else
@@ -458,7 +461,7 @@ int os_set_fd_block(int fd, int blocking)
 	if (fcntl(fd, F_SETFL, flags) < 0)
 		return -errno;
 
-	return 0;
+	return oldmode;
 }
 
 int os_accept_connection(int fd)
-- 
2.26.2

Re: [PATCH 2/4] um: os_set_fd_block: Return old blocking mode

[Johannes Berg]

On Sat, 2022-01-01 at 22:58 +0100, Richard Weinberger wrote:
> This will be needed when we restore the old mode upon exit.
> The function now returns < 0 in case of an error, 1 if O_NONBLOCK
> was set, 0 otherwise.
> 
> +++ b/arch/um/drivers/chan_user.c
> @@ -257,7 +257,7 @@ static int winch_tramp(int fd, struct tty_port *port, int *fd_out,
>  	}
>  
>  	err = os_set_fd_block(*fd_out, 0);
> -	if (err) {
> +	if (err < 0) {
>  		printk(UM_KERN_ERR "winch_tramp: failed to set thread_fd "
>  		       "non-blocking.\n");
>  		goto out_close;

This does 'return err' afterwards, which then changes the logic from 0
to 1, which seems wrong - especially given that it gets passed further
elsewhere as a pid, which seems very questionable in the first place,
but at least 1 would be valid pid unlike 0?

IOW, I think you need

-	return err;
+	return 0;

a couple of lines later.

johannes

[PATCH 3/4] um: chan: Restore blocking mode upon exit

[Richard Weinberger]

Make sure that O_NONBLOCK is cleared upon exit if UML sets it.
Otherwise stdin/out behaves strange after UML terminates.

Signed-off-by: Richard Weinberger <richard@nod.at>
---
 arch/um/drivers/chan.h      | 1 +
 arch/um/drivers/chan_kern.c | 3 +++
 2 files changed, 4 insertions(+)

diff --git a/arch/um/drivers/chan.h b/arch/um/drivers/chan.h
index c37cc4f26f91..6b8c462f146e 100644
--- a/arch/um/drivers/chan.h
+++ b/arch/um/drivers/chan.h
@@ -22,6 +22,7 @@ struct chan {
 	unsigned int output:1;
 	unsigned int opened:1;
 	unsigned int enabled:1;
+	unsigned int fd_blocking:1;
 	int fd;
 	const struct chan_ops *ops;
 	void *data;
diff --git a/arch/um/drivers/chan_kern.c b/arch/um/drivers/chan_kern.c
index 86be115b5890..6d512589fa50 100644
--- a/arch/um/drivers/chan_kern.c
+++ b/arch/um/drivers/chan_kern.c
@@ -102,6 +102,7 @@ static int open_one_chan(struct chan *chan)
 	}
 
 	chan->fd = fd;
+	chan->fd_blocking = ret;
 
 	chan->opened = 1;
 	return 0;
@@ -223,6 +224,8 @@ static void close_one_chan(struct chan *chan, int delay_free_irq)
 	if (chan->ops->close != NULL)
 		(*chan->ops->close)(chan->fd, chan->data);
 
+	os_set_fd_block(chan->fd, chan->fd_blocking);
+
 	chan->opened = 0;
 	chan->fd = -1;
 }
-- 
2.26.2

[PATCH 4/4] um: irq: Restore O_ASYNC upon exit

[Richard Weinberger]

Just like with O_NONBLOCK, make sure UML clears O_ASYNC
upon exit when it set it.

Signed-off-by: Richard Weinberger <richard@nod.at>
---
 arch/um/kernel/irq.c | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/arch/um/kernel/irq.c b/arch/um/kernel/irq.c
index a8873d9bc28b..e020bdccfa23 100644
--- a/arch/um/kernel/irq.c
+++ b/arch/um/kernel/irq.c
@@ -342,6 +342,7 @@ static void free_irq_by_irq_and_dev(unsigned int irq, void *dev)
 				continue;
 
 			os_del_epoll_fd(entry->fd);
+			os_clear_fd_async(entry->fd);
 			reg->events = 0;
 			update_or_free_irq_entry(entry);
 			goto out;
@@ -396,8 +397,10 @@ int deactivate_all_fds(void)
 	os_set_ioignore();
 
 	/* we can no longer call kfree() here so just deactivate */
-	list_for_each_entry(entry, &active_fds, list)
+	list_for_each_entry(entry, &active_fds, list) {
 		os_del_epoll_fd(entry->fd);
+		os_clear_fd_async(entry->fd);
+	}
 	os_close_epoll_fd();
 	return 0;
 }
-- 
2.26.2

Re: [PATCH 4/4] um: irq: Restore O_ASYNC upon exit

[Johannes Berg]

On Sat, 2022-01-01 at 22:58 +0100, Richard Weinberger wrote:
> Just like with O_NONBLOCK, make sure UML clears O_ASYNC
> upon exit when it set it.
> 

"when it set it" isn't really quite true - you always clear it, should
it be similar to the O_NONBLOCK version to see the old mode?

johannes