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_-_@email.froward.int.ebiederm.org
Cc: Brendan Higgins brendan.higgins@linux.dev Cc: David Gow davidgow@google.com Cc: "Eric W. Biederman" ebiederm@xmission.com Cc: Petr Skocik pskocik@gmail.com Cc: Oleg Nesterov oleg@redhat.com Cc: Peter Zijlstra peterz@infradead.org Cc: Thomas Gleixner tglx@linutronix.de Cc: Jens Axboe axboe@kernel.dk Cc: linux-kselftest@vger.kernel.org Cc: kunit-dev@googlegroups.com Signed-off-by: Kees Cook keescook@chromium.org --- 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 */