Module Name:    src
Committed By:   hannken
Date:           Wed Jan 17 10:18:41 UTC 2024

Modified Files:
        src/sys/kern: init_main.c kern_hook.c

Log Message:
Protect kernel hooks exechook, exithook and forkhook with rwlock.
Lock as writer on establish/disestablish and as reader on list traverse.

For exechook ride "exec_lock" as it is already take as reader when
traversing the list.  Add local locks for exithook and forkhook.

Move exec_init before signal_init as signal_init calls exechook_establish()
that needs "exec_lock".

PR kern/39913 "exec, fork, exit hooks need locking"


To generate a diff of this commit:
cvs rdiff -u -r1.546 -r1.547 src/sys/kern/init_main.c
cvs rdiff -u -r1.14 -r1.15 src/sys/kern/kern_hook.c

Please note that diffs are not public domain; they are subject to the
copyright notices on the relevant files.

Modified files:

Index: src/sys/kern/init_main.c
diff -u src/sys/kern/init_main.c:1.546 src/sys/kern/init_main.c:1.547
--- src/sys/kern/init_main.c:1.546	Sat Sep 23 18:21:11 2023
+++ src/sys/kern/init_main.c	Wed Jan 17 10:18:41 2024
@@ -1,4 +1,4 @@
-/*	$NetBSD: init_main.c,v 1.546 2023/09/23 18:21:11 ad Exp $	*/
+/*	$NetBSD: init_main.c,v 1.547 2024/01/17 10:18:41 hannken Exp $	*/
 
 /*-
  * Copyright (c) 2008, 2009, 2019, 2023 The NetBSD Foundation, Inc.
@@ -97,7 +97,7 @@
  */
 
 #include <sys/cdefs.h>
-__KERNEL_RCSID(0, "$NetBSD: init_main.c,v 1.546 2023/09/23 18:21:11 ad Exp $");
+__KERNEL_RCSID(0, "$NetBSD: init_main.c,v 1.547 2024/01/17 10:18:41 hannken Exp $");
 
 #include "opt_cnmagic.h"
 #include "opt_ddb.h"
@@ -407,6 +407,9 @@ main(void)
 	/* Must be called after lwpinit (lwpinit_specificdata) */
 	psref_init();
 
+	/* Initialize exec structures */
+	exec_init(1);		/* signal_init calls exechook_establish() */
+
 	/* Initialize signal-related data structures. */
 	signal_init();
 
@@ -579,9 +582,6 @@ main(void)
 
 	vmem_rehash_start();	/* must be before exec_init */
 
-	/* Initialize exec structures */
-	exec_init(1);		/* seminit calls exithook_establish() */
-
 #if NVERIEXEC > 0
 	/*
 	 * Initialise the Veriexec subsystem.

Index: src/sys/kern/kern_hook.c
diff -u src/sys/kern/kern_hook.c:1.14 src/sys/kern/kern_hook.c:1.15
--- src/sys/kern/kern_hook.c:1.14	Wed Oct 26 23:21:06 2022
+++ src/sys/kern/kern_hook.c	Wed Jan 17 10:18:41 2024
@@ -1,4 +1,4 @@
-/*	$NetBSD: kern_hook.c,v 1.14 2022/10/26 23:21:06 riastradh Exp $	*/
+/*	$NetBSD: kern_hook.c,v 1.15 2024/01/17 10:18:41 hannken Exp $	*/
 
 /*-
  * Copyright (c) 1997, 1998, 1999, 2002, 2007, 2008 The NetBSD Foundation, Inc.
@@ -31,7 +31,7 @@
  */
 
 #include <sys/cdefs.h>
-__KERNEL_RCSID(0, "$NetBSD: kern_hook.c,v 1.14 2022/10/26 23:21:06 riastradh Exp $");
+__KERNEL_RCSID(0, "$NetBSD: kern_hook.c,v 1.15 2024/01/17 10:18:41 hannken Exp $");
 
 #include <sys/param.h>
 
