[PATCH stable 6.12 3/9] um: Use os_set_pdeathsig helper in winch thread/process

[Florian Fainelli]

From: Tiwei Bie <tiwe...@antgroup.com>

commit 42b8b00c8ab1ac18fccde3f29ee589626a561ea7 upstream

Since we have a helper now, let's switch to using it. It will make
the code slightly more consistent.

Signed-off-by: Tiwei Bie <tiwe...@antgroup.com>
Link: https://patch.msgid.link/20241024142828.26...@antgroup.com
Signed-off-by: Johannes Berg <johann...@intel.com>
Signed-off-by: Florian Fainelli <florian....@broadcom.com>
---
arch/um/drivers/chan_user.c | 3 +--
1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/arch/um/drivers/chan_user.c b/arch/um/drivers/chan_user.c
index 1434114b2f34..35f9beeb19b3 100644
--- a/arch/um/drivers/chan_user.c
+++ b/arch/um/drivers/chan_user.c
@@ -10,7 +10,6 @@
#include <signal.h>
#include <termios.h>
#include <sys/ioctl.h>
-#include <sys/prctl.h>
#include "chan_user.h"
#include <os.h>
#include <um_malloc.h>
@@ -162,7 +161,7 @@ static __noreturn int winch_thread(void *arg)
int count;
char c = 1;

- prctl(PR_SET_PDEATHSIG, SIGKILL);
+ os_set_pdeathsig();

pty_fd = data->pty_fd;
pipe_fd = data->pipe_fd;
--
2.34.1

[Florian Fainelli]

This patch series addresses an issue we encountered on older Ubuntu
20.04.6 LTS servers whereby running Kunit would not return to a prompt
and it would not be possible to Ctrl-C the running session either.

Benjamin Berg (2):
um: Set parent death signal for winch thread/process
um: Set parent death signal for userspace process

David Gow (1):
kunit: tool: Terminate kernel under test on SIGINT

Shuvam Pandey (1):
kunit: tool: skip stty when stdin is not a tty

Tiwei Bie (5):
um: Add os_set_pdeathsig helper function
um: Use os_set_pdeathsig helper in winch thread/process
um: Set parent-death signal for ubd io thread/process
um: Set parent-death signal for write_sigio thread/process
um: Preserve errno within signal handler

arch/um/drivers/chan_user.c | 2 ++
arch/um/drivers/ubd_kern.c | 1 +
arch/um/include/shared/os.h | 2 ++
arch/um/os-Linux/process.c | 6 ++++
arch/um/os-Linux/sigio.c | 1 +
arch/um/os-Linux/signal.c | 3 ++
arch/um/os-Linux/skas/process.c | 3 ++
tools/testing/kunit/kunit_kernel.py | 39 +++++++++++++++++-------
tools/testing/kunit/kunit_tool_test.py | 42 ++++++++++++++++++++++++++
9 files changed, 88 insertions(+), 11 deletions(-)

--
2.34.1

[Florian Fainelli]

From: Tiwei Bie <tiwe...@antgroup.com>

commit 4e5adbe447db382cc76e05613581f96aef4f91d2 upstream

This helper can be used to set the parent-death signal of the calling
process to SIGKILL to ensure that the process will be killed if the
UML kernel dies unexpectedly without proper cleanup. This helper will
be used in the follow-up patches.

Signed-off-by: Tiwei Bie <tiwe...@antgroup.com>
Link: https://patch.msgid.link/20241024142828.26...@antgroup.com
Signed-off-by: Johannes Berg <johann...@intel.com>
Signed-off-by: Florian Fainelli <florian....@broadcom.com>
---

arch/um/include/shared/os.h | 2 ++
arch/um/os-Linux/process.c | 6 ++++++
2 files changed, 8 insertions(+)

diff --git a/arch/um/include/shared/os.h b/arch/um/include/shared/os.h
index 77a8593f219a..2a0843cc2208 100644
--- a/arch/um/include/shared/os.h
+++ b/arch/um/include/shared/os.h
@@ -219,6 +219,8 @@ extern int os_unmap_memory(void *addr, int len);
extern int os_drop_memory(void *addr, int length);
extern int can_drop_memory(void);

