public inbox for linux-kernel@vger.kernel.org
 help / color / mirror / Atom feed
From: Kees Cook <keescook@chromium.org>
To: Brendan Higgins <brendan.higgins@linux.dev>
Cc: Kees Cook <keescook@chromium.org>,
	David Gow <davidgow@google.com>,
	"Eric W. Biederman" <ebiederm@xmission.com>,
	Petr Skocik <pskocik@gmail.com>, Oleg Nesterov <oleg@redhat.com>,
	Peter Zijlstra <peterz@infradead.org>,
	Thomas Gleixner <tglx@linutronix.de>,
	Jens Axboe <axboe@kernel.dk>,
	linux-kselftest@vger.kernel.org, kunit-dev@googlegroups.com,
	Frederic Weisbecker <frederic@kernel.org>,
	Dmitry Vyukov <dvyukov@google.com>,
	Mike Christie <michael.christie@oracle.com>,
	Marco Elver <elver@google.com>,
	Luis Chamberlain <mcgrof@kernel.org>,
	Al Viro <viro@zeniv.linux.org.uk>,
	"haifeng.xu" <haifeng.xu@shopee.com>,
	Mathieu Desnoyers <mathieu.desnoyers@efficios.com>,
	linux-kernel@vger.kernel.org, linux-hardening@vger.kernel.org
Subject: [PATCH] [RFC] signal: Add KUnit tests
Date: Mon, 14 Aug 2023 14:05:12 -0700	[thread overview]
Message-ID: <20230814210508.never.871-kees@kernel.org> (raw)

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 */
-- 
2.34.1


             reply	other threads:[~2023-08-14 21:06 UTC|newest]

Thread overview: 4+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2023-08-14 21:05 Kees Cook [this message]
2023-08-17  4:08 ` [PATCH] [RFC] signal: Add KUnit tests Eric W. Biederman
2023-08-17 16:17   ` Oleg Nesterov
2023-08-17 16:57   ` Kees Cook

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20230814210508.never.871-kees@kernel.org \
    --to=keescook@chromium.org \
    --cc=axboe@kernel.dk \
    --cc=brendan.higgins@linux.dev \
    --cc=davidgow@google.com \
    --cc=dvyukov@google.com \
    --cc=ebiederm@xmission.com \
    --cc=elver@google.com \
    --cc=frederic@kernel.org \
    --cc=haifeng.xu@shopee.com \
    --cc=kunit-dev@googlegroups.com \
    --cc=linux-hardening@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-kselftest@vger.kernel.org \
    --cc=mathieu.desnoyers@efficios.com \
    --cc=mcgrof@kernel.org \
    --cc=michael.christie@oracle.com \
    --cc=oleg@redhat.com \
    --cc=peterz@infradead.org \
    --cc=pskocik@gmail.com \
    --cc=tglx@linutronix.de \
    --cc=viro@zeniv.linux.org.uk \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox