[RFC][PATCH 0/7]: Fix for unsafe notifier chain

[RFC][PATCH 0/7]: Fix for unsafe notifier chain

[Chandra Seetharaman]

In 2.6.14, notifier chains are unsafe. notifier_call_chain() walks through
the list of a call chain without any protection.

Alan Stern <stern@rowland.harvard.edu> 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.

I posted an RFC that meets the above listed requirements last Friday:
	- http://marc.theaimsgroup.com/?l=linux-kernel&m=113175279131346&w=2
	
Paul McKenney provided some suggestions w.r.t RCU usage. This patchset fixes
the issues he raised.  Keith Owens posted some changes to the diechain for
various architectures; his changes are included here.

This is posted as an RFC as we want to get a green signal from the owners of
the files that our classification of chains as ATOMIC or BLOCKING is ok.
Please comment.

This patchset has 7 patches:

1 of 7: Changes the definition of the heads. Same as what was posted last
	Friday with changes w.r.t Paul's comments.
2 of 7:	Changes that affected only the notifier_head definition.
3 of 7: Changes in which we removed some protection (it's no longer needed
	as the basic infrastructure itself provides the protection).
4 of 7: changes for diechain for different architectures.
5 of 7: changes removing calls to notifier_unregister in the callout.
6 of 7: changes to dcdbas.c (requires special handling).
7 of 7: changes to make usb_notify to use the notify chain infrastructure
	instead of its own.

----------------------------------------

Here are the list of chains and their classification:

BLOCKING:
+++++++++
arch/powerpc/platforms/pseries/reconfig.c:	pSeries_reconfig_chain
arch/s390/kernel/process.c:		idle_chain
drivers/base/memory.c:			memory_chain
drivers/cpufreq/cpufreq.c:		cpufreq_policy_notifier_list
drivers/cpufreq/cpufreq.c:		cpufreq_transition_notifier_list
drivers/macintosh/adb.c:		adb_client_list
drivers/macintosh/via-pmu.c:		sleep_notifier_list
drivers/macintosh/via-pmu68k.c:		sleep_notifier_list
drivers/macintosh/windfarm_core.c:	wf_client_list
drivers/usb/core/notify.c:		usb_notifier_list
drivers/video/fbmem.c:			fb_notifier_list
kernel/cpu.c:				cpu_chain
kernel/module.c:			module_notify_list
kernel/profile.c:			munmap_notifier
kernel/profile.c:			task_exit_notifier
kernel/sys.c:				reboot_notifier_list
net/core/dev.c:				netdev_chain
net/decnet/dn_dev.c:			dnaddr_chain
net/ipv4/devinet.c:			inetaddr_chain

ATOMIC:
+++++++
arch/i386/kernel/traps.c:		i386die_chain
arch/ia64/kernel/traps.c:		ia64die_chain
arch/powerpc/kernel/traps.c:		powerpc_die_chain
arch/sparc64/kernel/traps.c:		sparc64die_chain
arch/x86_64/kernel/traps.c:		die_chain
drivers/char/ipmi/ipmi_si_intf.c:	xaction_notifier_list
kernel/panic.c:				panic_notifier_list
kernel/profile.c:			task_free_notifier
net/bluetooth/hci_core.c:		hci_notifier
net/ipv4/netfilter/ip_conntrack_core.c:	ip_conntrack_chain
net/ipv4/netfilter/ip_conntrack_core.c:	ip_conntrack_expect_chain
net/ipv6/addrconf.c:			inet6addr_chain
net/netfilter/nf_conntrack_core.c:	nf_conntrack_chain
nen/netfilter/nf_conntrack_core.c:	nf_conntrack_expect_chain
net/netlink/af_netlink.c:		netlink_chain


-- 

----------------------------------------------------------------------
    Chandra Seetharaman               | Be careful what you choose....
              - sekharan@us.ibm.com   |      .......you may get it.
----------------------------------------------------------------------

Re: [RFC][PATCH 0/7]: Fix for unsafe notifier chain

[Alan Stern]

Andrew:

I wasn't on the CC: list for parts of this conversation, so I'm a little 
behind-hand.  But Chandra is off on vacation now, and he asked me to 
continue pursuing this.

On November 27, 2005, Andrew Morton wrote:

> This all looks exotically complex.
> 
> > ...
> > 
> > Here are the details:
> > In 2.6.14, notifier chains are unsafe. notifier_call_chain() walks through
> > the list of a call chain without any protection.

...

> - You don't state _why_ a callback cannot call
>   notifier_chain_unregister().  I assume that's because of the use of
>   write_lock() locking?

Yes, that's part of it.

>   We could do this with a new callback function return code and do it in
>   the core, or just change the code so it is permitted.

I don't think that could be made to work.  The new unregister routine
guarantees that when it returns, the callout is not running on any
processor and will not be invoked.  Clearly this guarantee is necessary
when unregistering a callout that's part of a module about to be unloaded.  
Equally clearly, the guarantee cannot be met when a callout tries to
unregister itself.

On the other hand, this is a very minor issue.  We identified only two
callout routines that want to unregister themselves, both on the reboot
notifier chain.  It's simple to work around the self-unregistration, and
one of the patches in our series (5 of 7) did just that.  That patch alone
is harmless, and in fact it could be accepted independently of the rest of
our changes.

> - You don't explain why RCU has been introduced into this subsystem. 
>   Seems overkillish, or was it done as a way to solve the correctness
>   problems?

[Chandra may have explained this already; if so please forgive the
repetition.]  At least one notifier chain is invoked inside an NMI
handler, so normal locking can't be used with it.  RCU seems like an ideal
way to handle such cases.

The RCU overhead in our patches is really very small.  It amounts to a 
preempt_disable in notifier_call_chain, together with a few memory 
barriers and a call to synchronize_rcu in notifier_unregister.

The memory barriers are necessary even without RCU; they are the same as 
the ones Andi put in his proposed patch.  So the only noticeable overhead 
is in the unregister routine, which runs very infrequently.  (Andi's patch 
explicitly ignores the problems raised by unregistration.)

> - Overall take on the patches: the problem here is that notifier chains
>   try to provide their own locking.  Each time we design a container which
>   does that, we screw it up and we regret it.
> 
>   Please consider removing all locking from the notifer chains and moving
>   it into the callers.

That's a good point.  Would you accept a compromise solution?

A high percentage of the existing notifier chains are of the blocking sort
(the callouts are allowed to sleep).  They are well served by a simple
rw-semaphore, as in our patch.  It seems foolish to force the duplication
of this locking code in all the places that would need it.

Likewise, the atomic-type chains (where the callouts must run in an atomic 
context) are generally well served by the RCU mechanism, especially in 
cases where callouts are never unregistered.

So I propose that, in addition to those two types of chains, we define a
third type: raw notifiers.  These will be implemented with no protection
at all.  No rw-semaphore, no spinlock, no RCU, no need to avoid
self-unregistration, nothing -- all protection will be up to the users.  
In other words, just what you asked for.

This gives us the best of both worlds.  The common cases can benefit from
the centralized locking and protection, while anyone who has special needs
can easily provide for them.

If you think this would be okay, I'll rewrite the notifier patch to 
include the raw type.

Alan Stern

Re: [RFC][PATCH 0/7]: Fix for unsafe notifier chain

[Andrew Morton]

Alan Stern <stern@rowland.harvard.edu> wrote:
>
> A high percentage of the existing notifier chains are of the blocking sort
> (the callouts are allowed to sleep).  They are well served by a simple
> rw-semaphore, as in our patch.  It seems foolish to force the duplication
> of this locking code in all the places that would need it.
> 
> Likewise, the atomic-type chains (where the callouts must run in an atomic 
> context) are generally well served by the RCU mechanism, especially in 
> cases where callouts are never unregistered.
> 
> So I propose that, in addition to those two types of chains, we define a
> third type: raw notifiers.  These will be implemented with no protection
> at all.  No rw-semaphore, no spinlock, no RCU, no need to avoid
> self-unregistration, nothing -- all protection will be up to the users.  
> In other words, just what you asked for.
> 
> This gives us the best of both worlds.  The common cases can benefit from
> the centralized locking and protection, while anyone who has special needs
> can easily provide for them.
> 
> If you think this would be okay, I'll rewrite the notifier patch to 
> include the raw type.

