[PATCH] [RFC] signal: Add KUnit tests

From: Kees Cook
Date: Mon Aug 14 2023 - 17:06:29 EST


This is a continuation of the proposal[1] for mocking init_task for
KUnit testing. Changing the behavior of kill_something_info() is moving
forward[2] and I'd _really_ like to have some unit tests in place to
actually test the behavioral changes.

I tried to incorporate feedback from Daniel and David, and I think the
result is fairly workable -- the only tricky part is building valid
task_struct instances. :)

Notably, I haven't actually gotten as far as testing the actual proposed
behavioral change since I wanted to make sure this approach wasn't going
to totally crash and burn.

Thoughts?

[1] https://lore.kernel.org/all/202212012008.D6F6109@keescook/
[2] https://lore.kernel.org/all/87jzu12pjh.fsf_-_@xxxxxxxxxxxxxxxxxxxxxxxxxxxxxx

Cc: Brendan Higgins <brendan.higgins@xxxxxxxxx>
Cc: David Gow <davidgow@xxxxxxxxxx>
Cc: "Eric W. Biederman" <ebiederm@xxxxxxxxxxxx>
Cc: Petr Skocik <pskocik@xxxxxxxxx>
Cc: Oleg Nesterov <oleg@xxxxxxxxxx>
Cc: Peter Zijlstra <peterz@xxxxxxxxxxxxx>
Cc: Thomas Gleixner <tglx@xxxxxxxxxxxxx>
Cc: Jens Axboe <axboe@xxxxxxxxx>
Cc: linux-kselftest@xxxxxxxxxxxxxxx
Cc: kunit-dev@xxxxxxxxxxxxxxxx
Signed-off-by: Kees Cook <keescook@xxxxxxxxxxxx>
---
include/kunit/resource.h | 15 ++++
include/linux/sched/signal.h | 11 ++-
kernel/signal.c | 135 +++++++++++++++++++++++++++++++++++
3 files changed, 158 insertions(+), 3 deletions(-)

diff --git a/include/kunit/resource.h b/include/kunit/resource.h
index c7383e90f5c9..dbf84a58f7a6 100644
--- a/include/kunit/resource.h
+++ b/include/kunit/resource.h
@@ -479,4 +479,19 @@ void kunit_remove_action(struct kunit *test,
void kunit_release_action(struct kunit *test,
kunit_action_t *action,
void *ctx);
+
+#define kunit_get_mock_pointer(name, actual) ({ \
+ typeof(*(actual)) *ptr = actual; \
+ struct kunit_resource *resource; \
+ \
+ if (kunit_get_current_test()) { \
+ resource = kunit_find_named_resource(current->kunit_test, name); \
+ if (resource) { \
+ ptr = resource->data; \
+ kunit_put_resource(resource); \
+ } \
+ } \
+ ptr; \
+})
+
#endif /* _KUNIT_RESOURCE_H */
diff --git a/include/linux/sched/signal.h b/include/linux/sched/signal.h
index 669e8cff40c7..700271f43491 100644
--- a/include/linux/sched/signal.h
+++ b/include/linux/sched/signal.h
@@ -637,14 +637,19 @@ static inline unsigned long sigsp(unsigned long sp, struct ksignal *ksig)
extern void __cleanup_sighand(struct sighand_struct *);
extern void flush_itimer_signals(void);

+/* This is used for KUnit mocking. */
+#ifndef __init_task_ptr
+#define __init_task_ptr (&init_task)
+#endif
+
#define tasklist_empty() \
- list_empty(&init_task.tasks)
+ list_empty(&__init_task_ptr->tasks)

#define next_task(p) \
list_entry_rcu((p)->tasks.next, struct task_struct, tasks)

#define for_each_process(p) \
- for (p = &init_task ; (p = next_task(p)) != &init_task ; )
+ for (p = __init_task_ptr ; (p = next_task(p)) != __init_task_ptr ; )

extern bool current_is_single_threaded(void);

@@ -653,7 +658,7 @@ extern bool current_is_single_threaded(void);
* 'break' will not work as expected - use goto instead.
*/
#define do_each_thread(g, t) \
- for (g = t = &init_task ; (g = t = next_task(g)) != &init_task ; ) do
+ for (g = t = __init_task_ptr ; (g = t = next_task(g)) != __init_task_ptr ; ) do

#define while_each_thread(g, t) \
while ((t = next_thread(t)) != g)
diff --git a/kernel/signal.c b/kernel/signal.c
index b5370fe5c198..7607d302ebb9 100644
--- a/kernel/signal.c
+++ b/kernel/signal.c
@@ -11,6 +11,14 @@
* to allow signals to be sent reliably.
*/