+void os_set_pdeathsig(void);
+
/* execvp.c */
extern int execvp_noalloc(char *buf, const char *file, char *const argv[]);
/* helper.c */
diff --git a/arch/um/os-Linux/process.c b/arch/um/os-Linux/process.c
index 2686120ab232..4015d3dc3958 100644
--- a/arch/um/os-Linux/process.c
+++ b/arch/um/os-Linux/process.c
@@ -12,6 +12,7 @@
#include <fcntl.h>
#include <sys/mman.h>
#include <sys/ptrace.h>
+#include <sys/prctl.h>
#include <sys/wait.h>
#include <asm/unistd.h>
#include <init.h>
@@ -234,3 +235,8 @@ void init_new_thread_signals(void)
set_handler(SIGIO);
signal(SIGWINCH, SIG_IGN);
}
+
+void os_set_pdeathsig(void)
+{
+ prctl(PR_SET_PDEATHSIG, SIGKILL);
+}
--
2.34.1

[Florian Fainelli]

From: Tiwei Bie <tiwe...@antgroup.com>

commit c6c4adee65969218b0b7b13f568fd2c6f2333373 upstream

The write_sigio thread is not really a traditional thread. Set
the parent-death signal for it to ensure that it will be killed

if the UML kernel dies unexpectedly without proper cleanup.

Signed-off-by: Tiwei Bie <tiwe...@antgroup.com>
Link: https://patch.msgid.link/20241024142828.26...@antgroup.com
Signed-off-by: Johannes Berg <johann...@intel.com>
Signed-off-by: Florian Fainelli <florian....@broadcom.com>
---

arch/um/os-Linux/sigio.c | 1 +
1 file changed, 1 insertion(+)

diff --git a/arch/um/os-Linux/sigio.c b/arch/um/os-Linux/sigio.c
index 9e71794839e8..9aac8def4d63 100644
--- a/arch/um/os-Linux/sigio.c
+++ b/arch/um/os-Linux/sigio.c
@@ -55,6 +55,7 @@ static int write_sigio_thread(void *unused)
int i, n, respond_fd;
char c;