The default version of notifier chains should be the lockless version -
just like list_head, radix_tree, etc.  (idr tries to provide its own
locking and has turned out to be a classic case of why we shouldn't do
that).

And sure, it makes sense to provide additional, higher-level data
structures and APIs which layer on top of that, purely as a code
consolidation exercise.  But they shouldn't be called notifier_chains. 
Call them notifier_chain_mutex_locked and notifier_chain_rwlocked and
notifier_chain_spinlocked or whatever.

As for the NMIs and RCU: I suspect it was simply a mistake to try to use
notifier chains for NMI registration in the first place - they are simply
too complex a data structure for this.  I think I previously suggested
removing that code and using just a fixed-size array of function pointers. 
But if the RCUified notifier chain solution is less-than-totally-gross then
that might be OK as well.

Re: [RFC][PATCH 0/7]: Fix for unsafe notifier chain

[Alan Stern]

On Wed, 7 Dec 2005, Andrew Morton wrote:

> The default version of notifier chains should be the lockless version -
> just like list_head, radix_tree, etc.  (idr tries to provide its own
> locking and has turned out to be a classic case of why we shouldn't do
> that).
> 
> And sure, it makes sense to provide additional, higher-level data
> structures and APIs which layer on top of that, purely as a code
> consolidation exercise.  But they shouldn't be called notifier_chains. 
> Call them notifier_chain_mutex_locked and notifier_chain_rwlocked and
> notifier_chain_spinlocked or whatever.

The code below defines three new data structures: atomic_notifier_head,
blocking_notifier_head, and raw_notifier_head.  The first two correspond
to what we had in the earlier patch, and raw_notifier_head is almost the
same as the current implementation, with no locking or protection at all.  
(None is the default; the type has to be specified when a notifier chain
is defined or used.)

Each of the three has its own API, which is layered on top of a single set 
of core routines.  Is this basically what you had in mind?

> As for the NMIs and RCU: I suspect it was simply a mistake to try to use
> notifier chains for NMI registration in the first place - they are simply
> too complex a data structure for this.  I think I previously suggested
> removing that code and using just a fixed-size array of function pointers. 
> But if the RCUified notifier chain solution is less-than-totally-gross then
> that might be OK as well.

Let me know if the atomic_notifier_head code looks totally gross.  If you
think this scheme is okay, I'll go through and make the corresponding
changes to all the users of the notifier_chain API.

Alan Stern



Index: usb-2.6/include/linux/notifier.h
===================================================================
--- usb-2.6.orig/include/linux/notifier.h
+++ usb-2.6/include/linux/notifier.h
@@ -10,25 +10,100 @@
 #ifndef _LINUX_NOTIFIER_H
 #define _LINUX_NOTIFIER_H
 #include <linux/errno.h>
+#include <asm/semaphore.h>
+#include <linux/rwsem.h>
 
