Hello,
In 2.6.14, the notifier chains are unsafe. notifier_call_chain() walks through
the list of a call chain without any protection.
Alan Stern <[email protected]> brought the issue and suggested a fix
in lkml on Oct 24 2005:
http://marc.theaimsgroup.com/?l=linux-kernel&m=113018709002036&w=2
There was a lot of discussion on that thread regarding the issue, and
following were the conclusions about the requirements of the notifier
call mechanism:
- The chain list has to be protected in all the places where the
list is accessed.
- we need a new notifier_head data structure to encompass the head
of the notifier chain and a semaphore that protects the list.
- There should be two types of call chains: one that is called in
a process context and another that is called in atomic/interrupt
context.
- No lock should be acquired in notifier_call_chain() for an
atomic-type chain.
- notifier_chain_register() and notifier_chain_unregister() should
be called only from process context.
- notifier_chain_unregister() should _not_ be called from a
callout routine.
This patch is a modified version of Alan's original patch and meets these
requirements. The patch is not complete, since all the notifier call
chain definitions have to changed to use the new notifier_head data
structure.
Alan and I did think about changing the data structure to use list_head, but
deferred it (as a cleanup) as it is not directly tied with what Alan was
trying to fix.
----
Signed-off-by: Chandra Seetharaman <[email protected]>
Signed-off-by: Alan Stern <[email protected]>
-----
include/linux/notifier.h | 69 +++++++++++++++++++++++++++++---
kernel/sys.c | 101 ++++++++++++++++++++++++++++-------------------
2 files changed, 126 insertions(+), 44 deletions(-)
Index: l2614-notifiers/include/linux/notifier.h
===================================================================
--- l2614-notifiers.orig/include/linux/notifier.h
+++ l2614-notifiers/include/linux/notifier.h
@@ -10,25 +10,84 @@
#ifndef _LINUX_NOTIFIER_H
#define _LINUX_NOTIFIER_H
#include <linux/errno.h>
+#include <linux/rwsem.h>
+
+/*
+ * Notifier chains are of two types:
+ * Atomic notifier chains: Chain callbacks run in interrupt/atomic
+ * context. Callouts are not allowed to block.
+ * Blocking notifier chains: Chain callback run in process context.
+ * Callouts are allowed to block.
+ *
+ * Type of a chain is defined in its head.
+ *
+ * notifier_chain_register() and notifier_chain_unregister() should be
+ * called only from process context.
+ *
+ * notifier_chain_unregister() _should not_ be called from the
+ * corresponding call chain.
+ *
+ */
+enum notifier_type {
+ ATOMIC_NOTIFIER,
+ BLOCKING_NOTIFIER,
+};
struct notifier_block
{
- int (*notifier_call)(struct notifier_block *self, unsigned long, void *);
+ int (*notifier_call)(struct notifier_block *, unsigned long, void *);
struct notifier_block *next;
int priority;
};
+struct notifier_head {
+ enum notifier_type type;
+ struct rw_semaphore rwsem;
+ struct notifier_block *head;
+};
+
+#define NOTIFIER_HEAD_INIT(name, head_type) { \
+ .type = head_type, \
+ .rwsem = __RWSEM_INITIALIZER((name).rwsem), \
+ .head = NULL }
+
+#define NOTIFIER_HEAD(name, head_type) \
+ struct notifier_head name = NOTIFIER_HEAD_INIT(name, head_type)
+
+#define INIT_NOTIFIER_HEAD(name, head_type) do { \
+ (ptr)->type = head_type; \
+ init_rwsem(&(ptr)->rwsem); \
+ ptr->head = NULL; \
+} while (0)
+
+#define ATOMIC_NOTIFIER_HEAD_INIT(name) \
+ NOTIFIER_HEAD_INIT(name, ATOMIC_NOTIFIER)
+#define ATOMIC_NOTIFIER_HEAD(name) \
+ NOTIFIER_HEAD(name, ATOMIC_NOTIFIER)
+#define ATOMIC_INIT_NOTIFIER_HEAD(name) \
+ INIT_NOTIFIER_HEAD(name, ATOMIC_NOTIFIER)
+
+#define BLOCKING_NOTIFIER_HEAD_INIT(name) \
+ NOTIFIER_HEAD_INIT(name, BLOCKING_NOTIFIER)
+#define BLOCKING_NOTIFIER_HEAD(name) \
+ NOTIFIER_HEAD(name, BLOCKING_NOTIFIER)
+#define BLOCKING_INIT_NOTIFIER_HEAD(name) \
+ INIT_NOTIFIER_HEAD(name, BLOCKING_NOTIFIER)
#ifdef __KERNEL__
-extern int notifier_chain_register(struct notifier_block **list, struct notifier_block *n);
-extern int notifier_chain_unregister(struct notifier_block **nl, struct notifier_block *n);
-extern int notifier_call_chain(struct notifier_block **n, unsigned long val, void *v);
+extern int notifier_chain_register(struct notifier_head *,
+ struct notifier_block *);
+extern int notifier_chain_unregister(struct notifier_head *,
+ struct notifier_block *);
+extern int notifier_call_chain(struct notifier_head *,
+ unsigned long val, void *v);
#define NOTIFY_DONE 0x0000 /* Don't care */
#define NOTIFY_OK 0x0001 /* Suits me */
#define NOTIFY_STOP_MASK 0x8000 /* Don't call further */
-#define NOTIFY_BAD (NOTIFY_STOP_MASK|0x0002) /* Bad/Veto action */
+#define NOTIFY_BAD (NOTIFY_STOP_MASK|0x0002)
+ /* Bad/Veto action */
/*
* Clean way to return from the notifier and stop further calls.
*/
Index: l2614-notifiers/kernel/sys.c
===================================================================
--- l2614-notifiers.orig/kernel/sys.c
+++ l2614-notifiers/kernel/sys.c
@@ -92,31 +92,35 @@ int cad_pid = 1;
* and the like.
*/
-static struct notifier_block *reboot_notifier_list;
-static DEFINE_RWLOCK(notifier_lock);
+static BLOCKING_NOTIFIER_HEAD(reboot_notifier_list);
/**
* notifier_chain_register - Add notifier to a notifier chain
- * @list: Pointer to root list pointer
+ * @nh: Pointer to head of the notifier chain
* @n: New entry in notifier chain
*
* Adds a notifier to a notifier chain.
+ * Must be called from process context.
*
* Currently always returns zero.
*/
-int notifier_chain_register(struct notifier_block **list, struct notifier_block *n)
+int notifier_chain_register(struct notifier_head *nh, struct notifier_block *n)
{
- write_lock(¬ifier_lock);
- while(*list)
- {
- if(n->priority > (*list)->priority)
- break;
- list= &((*list)->next);
- }
- n->next = *list;
- *list=n;
- write_unlock(¬ifier_lock);
+ struct notifier_block **nl;
+
+ down_write(&nh->rwsem);
+ nl = &nh->head;
+ while ((*nl) != NULL) {
+ if (n->priority > (*nl)->priority)
+ break;
+ nl = &((*nl)->next);
+ }
+ rcu_assign_pointer(n->next, *nl);
+ rcu_assign_pointer(*nl, n);
+ up_write(&nh->rwsem);
+ if (nh->type == ATOMIC_NOTIFIER)
+ synchronize_rcu();
return 0;
}
@@ -124,28 +128,32 @@ EXPORT_SYMBOL(notifier_chain_register);
/**
* notifier_chain_unregister - Remove notifier from a notifier chain
- * @nl: Pointer to root list pointer
+ * @nh: Pointer to head of the notifier chain
* @n: New entry in notifier chain
*
* Removes a notifier from a notifier chain.
+ * Must be called from process context.
*
* Returns zero on success, or %-ENOENT on failure.
*/
-
-int notifier_chain_unregister(struct notifier_block **nl, struct notifier_block *n)
+int notifier_chain_unregister(struct notifier_head *nh,
+ struct notifier_block *n)
{
- write_lock(¬ifier_lock);
- while((*nl)!=NULL)
- {
- if((*nl)==n)
- {
- *nl=n->next;
- write_unlock(¬ifier_lock);
+ struct notifier_block **nl;
+
+ down_write(&nh->rwsem);
+ nl = &nh->head;
+ while ((*nl) != NULL) {
+ if ((*nl) == n) {
+ rcu_assign_pointer(*nl, n->next);
+ up_write(&nh->rwsem);
+ if (nh->type == ATOMIC_NOTIFIER)
+ synchronize_rcu();
return 0;
}
- nl=&((*nl)->next);
+ nl = &((*nl)->next);
}
- write_unlock(¬ifier_lock);
+ up_write(&nh->rwsem);
return -ENOENT;
}
@@ -153,12 +161,18 @@ EXPORT_SYMBOL(notifier_chain_unregister)
/**
* notifier_call_chain - Call functions in a notifier chain
- * @n: Pointer to root pointer of notifier chain
+ * @nh: Pointer to the head of notifier chain
* @val: Value passed unmodified to notifier function
* @v: Pointer passed unmodified to notifier function
*
* Calls each function in a notifier chain in turn.
*
+ * If @nh points to an %ATOMIC_NOTIFIER_HEAD then this routine may
+ * be called in any context, as it will not sleep.
+ *
+ * If @nh points to a %BLOCKING_NOTIFIER_HEAD then this routine may
+ * be called only in process context.
+ *
* If the return value of the notifier can be and'd
* with %NOTIFY_STOP_MASK, then notifier_call_chain
* will return immediately, with the return value of
@@ -167,20 +181,29 @@ EXPORT_SYMBOL(notifier_chain_unregister)
* of the last notifier function called.
*/
-int notifier_call_chain(struct notifier_block **n, unsigned long val, void *v)
+int notifier_call_chain(struct notifier_head *nh, unsigned long val, void *v)
{
- int ret=NOTIFY_DONE;
- struct notifier_block *nb = *n;
+ int ret = NOTIFY_DONE;
+ struct notifier_block *nb;
- while(nb)
- {
- ret=nb->notifier_call(nb,val,v);
- if(ret&NOTIFY_STOP_MASK)
- {
- return ret;
- }
- nb=nb->next;
- }
+ if (!nh->head)
+ return ret;
+ if (nh->type == ATOMIC_NOTIFIER)
+ rcu_read_lock();
+ else
+ down_read(&nh->rwsem);
+ nb = rcu_dereference(nh->head);
+ while (nb) {
+ ret = nb->notifier_call(nb, val, v);
+ if ((ret & NOTIFY_STOP_MASK) == NOTIFY_STOP_MASK)
+ goto done;
+ nb = rcu_dereference(nb->next);
+ }
+done:
+ if (nh->type == ATOMIC_NOTIFIER)
+ rcu_read_unlock();
+ else
+ up_read(&nh->rwsem);
return ret;
}
--
----------------------------------------------------------------------
Chandra Seetharaman | Be careful what you choose....
- [email protected] | .......you may get it.
----------------------------------------------------------------------
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
[Index of Archives]
[Kernel Newbies]
[Netfilter]
[Bugtraq]
[Photo]
[Stuff]
[Gimp]
[Yosemite News]
[MIPS Linux]
[ARM Linux]
[Linux Security]
[Linux RAID]
[Video 4 Linux]
[Linux for the blind]
[Linux Resources]
