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

Chandra Seetharaman
2005-11-23T23:37:56Z

Andrew,

I posted this set of patch to lkml last Friday as an RFC. Can you
consider these for -mm inclusion.

These patches apply on 2.6.15-rc2.

Thanks,

chandra

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.

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.
----------------------------------------------------------------------