-struct notifier_block
-{
-	int (*notifier_call)(struct notifier_block *self, unsigned long, void *);
+/*
+ * Notifier chains are of three types:
+ *
+ *	Atomic notifier chains: Chain callbacks run in interrupt/atomic
+ *		context. Callouts are not allowed to block.
+ *	Blocking notifier chains: Chain callbacks run in process context.
+ *		Callouts are allowed to block.
+ *	Raw notifier chains: There are no restrictions on callbacks,
+ *		registration, or unregistration.  All locking and protection
+ *		must be provided by the caller.
+ *
+ * atomic_notifier_chain_register() and blocking_notifier_chain_register()
+ * may be called only from process context, and likewise for the
+ * corresponding _unregister() routines.
+ *
+ * atomic_notifier_chain_unregister() and blocking_notifier_chain_unregister()
+ * _must not_ be called from within the call chain.
+ */
+
+struct notifier_block {
+	int (*notifier_call)(struct notifier_block *, unsigned long, void *);
 	struct notifier_block *next;
 	int priority;
 };
 
+struct atomic_notifier_head {
+	struct semaphore sem;
+	struct notifier_block *head;
+};
+
+struct blocking_notifier_head {
+	struct rw_semaphore rwsem;
+	struct notifier_block *head;
+};
+
+struct raw_notifier_head {
+	struct notifier_block *head;
+};
+
+#define ATOMIC_INIT_NOTIFIER_HEAD(name) do {	\
+		init_MUTEX(&(name)->rwsem);	\
+		(name)->head = NULL;		\
+	} while (0)
+#define BLOCKING_INIT_NOTIFIER_HEAD(name) do {	\
+		init_rwsem(&(name)->rwsem);	\
+		(name)->head = NULL;		\
+	} while (0)
+#define RAW_INIT_NOTIFIER_HEAD(name) do {	\
+		(name)->head = NULL;		\
+	} while (0)
+
+#define ATOMIC_NOTIFIER_HEAD(name)				\
+	struct atomic_notifier_head name = {			\
+		.sem = __SEMAPHORE_INITIALIZER((name).sem, 1),	\
+		.head = NULL }
+#define BLOCKING_NOTIFIER_HEAD(name)				\
+	struct blocking_notifier_head name = {			\
+		.rwsem = __RWSEM_INITIALIZER((name).rwsem),	\
+		.head = NULL }
+#define RAW_NOTIFIER_HEAD(name)					\
+	struct raw_notifier_head name = {			\
+	.head = NULL }
 
 #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 atomic_notifier_chain_register(struct atomic_notifier_head *,
