All of lore.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:05 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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.