@@ -42,6 +42,7 @@ __KERNEL_RCSID(0, "$NetBSD: kern_hook.c,
 #include <sys/hook.h>
 #include <sys/kmem.h>
 #include <sys/malloc.h>
+#include <sys/once.h>
 #include <sys/rwlock.h>
 #include <sys/systm.h>
 
@@ -74,25 +75,48 @@ struct khook_list {
 
 int	powerhook_debug = 0;
 
+static ONCE_DECL(hook_control);
+static krwlock_t exithook_lock;
+static krwlock_t forkhook_lock;
+
+static int
+hook_init(void)
+{
+
+	rw_init(&exithook_lock);
+	rw_init(&forkhook_lock);
+
+	return 0;
+}
+
 static void *
-hook_establish(hook_list_t *list, void (*fn)(void *), void *arg)
+hook_establish(hook_list_t *list, krwlock_t *lock,
+    void (*fn)(void *), void *arg)
 {
 	struct hook_desc *hd;
 
-	hd = malloc(sizeof(*hd), M_DEVBUF, M_NOWAIT);
-	if (hd == NULL)
-		return (NULL);
+	RUN_ONCE(&hook_control, hook_init);
 
-	hd->hk_fn = fn;
-	hd->hk_arg = arg;
-	LIST_INSERT_HEAD(list, hd, hk_list);
+	hd = malloc(sizeof(*hd), M_DEVBUF, M_NOWAIT);
+	if (hd != NULL) {
+		if (lock)
+			rw_enter(lock, RW_WRITER);
+		hd->hk_fn = fn;
+		hd->hk_arg = arg;
+		LIST_INSERT_HEAD(list, hd, hk_list);
+		if (lock)
+			rw_exit(lock);
+	}
 
 	return (hd);
 }
 
 static void
-hook_disestablish(hook_list_t *list, void *vhook)
+hook_disestablish(hook_list_t *list, krwlock_t *lock, void *vhook)
 {
+
+	if (lock)
+		rw_enter(lock, RW_WRITER);
 #ifdef DIAGNOSTIC
 	struct hook_desc *hd;
 
@@ -106,6 +130,8 @@ hook_disestablish(hook_list_t *list, voi
 #endif
 	LIST_REMOVE((struct hook_desc *)vhook, hk_list);
 	free(vhook, M_DEVBUF);
+	if (lock)
+		rw_exit(lock);
 }
 
 static void
@@ -120,14 +146,20 @@ hook_destroy(hook_list_t *list)
 }
 
 static void
-hook_proc_run(hook_list_t *list, struct proc *p)
+hook_proc_run(hook_list_t *list, krwlock_t *lock, struct proc *p)
 {
 	struct hook_desc *hd;
 
+	RUN_ONCE(&hook_control, hook_init);
+
+	if (lock)
+		rw_enter(lock, RW_READER);
 	LIST_FOREACH(hd, list, hk_list) {
 		__FPTRCAST(void (*)(struct proc *, void *), *hd->hk_fn)(p,
 		    hd->hk_arg);
 	}
+	if (lock)
+		rw_exit(lock);
 }
 
 /*
@@ -146,13 +178,13 @@ static hook_list_t shutdownhook_list = L
 void *
 shutdownhook_establish(void (*fn)(void *), void *arg)
 {
-	return hook_establish(&shutdownhook_list, fn, arg);
+	return hook_establish(&shutdownhook_list, NULL, fn, arg);
 }
 
 void
 shutdownhook_disestablish(void *vhook)
 {
-	hook_disestablish(&shutdownhook_list, vhook);
+	hook_disestablish(&shutdownhook_list, NULL, vhook);
 }
 
 /*
@@ -193,14 +225,14 @@ static hook_list_t mountroothook_list=LI
 void *
 mountroothook_establish(void (*fn)(device_t), device_t dev)
 {
-	return hook_establish(&mountroothook_list, __FPTRCAST(void (*), fn),
-	    dev);
+	return hook_establish(&mountroothook_list, NULL,
+	    __FPTRCAST(void (*), fn), dev);
 }
 
 void
 mountroothook_disestablish(void *vhook)
 {
-	hook_disestablish(&mountroothook_list, vhook);
+	hook_disestablish(&mountroothook_list, NULL, vhook);
 }
 
 void
@@ -227,14 +259,14 @@ static hook_list_t exechook_list = LIST_
 void *
 exechook_establish(void (*fn)(struct proc *, void *), void *arg)
 {
-	return hook_establish(&exechook_list, __FPTRCAST(void (*)(void *), fn),
-	    arg);
+	return hook_establish(&exechook_list, &exec_lock,
+		__FPTRCAST(void (*)(void *), fn), arg);
 }
 
 void
 exechook_disestablish(void *vhook)
 {
-	hook_disestablish(&exechook_list, vhook);
+	hook_disestablish(&exechook_list, &exec_lock, vhook);
 }
 
 /*
@@ -243,7 +275,9 @@ exechook_disestablish(void *vhook)
 void
 doexechooks(struct proc *p)
 {
-	hook_proc_run(&exechook_list, p);
+	KASSERT(rw_lock_held(&exec_lock));
+
+	hook_proc_run(&exechook_list, NULL, p);
 }
 
 static hook_list_t exithook_list = LIST_HEAD_INITIALIZER(exithook_list);
@@ -251,22 +285,16 @@ static hook_list_t exithook_list = LIST_
 void *
 exithook_establish(void (*fn)(struct proc *, void *), void *arg)
 {
-	void *rv;
 
-	rw_enter(&exec_lock, RW_WRITER);
-	rv = hook_establish(&exithook_list, __FPTRCAST(void (*)(void *), fn),
-	    arg);
-	rw_exit(&exec_lock);
-	return rv;
+	return hook_establish(&exithook_list, &exithook_lock,
+	    __FPTRCAST(void (*)(void *), fn), arg);
 }
 
 void
 exithook_disestablish(void *vhook)
 {
 
-	rw_enter(&exec_lock, RW_WRITER);
-	hook_disestablish(&exithook_list, vhook);
-	rw_exit(&exec_lock);
+	hook_disestablish(&exithook_list, &exithook_lock, vhook);
 }
 
 /*
@@ -275,7 +303,7 @@ exithook_disestablish(void *vhook)
 void
 doexithooks(struct proc *p)
 {
-	hook_proc_run(&exithook_list, p);
+	hook_proc_run(&exithook_list, &exithook_lock, p);
 }
 
 static hook_list_t forkhook_list = LIST_HEAD_INITIALIZER(forkhook_list);
@@ -283,14 +311,14 @@ static hook_list_t forkhook_list = LIST_
 void *
 forkhook_establish(void (*fn)(struct proc *, struct proc *))
 {
-	return hook_establish(&forkhook_list, __FPTRCAST(void (*)(void *), fn),
-	    NULL);
+	return hook_establish(&forkhook_list, &forkhook_lock,
+	    __FPTRCAST(void (*)(void *), fn), NULL);
 }
 
 void
 forkhook_disestablish(void *vhook)
 {
-	hook_disestablish(&forkhook_list, vhook);
+	hook_disestablish(&forkhook_list, &forkhook_lock, vhook);
 }
 
 /*
@@ -301,10 +329,14 @@ doforkhooks(struct proc *p2, struct proc
 {
 	struct hook_desc *hd;
 
+	RUN_ONCE(&hook_control, hook_init);
+
+	rw_enter(&forkhook_lock, RW_READER);
 	LIST_FOREACH(hd, &forkhook_list, hk_list) {
 		__FPTRCAST(void (*)(struct proc *, struct proc *), *hd->hk_fn)
 		    (p2, p1);
 	}
+	rw_exit(&forkhook_lock);
 }
 
 static hook_list_t critpollhook_list = LIST_HEAD_INITIALIZER(critpollhook_list);
@@ -312,13 +344,13 @@ static hook_list_t critpollhook_list = L
 void *
 critpollhook_establish(void (*fn)(void *), void *arg)
 {
-	return hook_establish(&critpollhook_list, fn, arg);
+	return hook_establish(&critpollhook_list, NULL, fn, arg);
 }
 
 void
 critpollhook_disestablish(void *vhook)
 {
-	hook_disestablish(&critpollhook_list, vhook);
+	hook_disestablish(&critpollhook_list, NULL, vhook);
 }
 
 /*

Reply via email to