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
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 <[email protected]>
---
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
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 <[email protected]>
---
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
Just like with O_NONBLOCK, make sure UML clears O_ASYNC
upon exit when it set it.
Signed-off-by: Richard Weinberger <[email protected]>
---
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
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
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