+#if IS_ENABLED(CONFIG_KUNIT)
+/* This must be defined before we include include/linux/sched/signal.h */
+#define __init_task_ptr kunit_get_mock_pointer("mock_init_task", &init_task)
+
+#include <kunit/resource.h>
+#include <kunit/test-bug.h>
+#endif
+
#include <linux/slab.h>
#include <linux/export.h>
#include <linux/init.h>
@@ -4842,3 +4850,130 @@ void kdb_send_sig(struct task_struct *t, int sig)
kdb_printf("Signal %d is sent to process %d.\n", sig, t->pid);
}
#endif /* CONFIG_KGDB_KDB */
+
+#if IS_ENABLED(CONFIG_KUNIT)
+static void test_empty_task_list(struct kunit *test)
+{
+ struct kunit_resource resource;
+ static struct task_struct empty_task_list = {
+ .tasks = LIST_HEAD_INIT(empty_task_list.tasks),
+ };
+ struct task_struct *p;
+ int count = 0;
+
+ kunit_add_named_resource(test, NULL, NULL, &resource,
+ "mock_init_task", &empty_task_list);
+
+ KUNIT_EXPECT_TRUE(test, tasklist_empty());
+
+ for_each_process(p)
+ count++;
+
+ /* System hangs without this... */
+ kunit_remove_resource(test, &resource);
+
+ KUNIT_EXPECT_EQ(test, count, 0);
+}
+
+static void test_for_each_process(struct kunit *test)
+{
+ struct kunit_resource resource;
+ static struct task_struct task1 = {
+ .pid = 1,
+ .tasks = LIST_HEAD_INIT(task1.tasks),
+ };
+ static struct task_struct task2 = {
+ .pid = 2,
+ }, task3 = {
+ .pid = 3,
+ };
+ struct task_struct *p;
+ int count = 0;
+
+ list_add(&task2.tasks, &task1.tasks);
+ list_add(&task3.tasks, &task1.tasks);
+
+ kunit_add_named_resource(test, NULL, NULL, &resource,
+ "mock_init_task", &task1);
+
+ /* Walk the process list backwards. */
+ for_each_process(p) {
+ KUNIT_EXPECT_EQ(test, 3 - count, p->pid);
+ count++;
+ }
+
+ /* System hangs without this... */
+ kunit_remove_resource(test, &resource);
+
+ /* init_task isn't counted... */
+ KUNIT_EXPECT_EQ(test, count, 2);
+}
+
+static void test_kill_something_info(struct kunit *test)
+{
+ struct kunit_resource resource;
+ static struct task_struct task1 = {
+ .pid = 1,
+ .tasks = LIST_HEAD_INIT(task1.tasks),
+ };
+ static struct task_struct task2 = {
+ .pid = 2,
+ }, task3 = {
+ .pid = 3,
+ };
+ struct kernel_siginfo siginfo = {
+ .si_code = SI_KERNEL,
+ };
+ struct task_struct *p;
+ int count = 0;
+
+ list_add(&task2.tasks, &task1.tasks);
+ list_add(&task3.tasks, &task1.tasks);
+
+ kunit_add_named_resource(test, NULL, NULL, &resource,
+ "mock_init_task", &task1);
+
+ /* Make sure we have a process list. */
+ for_each_process(p)
+ count++;
+ KUNIT_EXPECT_EQ(test, count, 2);
+
+ /* INT_MIN pid must return ESRCH */
+ KUNIT_EXPECT_EQ(test, -ESRCH,
+ kill_something_info(SIGHUP, SEND_SIG_NOINFO, INT_MIN));
+
+ /* Invalid signal: EINVAL */
+ KUNIT_EXPECT_EQ(test, -EINVAL,
+ kill_something_info(_NSIG + 1, SEND_SIG_NOINFO, 2));
+
+ /* Missing pid: ESRCH */
+ KUNIT_EXPECT_EQ(test, -ESRCH,
+ kill_something_info(SIGHUP, SEND_SIG_NOINFO, 42));
+
+ /* Bypass permission checks with SEND_SIG_NOINFO. */
+ KUNIT_EXPECT_EQ(test, 0,
+ kill_something_info(SIGHUP, SEND_SIG_NOINFO, 2));
+
+ /* XXX: Hm, I was expecting this to explode in cred deref... */
+ KUNIT_EXPECT_EQ(test, 0,
+ kill_something_info(SIGHUP, &siginfo, 3));
+
+ /* XXX more tests here, perhaps after mocking out group_send_sig_info() ... */
+
+ /* System hangs without this... */
+ kunit_remove_resource(test, &resource);
+}
+
+static struct kunit_case test_cases[] = {
+ KUNIT_CASE(test_empty_task_list),
+ KUNIT_CASE(test_for_each_process),
+ KUNIT_CASE(test_kill_something_info),
+ {}
+};
+
+static struct kunit_suite test_suite = {
+ .name = "signal",
+ .test_cases = test_cases,
+};
+kunit_test_suite(test_suite);
+#endif /* CONFIG_KUNIT */
--
2.34.1