tech-kern archive

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index][Old Index]

Re: sysctl's in modules - specifically compat_netbsd32



On Sat, 13 Dec 2008, Paul Goyette wrote:

This is a problem with sysctl.  Basically there should be a way to say
"this is a system-provided node, but it can be unloaded if the code
providing it is unloaded".  Now you can work around the problem simply
by providing a storage parameter for the unwind log (which internally
removes the permanent flag).

Hmm, looking at the code netbsd32_sysctl_init() already provides
the log parameter.  It must be tripping on something else besides
CTLFLAG_PERMANENT.

No, it really is tripping over the PERMANENT flag!

In kern/kern_sysctl.c line 736 the check for "init_done" is made by looking at the flags of the system's default sysctl tree, regardless of which root is being updated. So even though netbsd32_sysctl_init() is adding its shadow nodes to a non-default tree, it fails because of the init_done check.

I think the following patch is appropriate. It changes the init_done check to examine the PERMANENT flag in the affected tree's root node, rather than the default system tree's node. (Note I have not yet done even a compile test of this patch yet!)

Index: kern/kern_sysctl.c
===================================================================
RCS file: /cvsroot/src/sys/kern/kern_sysctl.c,v
retrieving revision 1.219
diff -u -p -r1.219 kern_sysctl.c
--- kern/kern_sysctl.c  12 Nov 2008 12:36:16 -0000      1.219
+++ kern/kern_sysctl.c  14 Dec 2008 02:18:17 -0000
@@ -733,7 +733,7 @@ sysctl_create(SYSCTLFN_ARGS)
        * the tree itself is not writeable or
        * the entire sysctl system is not writeable
        */
-       if ((sysctl_root.sysctl_flags & CTLFLAG_PERMANENT) &&
+       if ((sysctl_rootof(rnode)->sysctl_flags & CTLFLAG_PERMANENT) &&
           (!(sysctl_rootof(rnode)->sysctl_flags & CTLFLAG_READWRITE) ||
            !(sysctl_root.sysctl_flags & CTLFLAG_READWRITE)))
               return (EPERM);

OK, I have verified that the above patch compiles, builds, and even addresses the problem. So that pretty much confirms that the problem is caused by the CTLFLAG_PERMANENT check.

Does it really make sense for the state of one sysctl tree to control whether or not another tree can be manipulated? I'm stil questioning the last condition in this check, which requires that the main/default tree's root be writeable in order to add an entry to any tree.


----------------------------------------------------------------------
|   Paul Goyette   | PGP DSS Key fingerprint: |  E-mail addresses:   |
| Customer Service | FA29 0E3B 35AF E8AE 6651 |  paul%whooppee.com@localhost   |
| Network Engineer | 0786 F758 55DE 53BA 7731 | pgoyette%juniper.net@localhost |
----------------------------------------------------------------------


Home | Main Index | Thread Index | Old Index