Hi
Looking at 2.6.3-bk, it appears that the sysctl code does not raise a
module's reference count before calling a sysctl handler registered by
that module.
- are modules allowed to register sysctl handlers?
register_sysctl_table is exported, so I imagine so.
- am I missing something and modules are in fact protected against
concurrent unloading and invocation of sysctl?
Cheers,
Muli
--
Muli Ben-Yehuda
http://www.mulix.org | http://mulix.livejournal.com/
On Tue, Mar 02, 2004 at 02:29:09PM +0200, Muli Ben-Yehuda wrote:
> Hi
>
> Looking at 2.6.3-bk, it appears that the sysctl code does not raise a
> module's reference count before calling a sysctl handler registered by
> that module.
>
> - are modules allowed to register sysctl handlers?
> register_sysctl_table is exported, so I imagine so.
>
> - am I missing something and modules are in fact protected against
> concurrent unloading and invocation of sysctl?
They are not and no, bumping refcount would not be anywhere near enough.
It's not just modules - no module refcount will help e.g. per-interface
sysctls. All dynamic sysctls are b0rken and the best course of action
is to avoid using that crap.
We'll need to fix that mess, but it won't be easy.
On Tue, 2004-03-02 at 23:41, [email protected]
wrote:
> On Tue, Mar 02, 2004 at 02:29:09PM +0200, Muli Ben-Yehuda wrote:
> > Hi
> >
> > Looking at 2.6.3-bk, it appears that the sysctl code does not raise a
> > module's reference count before calling a sysctl handler registered by
> > that module.
> >
> > - are modules allowed to register sysctl handlers?
> > register_sysctl_table is exported, so I imagine so.
> >
> > - am I missing something and modules are in fact protected against
> > concurrent unloading and invocation of sysctl?
>
> They are not and no, bumping refcount would not be anywhere near enough.
Al is referring to the fact that there's no protection for a dynamically
allocated ctl_table.
However, an owner field and standard module_get() would solve the case
of statically declared ctl_table.
I don't know that there's a current user who requires it though?
Rusty.
--
Anyone who quotes me in their signature is an idiot -- Rusty Russell
On Wed, Mar 03, 2004 at 11:05:39AM +1100, Rusty Russell wrote:
> Al is referring to the fact that there's no protection for a dynamically
> allocated ctl_table.
>
> However, an owner field and standard module_get() would solve the case
> of statically declared ctl_table.
That's what I had in mind.
> I don't know that there's a current user who requires it though?
Yes, there are. Using the attached scriptlet on a 2.6.1 tree I had
lying around:
[root@hydra kernel]# /tmp/find-register-sysctl
/lib/modules/2.6.1/kernel/drivers/cdrom/cdrom.ko uses register_sysctl
/lib/modules/2.6.1/kernel/drivers/parport/parport.ko uses register_sysctl
/lib/modules/2.6.1/kernel/net/appletalk/appletalk.ko uses register_sysctl
/lib/modules/2.6.1/kernel/net/ipx/ipx.ko uses register_sysctl
/lib/modules/2.6.1/kernel/net/irda/irda.ko uses register_sysctl
/lib/modules/2.6.1/kernel/net/sctp/sctp.ko uses register_sysctl
I'm building 2.6.3-bk with allmodconfig now. Once it's done, I'll post
the resulting list.
#!/bin/sh
KERNEL_VER=2.6.1
for f in `find /lib/modules/${KERNEL_VER} -name "*.ko"`; do
nm $f | grep register_sysctl 2>&1 > /dev/null
if [ "x$?" == "x0" ]; then
echo "$f uses register_sysctl"
fi
done
Cheers,
Muli
--
Muli Ben-Yehuda
http://www.mulix.org | http://mulix.livejournal.com/
On Wed, Mar 03, 2004 at 11:22:39AM +0200, Muli Ben-Yehuda wrote:
> > However, an owner field and standard module_get() would solve the case
> > of statically declared ctl_table.
>
> That's what I had in mind.
>
> > I don't know that there's a current user who requires it though?
>
> Yes, there are. Using the attached scriptlet on a 2.6.1 tree I had
> lying around:
>
> [root@hydra kernel]# /tmp/find-register-sysctl
> /lib/modules/2.6.1/kernel/drivers/cdrom/cdrom.ko uses register_sysctl
> /lib/modules/2.6.1/kernel/drivers/parport/parport.ko uses register_sysctl
> /lib/modules/2.6.1/kernel/net/appletalk/appletalk.ko uses register_sysctl
> /lib/modules/2.6.1/kernel/net/ipx/ipx.ko uses register_sysctl
> /lib/modules/2.6.1/kernel/net/irda/irda.ko uses register_sysctl
> /lib/modules/2.6.1/kernel/net/sctp/sctp.ko uses register_sysctl
At least parport has both module-wide and per-port sysctls. The
latter are dynamic even if module support is turned off. I seriously
suspect that other examples are similar.
On Wed, Mar 03, 2004 at 10:43:32AM +0000, [email protected] wrote:
> At least parport has both module-wide and per-port sysctls. The
> latter are dynamic even if module support is turned off. I seriously
> suspect that other examples are similar.
For reference, here's the list of register_sysctl_table() users from
2.6.3-bk with an "almost allmodconfig" configuration. Those that I've
randomly looked at all used a statically allocated ctl_table
structure. Will adding a .owner field to it with the relevant
module_get/module_put in the registration functions hinder future
efforts to fix it properly? It won't be perfect, but it will be better
than what we currently have.
./drivers/cdrom/cdrom.ko uses register_sysctl
./drivers/char/rtc.ko uses register_sysctl
./drivers/cpufreq/cpufreq_userspace.ko uses register_sysctl
./drivers/md/md.ko uses register_sysctl
./drivers/net/wireless/arlan.ko uses register_sysctl
./drivers/parport/parport.ko uses register_sysctl
./drivers/scsi/scsi_mod.ko uses register_sysctl
./fs/coda/coda.ko uses register_sysctl
./fs/lockd/lockd.ko uses register_sysctl
./fs/ntfs/ntfs.ko uses register_sysctl
./fs/xfs/xfs.ko uses register_sysctl
./net/appletalk/appletalk.ko uses register_sysctl
./net/ax25/ax25.ko uses register_sysctl
./net/bridge/bridge.ko uses register_sysctl
./net/decnet/decnet.ko uses register_sysctl
./net/ipv4/ipvs/ip_vs_lblc.ko uses register_sysctl
./net/ipv4/ipvs/ip_vs_lblcr.ko uses register_sysctl
./net/ipv4/ipvs/ip_vs.ko uses register_sysctl
./net/ipv4/netfilter/ip_conntrack.ko uses register_sysctl
./net/ipv4/netfilter/ip_queue.ko uses register_sysctl
./net/ipv6/netfilter/ip6_queue.ko uses register_sysctl
./net/ipv6/ipv6.ko uses register_sysctl
./net/ipx/ipx.ko uses register_sysctl
./net/irda/irda.ko uses register_sysctl
./net/netrom/netrom.ko uses register_sysctl
./net/rose/rose.ko uses register_sysctl
./net/rxrpc/rxrpc.ko uses register_sysctl
./net/sctp/sctp.ko uses register_sysctl
./net/sunrpc/sunrpc.ko uses register_sysctl
./net/unix/unix.ko uses register_sysctl
./net/x25/x25.ko uses register_sysctl
Cheers,
Muli
--
Muli Ben-Yehuda
http://www.mulix.org | http://mulix.livejournal.com/
On Thu, Mar 04, 2004 at 12:38:25PM +0200, Muli Ben-Yehuda wrote:
> ./drivers/cpufreq/cpufreq_userspace.ko uses register_sysctl
Obsolete, and will be going away in 2.7
There's a perfectly functional sysfs interface for this stuff now.
Dave