+		struct notifier_block *);
+extern int blocking_notifier_chain_register(struct blocking_notifier_head *,
+		struct notifier_block *);
+extern int raw_notifier_chain_register(struct raw_notifier_head *,
+		struct notifier_block *);
+
+extern int atomic_notifier_chain_unregister(struct atomic_notifier_head *,
+		struct notifier_block *);
+extern int blocking_notifier_chain_unregister(struct blocking_notifier_head *,
+		struct notifier_block *);
+extern int raw_notifier_chain_unregister(struct raw_notifier_head *,
+		struct notifier_block *);
+
+extern int atomic_notifier_call_chain(struct atomic_notifier_head *,
+		unsigned long val, void *v);
+extern int blocking_notifier_call_chain(struct blocking_notifier_head *,
+		unsigned long val, void *v);
+extern int raw_notifier_call_chain(struct raw_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: usb-2.6/kernel/sys.c
===================================================================
--- usb-2.6.orig/kernel/sys.c
+++ usb-2.6/kernel/sys.c
@@ -94,67 +94,192 @@ int cad_pid = 1;
  */
 
 static struct notifier_block *reboot_notifier_list;
-static DEFINE_RWLOCK(notifier_lock);
+
+/*
+ *	Notifier chain core routines.  The exported routines below
+ *	are layered on top of these, with appropriate locking added.
+ */ 
+
+static int notifier_chain_register(struct notifier_block **nl,
+		struct notifier_block *n)
+{
+	while ((*nl) != NULL) {
+		if (n->priority > (*nl)->priority)
+			break;
+		nl = &((*nl)->next);
+	}
+	n->next = *nl;
+	rcu_assign_pointer(*nl, n);
+	return 0;
+}
+
+static int notifier_chain_unregister(struct notifier_block **nl,
+		struct notifier_block *n)
+{
+	while ((*nl) != NULL) {
+		if ((*nl) == n) {
+			rcu_assign_pointer(*nl, n->next);
+			return 0;
+		}
+		nl = &((*nl)->next);
+	}
+	return -ENOENT;
+}
+ 
+static int notifier_call_chain(struct notifier_block **nl,
+		unsigned long val, void *v)
+{
+	int ret = NOTIFY_DONE;
+	struct notifier_block *nb;
+
+	nb = rcu_dereference(*nl);
+	while (nb) {
+		ret = nb->notifier_call(nb, val, v);
+		if ((ret & NOTIFY_STOP_MASK) == NOTIFY_STOP_MASK)
+			break;
+		nb = rcu_dereference(nb->next);
+	}
+	return ret;
+}
+
+/*
+ *	Atomic notifier chain routines.  Registration and unregistration
+ *	use a mutex, and call_chain is synchronized by RCU (no locks).
+ */ 
 
 /**
- *	notifier_chain_register	- Add notifier to a notifier chain
- *	@list: Pointer to root list pointer
+ *	atomic_notifier_chain_register - Add notifier to an atomic notifier chain
+ *	@nh: Pointer to head of the atomic notifier chain
  *	@n: New entry in notifier chain
  *
- *	Adds a notifier to a notifier chain.
+ *	Adds a notifier to an atomic notifier chain.
+ *	Must be called in process context.
  *
  *	Currently always returns zero.
  */
+
+int atomic_notifier_chain_register(struct atomic_notifier_head *nh,
+		struct notifier_block *n)
+{
+	int ret;
+
+	down(&nh->sem);
+	ret = notifier_chain_register(&nh->head, n);
+	up(&nh->sem);
+	return ret;
+}
+
+EXPORT_SYMBOL(atomic_notifier_chain_register);
+
+/**
+ *	atomic_notifier_chain_unregister - Remove notifier from an atomic notifier chain
+ *	@nh: Pointer to head of the atomic notifier chain
+ *	@n: Entry to remove from notifier chain
+ *
+ *	Removes a notifier from an atomic notifier chain.
+ *	Must be called from process context.
+ *
+ *	Returns zero on success, or %-ENOENT on failure.
+ */
+int atomic_notifier_chain_unregister(struct atomic_notifier_head *nh,
+		struct notifier_block *n)
+{
+	int ret;
+
+	down(&nh->sem);
+	ret = notifier_chain_unregister(&nh->head, n);
+	up(&nh->sem);
+	synchronize_rcu();
+	return ret;
+}
+
+EXPORT_SYMBOL(atomic_notifier_chain_unregister);
+
+/**
+ *	atomic_notifier_call_chain - Call functions in an atomic notifier chain
+ *	@nh: Pointer to head of the atomic 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.
+ *	This routine uses RCU to synchronize with changes in the chain.
+ *
+ *	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
+ *	the notifier function which halted execution.
+ *	Otherwise, the return value is the return value
+ *	of the last notifier function called.
+ */
  
-int notifier_chain_register(struct notifier_block **list, struct notifier_block *n)
+int atomic_notifier_call_chain(struct atomic_notifier_head *nh,
+		unsigned long val, void *v)
 {
-	write_lock(&notifier_lock);
-	while(*list)
-	{
-		if(n->priority > (*list)->priority)
-			break;
-		list= &((*list)->next);
-	}
-	n->next = *list;
-	*list=n;
-	write_unlock(&notifier_lock);
-	return 0;
+	int ret;
+
+	rcu_read_lock();
+	ret = notifier_call_chain(&nh->head, val, v);
+	rcu_read_unlock();
+	return ret;
 }
 
-EXPORT_SYMBOL(notifier_chain_register);
+EXPORT_SYMBOL(atomic_notifier_call_chain);
+
+/*
+ *	Blocking notifier chain routines.  All access to the chain is
+ *	synchronized by an rwsem.
+ */ 
 
 /**
- *	notifier_chain_unregister - Remove notifier from a notifier chain
- *	@nl: Pointer to root list pointer
+ *	blocking_notifier_chain_register - Add notifier to a blocking notifier chain
+ *	@nh: Pointer to head of the blocking notifier chain
  *	@n: New entry in notifier chain
  *
- *	Removes a notifier from a notifier chain.
+ *	Adds a notifier to a blocking notifier chain.
+ *	Must be called in process context.
  *
- *	Returns zero on success, or %-ENOENT on failure.
+ *	Currently always returns zero.
  */
  
-int notifier_chain_unregister(struct notifier_block **nl, struct notifier_block *n)
+int blocking_notifier_chain_register(struct blocking_notifier_head *nh,
+		struct notifier_block *n)
 {
-	write_lock(&notifier_lock);
-	while((*nl)!=NULL)
-	{
-		if((*nl)==n)
-		{
-			*nl=n->next;
-			write_unlock(&notifier_lock);
-			return 0;
-		}
-		nl=&((*nl)->next);
-	}
-	write_unlock(&notifier_lock);
-	return -ENOENT;
+	int ret;
+
+	down_write(&nh->rwsem);
+	ret = notifier_chain_register(&nh->head, n);
+	up_write(&nh->rwsem);
+	return ret;
+}
+
+EXPORT_SYMBOL(blocking_notifier_chain_register);
+
+/**
+ *	blocking_notifier_chain_unregister - Remove notifier from a blocking notifier chain
+ *	@nh: Pointer to head of the blocking notifier chain
+ *	@n: Entry to remove from notifier chain
+ *
+ *	Removes a notifier from a blocking notifier chain.
+ *	Must be called from process context.
+ *
+ *	Returns zero on success, or %-ENOENT on failure.
+ */
+int blocking_notifier_chain_unregister(struct blocking_notifier_head *nh,
+		struct notifier_block *n)
+{
+	int ret;
+
+	down_write(&nh->rwsem);
+	ret = notifier_chain_unregister(&nh->head, n);
+	up_write(&nh->rwsem);
+	return ret;
 }
 
-EXPORT_SYMBOL(notifier_chain_unregister);
+EXPORT_SYMBOL(blocking_notifier_chain_unregister);
 
 /**
- *	notifier_call_chain - Call functions in a notifier chain
- *	@n: Pointer to root pointer of notifier chain
+ *	blocking_notifier_call_chain - Call functions in a blocking notifier chain
+ *	@nh: Pointer to head of the blocking notifier chain
  *	@val: Value passed unmodified to notifier function
  *	@v: Pointer passed unmodified to notifier function
  *
@@ -168,24 +293,85 @@ 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 blocking_notifier_call_chain(struct blocking_notifier_head *nh,
+		unsigned long val, void *v)
 {
-	int ret=NOTIFY_DONE;
-	struct notifier_block *nb = *n;
+	int ret;
 
-	while(nb)
-	{
-		ret=nb->notifier_call(nb,val,v);
-		if(ret&NOTIFY_STOP_MASK)
-		{
-			return ret;
-		}
-		nb=nb->next;
-	}
+	down_read(&nh->rwsem);
+	ret = notifier_call_chain(&nh->head, val, v);
+	up_read(&nh->rwsem);
 	return ret;
 }
 
-EXPORT_SYMBOL(notifier_call_chain);
+EXPORT_SYMBOL(blocking_notifier_call_chain);
+
+/*
+ *	Raw notifier chain routines.  There is no protection;
+ *	the caller must provide it.  Use at your own risk!
+ */ 
+
+/**
+ *	raw_notifier_chain_register - Add notifier to a raw notifier chain
+ *	@nh: Pointer to head of the raw notifier chain
+ *	@n: New entry in notifier chain
+ *
+ *	Adds a notifier to a raw notifier chain.
+ *	All locking must be provided by the caller.
+ *
+ *	Currently always returns zero.
+ */
+
+int raw_notifier_chain_register(struct raw_notifier_head *nh,
+		struct notifier_block *n)
+{
+	return notifier_chain_register(&nh->head);
+}
+
+EXPORT_SYMBOL(raw_notifier_chain_register);
+
+/**
+ *	raw_notifier_chain_unregister - Remove notifier from a raw notifier chain
+ *	@nh: Pointer to head of the raw notifier chain
+ *	@n: Entry to remove from notifier chain
+ *
+ *	Removes a notifier from a raw notifier chain.
+ *	All locking must be provided by the caller.
+ *
+ *	Returns zero on success, or %-ENOENT on failure.
+ */
+int raw_notifier_chain_unregister(struct raw_notifier_head *nh,
+		struct notifier_block *n)
+{
+	return notifier_chain_unregister(&nh->head, n);
+}
+
+EXPORT_SYMBOL(raw_notifier_chain_unregister);
+
+/**
+ *	raw_notifier_call_chain - Call functions in a raw notifier chain
+ *	@nh: Pointer to head of the raw 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.
+ *	All locking must be provided by the caller.
+ *
+ *	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
+ *	the notifier function which halted execution.
+ *	Otherwise, the return value is the return value
+ *	of the last notifier function called.
+ */
+ 
+int raw_notifier_call_chain(struct raw_notifier_head *nh,
+		unsigned long val, void *v)
+{
+	return notifier_call_chain(&nh->head, val, v);
+}
+
+EXPORT_SYMBOL(raw_notifier_call_chain);
 
 /**
  *	register_reboot_notifier - Register function to be called at reboot time

Re: [RFC][PATCH 0/7]: Fix for unsafe notifier chain

[Keith Owens]

On Thu, 8 Dec 2005 14:53:56 -0500 (EST), 
Alan Stern <stern@rowland.harvard.edu> wrote:
>The code below defines three new data structures: atomic_notifier_head,
>blocking_notifier_head, and raw_notifier_head.  The first two correspond
>to what we had in the earlier patch, and raw_notifier_head is almost the
>same as the current implementation, with no locking or protection at all.  

Acked-By: Keith Owens <kaos@sgi.com>

I do not care how this problem is fixed, I am happy with any solution that

(a) stops notify chains being racy and
(b) allows users of notify_die() to be safely unloaded.

Re: [RFC][PATCH 0/7]: Fix for unsafe notifier chain

[Alan Stern]

On Tue, 13 Dec 2005, Keith Owens wrote:

> On Thu, 8 Dec 2005 14:53:56 -0500 (EST), 
> Alan Stern <stern@rowland.harvard.edu> wrote:
> >The code below defines three new data structures: atomic_notifier_head,
> >blocking_notifier_head, and raw_notifier_head.  The first two correspond
> >to what we had in the earlier patch, and raw_notifier_head is almost the
> >same as the current implementation, with no locking or protection at all.  
> 
> Acked-By: Keith Owens <kaos@sgi.com>
> 
> I do not care how this problem is fixed, I am happy with any solution that
> 
> (a) stops notify chains being racy and
> (b) allows users of notify_die() to be safely unloaded.

Andrew, I've been waiting to hear back about this.  Was that latest
proposal (three separate types of notifier chains, each with its own API,
one of them being completely raw) acceptable?

Alan Stern

Re: [RFC][PATCH 0/7]: Fix for unsafe notifier chain

[Andrew Morton]

Alan Stern <stern@rowland.harvard.edu> wrote:
>
> On Tue, 13 Dec 2005, Keith Owens wrote:
> 
> > On Thu, 8 Dec 2005 14:53:56 -0500 (EST), 
> > Alan Stern <stern@rowland.harvard.edu> wrote:
> > >The code below defines three new data structures: atomic_notifier_head,
> > >blocking_notifier_head, and raw_notifier_head.  The first two correspond
> > >to what we had in the earlier patch, and raw_notifier_head is almost the
> > >same as the current implementation, with no locking or protection at all.  
> > 
> > Acked-By: Keith Owens <kaos@sgi.com>
> > 
> > I do not care how this problem is fixed, I am happy with any solution that
> > 
> > (a) stops notify chains being racy and
> > (b) allows users of notify_die() to be safely unloaded.
> 
> Andrew, I've been waiting to hear back about this.

Was subconciously hoping it'd go away, I guess.

> Was that latest
> proposal (three separate types of notifier chains, each with its own API,
> one of them being completely raw) acceptable?

Yes, it looks sane enough.

Re: [RFC][PATCH 0/7]: Fix for unsafe notifier chain

[Keith Owens]

On Wed, 7 Dec 2005 15:36:12 -0800, 
Andrew Morton <akpm@osdl.org> wrote:
>As for the NMIs and RCU: I suspect it was simply a mistake to try to use
>notifier chains for NMI registration in the first place - they are simply
>too complex a data structure for this.  I think I previously suggested
>removing that code and using just a fixed-size array of function pointers.

The notifier chain is a priority ordered list.  Registration and
unregistration must be able to insert and delete anywhere in the list,
which does not fit with a fixed-size array of pointers.

There is also the problem that unregister must not return to its caller
if any NMI type callback is running on any cpu.  The list and function
body are held in caller defined storage and if unregistration returns
too soon then the storage could be freed while the callback is still
executing.

I do not understand why people think that NMI callbacks are a hard
problem to solve.  Besides RCU type solutions, there is also a patch
based on an idea by Corey Minyard, see
http://marc.theaimsgroup.com/?l=linux-kernel&m=113392370322545&w=2.