+ os_set_pdeathsig();
os_fix_helper_signals();
fds = &current_poll;
while (1) {
--
2.34.1

[Florian Fainelli]

From: Tiwei Bie <tiwe...@antgroup.com>

commit f68b2d5a907b53eed99cf2efcaaae116df73c298 upstream

We rely on errno to determine whether a syscall has failed, so we
need to ensure that accessing errno is async-signal-safe. Currently,
we preserve the errno in sig_handler_common(), but it doesn't cover
every possible case. Let's do it in hard_handler() instead, which
is the signal handler we actually register.

Signed-off-by: Tiwei Bie <tiwe...@antgroup.com>
Link: https://patch.msgid.link/20260106001228.15...@antgroup.com
Signed-off-by: Johannes Berg <johann...@intel.com>
[florian: hard_handler() in 6.12 retains the to_irq_stack/from_irq_stack
loop from before the upstream SMP refactoring; errno save/restore is wrapped
around that loop rather than replacing sig_handler_common's existing save/restore]

Signed-off-by: Florian Fainelli <florian....@broadcom.com>
---

arch/um/os-Linux/signal.c | 3 +++
1 file changed, 3 insertions(+)

diff --git a/arch/um/os-Linux/signal.c b/arch/um/os-Linux/signal.c
index b11ed66c8bb0..94abc9e3ec7e 100644
--- a/arch/um/os-Linux/signal.c
+++ b/arch/um/os-Linux/signal.c
@@ -190,6 +190,7 @@ static void hard_handler(int sig, siginfo_t *si, void *p)
{
ucontext_t *uc = p;
mcontext_t *mc = &uc->uc_mcontext;
+ int save_errno = errno;
unsigned long pending = 1UL << sig;

do {
@@ -227,6 +228,8 @@ static void hard_handler(int sig, siginfo_t *si, void *p)
if (!nested)
pending = from_irq_stack(nested);
} while (pending);
+
+ errno = save_errno;
}

void set_handler(int sig)
--
2.34.1

[Florian Fainelli]

From: Shuvam Pandey <shuvam...@gmail.com>

commit e42c349f4cdfa43cb39a68c8f764f8cafc23a9a9 upstream

run_kernel() cleanup and signal_handler() invoke stty unconditionally.
When stdin is not a tty (for example in CI or unit tests), this writes
noise to stderr.

Call stty only when stdin is a tty.

Add regression tests for these paths:
- run_kernel() with non-tty stdin
- signal_handler() with non-tty stdin
- signal_handler() with tty stdin

Signed-off-by: Shuvam Pandey <shuvam...@gmail.com>
Reviewed-by: David Gow <da...@davidgow.net>
Signed-off-by: Shuah Khan <sk...@linuxfoundation.org>
[florian: add missing 'import sys' required by sys.stdin.isatty(); the
module was already present in the mainline tree before this commit but
was absent from the 6.12 base of kunit_kernel.py]

Signed-off-by: Florian Fainelli <florian....@broadcom.com>
---

tools/testing/kunit/kunit_kernel.py | 11 +++++--
tools/testing/kunit/kunit_tool_test.py | 42 ++++++++++++++++++++++++++
2 files changed, 51 insertions(+), 2 deletions(-)

diff --git a/tools/testing/kunit/kunit_kernel.py b/tools/testing/kunit/kunit_kernel.py
index 3f2979f2f6ea..c09b66ba734b 100644
--- a/tools/testing/kunit/kunit_kernel.py
+++ b/tools/testing/kunit/kunit_kernel.py
@@ -14,6 +14,7 @@ import os
import shlex
import shutil
import signal
+import sys
import threading
from typing import Iterator, List, Optional, Tuple, Any
from types import FrameType
@@ -333,6 +334,12 @@ class LinuxSourceTree:
return False
return self.validate_config(build_dir)

+ def _restore_terminal_if_tty(self) -> None:
+ # stty requires a controlling terminal; skip headless runs.
+ if sys.stdin is None or not sys.stdin.isatty():
+ return
+ subprocess.call(['stty', 'sane'])
+
def run_kernel(self, args: Optional[List[str]]=None, build_dir: str='', filter_glob: str='', filter: str='', filter_action: Optional[str]=None, timeout: Optional[int]=None) -> Iterator[str]:
# Copy to avoid mutating the caller-supplied list. exec_tests() reuses
# the same args across repeated run_kernel() calls (e.g. --run_isolated),
@@ -380,11 +387,11 @@ class LinuxSourceTree:
output.close()

waiter.join()
- subprocess.call(['stty', 'sane'])
+ self._restore_terminal_if_tty()

def signal_handler(self, unused_sig: int, unused_frame: Optional[FrameType]) -> None:
logging.error('Build interruption occurred. Cleaning console.')
if self._process:
self._process.terminate()
self._process.wait()
- subprocess.call(['stty', 'sane'])
+ self._restore_terminal_if_tty()
diff --git a/tools/testing/kunit/kunit_tool_test.py b/tools/testing/kunit/kunit_tool_test.py
index 70e5d0abe87f..6e6200d52540 100755
--- a/tools/testing/kunit/kunit_tool_test.py
+++ b/tools/testing/kunit/kunit_tool_test.py
@@ -503,6 +503,48 @@ class LinuxSourceTreeTest(unittest.TestCase):
self.assertIn('kunit.filter_glob=suite.test1', start_calls[0])
self.assertIn('kunit.filter_glob=suite.test2', start_calls[1])

+ def test_run_kernel_skips_terminal_reset_without_tty(self):
+ def fake_start(unused_args, unused_build_dir):
+ return subprocess.Popen(['printf', 'KTAP version 1\n'],
+ text=True, stdout=subprocess.PIPE)
+
+ non_tty_stdin = mock.Mock()
+ non_tty_stdin.isatty.return_value = False
+
+ with tempfile.TemporaryDirectory('') as build_dir:
+ tree = kunit_kernel.LinuxSourceTree(build_dir, kunitconfig_paths=[os.devnull])
+ with mock.patch.object(tree._ops, 'start', side_effect=fake_start), \
+ mock.patch.object(kunit_kernel.sys, 'stdin', non_tty_stdin), \
+ mock.patch.object(kunit_kernel.subprocess, 'call') as mock_call:
+ for _ in tree.run_kernel(build_dir=build_dir):
+ pass
+
+ mock_call.assert_not_called()
+
+ def test_signal_handler_skips_terminal_reset_without_tty(self):
+ non_tty_stdin = mock.Mock()
+ non_tty_stdin.isatty.return_value = False
+ tree = kunit_kernel.LinuxSourceTree('', kunitconfig_paths=[os.devnull])
+
+ with mock.patch.object(kunit_kernel.sys, 'stdin', non_tty_stdin), \
+ mock.patch.object(kunit_kernel.subprocess, 'call') as mock_call, \
+ mock.patch.object(kunit_kernel.logging, 'error') as mock_error:
+ tree.signal_handler(signal.SIGINT, None)
+ mock_error.assert_called_once()
+ mock_call.assert_not_called()
+
+ def test_signal_handler_resets_terminal_with_tty(self):
+ tty_stdin = mock.Mock()
+ tty_stdin.isatty.return_value = True
+ tree = kunit_kernel.LinuxSourceTree('', kunitconfig_paths=[os.devnull])
+
+ with mock.patch.object(kunit_kernel.sys, 'stdin', tty_stdin), \
+ mock.patch.object(kunit_kernel.subprocess, 'call') as mock_call, \
+ mock.patch.object(kunit_kernel.logging, 'error') as mock_error:
+ tree.signal_handler(signal.SIGINT, None)
+ mock_error.assert_called_once()
+ mock_call.assert_called_once_with(['stty', 'sane'])
+
def test_build_reconfig_no_config(self):
with tempfile.TemporaryDirectory('') as build_dir:
with open(kunit_kernel.get_kunitconfig_path(build_dir), 'w') as f:
--
2.34.1

[Florian Fainelli]

From: David Gow <da...@davidgow.net>

commit 8f260b02eeeffbf2263c2b82b6e3e32fd73cde2b upstream

kunit.py will attempt to catch SIGINT / ^C in order to ensure the TTY isn't
messed up, but never actually attempts to terminate the running kernel (be
it UML or QEMU). This can lead to a bit of frustration if the kernel has
crashed or hung.

Terminate the kernel process in the signal handler, if it's running. This
requires plumbing through the process handle in a few more places (and
having some checks to see if the kernel is still running in places where it
may have already been killed).

Reported-by: Andy Shevchenko <andriy.s...@intel.com>
Closes: https://lore.kernel.org/all/aaFmiAmg...@smile.fi.intel.com/
Signed-off-by: David Gow <da...@davidgow.net>
Reviewed-by: Andy Shevchenko <andriy.s...@intel.com>
Tested-by: Andy Shevchenko <andriy.s...@intel.com>
Signed-off-by: Shuah Khan <sk...@linuxfoundation.org>
[florian: resolved conflict in signal_handler(): upstream references
_restore_terminal_if_tty() which is introduced by the following commit;
retained subprocess.call(['stty', 'sane']) until that helper is available]

Signed-off-by: Florian Fainelli <florian....@broadcom.com>
---

tools/testing/kunit/kunit_kernel.py | 28 +++++++++++++++++++---------
1 file changed, 19 insertions(+), 9 deletions(-)

diff --git a/tools/testing/kunit/kunit_kernel.py b/tools/testing/kunit/kunit_kernel.py
index 12b0f2ee5665..3f2979f2f6ea 100644
--- a/tools/testing/kunit/kunit_kernel.py
+++ b/tools/testing/kunit/kunit_kernel.py
@@ -15,7 +15,7 @@ import shlex
import shutil
import signal
import threading
-from typing import Iterator, List, Optional, Tuple
+from typing import Iterator, List, Optional, Tuple, Any
from types import FrameType

import kunit_config
@@ -252,6 +252,7 @@ class LinuxSourceTree:
if kconfig_add:
kconfig = kunit_config.parse_from_string('\n'.join(kconfig_add))
self._kconfig.merge_in_entries(kconfig)
+ self._process : Optional[subprocess.Popen[Any]] = None

def arch(self) -> str:
return self._arch
@@ -345,36 +346,45 @@ class LinuxSourceTree:
args.append('kunit.filter_action=' + filter_action)
args.append('kunit.enable=1')

- process = self._ops.start(args, build_dir)
- assert process.stdout is not None # tell mypy it's set
+ self._process = self._ops.start(args, build_dir)
+ assert self._process is not None # tell mypy it's set
+ assert self._process.stdout is not None # tell mypy it's set

# Enforce the timeout in a background thread.
def _wait_proc() -> None:
try:
- process.wait(timeout=timeout)
+ if self._process:
+ self._process.wait(timeout=timeout)
except Exception as e:
print(e)
- process.terminate()
- process.wait()
+ if self._process:
+ self._process.terminate()
+ self._process.wait()
waiter = threading.Thread(target=_wait_proc)
waiter.start()

output = open(get_outfile_path(build_dir), 'w')
try:
# Tee the output to the file and to our caller in real time.
- for line in process.stdout:
+ for line in self._process.stdout:
output.write(line)
yield line
# This runs even if our caller doesn't consume every line.
finally:
# Flush any leftover output to the file
- output.write(process.stdout.read())
+ if self._process:
+ if self._process.stdout:
+ output.write(self._process.stdout.read())
+ self._process.stdout.close()
+ self._process = None
output.close()
- process.stdout.close()

waiter.join()

subprocess.call(['stty', 'sane'])

def signal_handler(self, unused_sig: int, unused_frame: Optional[FrameType]) -> None:
logging.error('Build interruption occurred. Cleaning console.')

+ if self._process:
+ self._process.terminate()
+ self._process.wait()

subprocess.call(['stty', 'sane'])

--
2.34.1

[Florian Fainelli]

From: Tiwei Bie <tiwe...@antgroup.com>

commit 9b5e6c0f5a9199c69af81ac5bedc512ee7dc20b3 upstream

The ubd io thread is not really a traditional thread. Set the

parent-death signal for it to ensure that it will be killed if
the UML kernel dies unexpectedly without proper cleanup.

Signed-off-by: Tiwei Bie <tiwe...@antgroup.com>

Link: https://patch.msgid.link/20241024142828.26...@antgroup.com
Signed-off-by: Johannes Berg <johann...@intel.com>

Signed-off-by: Florian Fainelli <florian....@broadcom.com>
---

arch/um/drivers/ubd_kern.c | 1 +

1 file changed, 1 insertion(+)

diff --git a/arch/um/drivers/ubd_kern.c b/arch/um/drivers/ubd_kern.c
index 2bfb17373244..66c1a8835e36 100644
--- a/arch/um/drivers/ubd_kern.c
+++ b/arch/um/drivers/ubd_kern.c
@@ -1501,6 +1501,7 @@ int io_thread(void *arg)
{
int n, count, written, res;

+ os_set_pdeathsig();
os_fix_helper_signals();

while(1){
--
2.34.1

[Florian Fainelli]

From: Benjamin Berg <benjam...@intel.com>

commit 801e00d3a1b78b7f71675fae79946ff4aa3ee070 upstream

Enable PR_SET_PDEATHSIG so that the UML userspace process will be killed
when the kernel exits unexpectedly.

Signed-off-by: Benjamin Berg <benjam...@intel.com>
Link: https://patch.msgid.link/20240919124511.2...@sipsolutions.net
Signed-off-by: Johannes Berg <johann...@intel.com>
[florian: upstream applies os_set_pdeathsig() in stub_exe.c, a file
introduced in v6.13 that does not exist in 6.12; applied instead to
userspace_tramp() in arch/um/os-Linux/skas/process.c, which is the
equivalent entry-point for the userspace process in 6.12]

Signed-off-by: Florian Fainelli <florian....@broadcom.com>
---

arch/um/os-Linux/skas/process.c | 3 +++

1 file changed, 3 insertions(+)

diff --git a/arch/um/os-Linux/skas/process.c b/arch/um/os-Linux/skas/process.c
index b6f656bcffb1..cca016b0b29b 100644
--- a/arch/um/os-Linux/skas/process.c
+++ b/arch/um/os-Linux/skas/process.c
@@ -214,6 +214,9 @@ static int userspace_tramp(void *stack)
(unsigned long) stub_segv_handler -
(unsigned long) __syscall_stub_start;

+ /* Make sure this process dies if the kernel dies */
+ os_set_pdeathsig();
+
ptrace(PTRACE_TRACEME, 0, 0, 0);

signal(SIGTERM, SIG_DFL);
--
2.34.1