Author: avg
Date: Thu Oct 5 12:32:14 2017
New Revision: 324311
URL: https://svnweb.freebsd.org/changeset/base/324311
Log:
sysctl-s in a module should be accessible only when the module is initialized
A sysctl can have a custom handler that may access data that is initialized
via SYSINIT(9) or via a module event handler (also invoked via SYSINIT).
Thus, it is not safe to allow access to the module's sysctl-s until
the initialization is performed. Likewise, we should not allow access
to teh sysctl-s after the module is uninitialized.
The latter is easy to achieve by properly ordering
linker_file_unregister_sysctls
and linker_file_sysuninit.
The former is not as easy for two reasons:
- the initialization may depend on tunables which get set when sysctl-s are
registered, so we need to set the tunables before running sysinit-s
- the initialization may try to dynamically add more sysctl-s under statically
defined sysctl nodes
So, this change splits the sysctl setup into two phases. In the first phase
the sysctl-s are registered as before but they are disabled and hidden from
consumers. In the second phase, done after sysinit-s, normal access to the
sysctl-s is enabled.
The change should affect only dynamic module loading and unloading after
the system boot-up. Nothing changes for sysctl-s compiled into the kernel
and sysctl-s in preloaded modules.
Discussed with: hselasky, ian, jhb
Reviewed by: julian, kib
MFC after: 2 weeks
Sponsored by: Panzura
Differential Revision: https://reviews.freebsd.org/D12545
Modified:
head/sys/kern/kern_linker.c
head/sys/kern/kern_sysctl.c
head/sys/sys/sysctl.h
Modified: head/sys/kern/kern_linker.c
==============================================================================
--- head/sys/kern/kern_linker.c Thu Oct 5 12:29:34 2017 (r324310)
+++ head/sys/kern/kern_linker.c Thu Oct 5 12:32:14 2017 (r324311)
@@ -288,7 +288,7 @@ linker_file_sysuninit(linker_file_t lf)
}
static void
-linker_file_register_sysctls(linker_file_t lf)
+linker_file_register_sysctls(linker_file_t lf, bool enable)
{
struct sysctl_oid **start, **stop, **oidp;
@@ -303,8 +303,34 @@ linker_file_register_sysctls(linker_file_t lf)
sx_xunlock(&kld_sx);
sysctl_wlock();
+ for (oidp = start; oidp < stop; oidp++) {
+ if (enable)
+ sysctl_register_oid(*oidp);
+ else
+ sysctl_register_disabled_oid(*oidp);
+ }
+ sysctl_wunlock();
+ sx_xlock(&kld_sx);
+}
+
+static void
+linker_file_enable_sysctls(linker_file_t lf)
+{
+ struct sysctl_oid **start, **stop, **oidp;
+
+ KLD_DPF(FILE,
+ ("linker_file_enable_sysctls: enable SYSCTLs for %s\n",
+ lf->filename));
+
+ sx_assert(&kld_sx, SA_XLOCKED);
+
+ if (linker_file_lookup_set(lf, "sysctl_set", &start, &stop, NULL) != 0)
+ return;
+
+ sx_xunlock(&kld_sx);
+ sysctl_wlock();
for (oidp = start; oidp < stop; oidp++)
- sysctl_register_oid(*oidp);
+ sysctl_enable_oid(*oidp);
sysctl_wunlock();
sx_xlock(&kld_sx);
}
@@ -430,7 +456,7 @@ linker_load_file(const char *filename, linker_file_t *
return (error);
}
modules = !TAILQ_EMPTY(&lf->modules);
- linker_file_register_sysctls(lf);
+ linker_file_register_sysctls(lf, false);
linker_file_sysinit(lf);
lf->flags |= LINKER_FILE_LINKED;
@@ -443,6 +469,7 @@ linker_load_file(const char *filename, linker_file_t *
linker_file_unload(lf, LINKER_UNLOAD_FORCE);
return (ENOEXEC);
}
+ linker_file_enable_sysctls(lf);
EVENTHANDLER_INVOKE(kld_load, lf);
*result = lf;
return (0);
@@ -692,8 +719,8 @@ linker_file_unload(linker_file_t file, int flags)
*/
if (file->flags & LINKER_FILE_LINKED) {
file->flags &= ~LINKER_FILE_LINKED;
- linker_file_sysuninit(file);
linker_file_unregister_sysctls(file);
+ linker_file_sysuninit(file);
}
TAILQ_REMOVE(&linker_files, file, link);
@@ -1642,7 +1669,7 @@ restart:
if (linker_file_lookup_set(lf, "sysinit_set", &si_start,
&si_stop, NULL) == 0)
sysinit_add(si_start, si_stop);
- linker_file_register_sysctls(lf);
+ linker_file_register_sysctls(lf, true);
lf->flags |= LINKER_FILE_LINKED;
continue;
fail:
Modified: head/sys/kern/kern_sysctl.c
==============================================================================
--- head/sys/kern/kern_sysctl.c Thu Oct 5 12:29:34 2017 (r324310)
+++ head/sys/kern/kern_sysctl.c Thu Oct 5 12:32:14 2017 (r324311)
@@ -510,6 +510,37 @@ retry:
}
void
+sysctl_register_disabled_oid(struct sysctl_oid *oidp)
+{
+
+ /*
+ * Mark the leaf as dormant if it's not to be immediately enabled.
+ * We do not disable nodes as they can be shared between modules
+ * and it is always safe to access a node.
+ */
+ KASSERT((oidp->oid_kind & CTLFLAG_DORMANT) == 0,
+ ("internal flag is set in oid_kind"));
+ if ((oidp->oid_kind & CTLTYPE) != CTLTYPE_NODE)
+ oidp->oid_kind |= CTLFLAG_DORMANT;
+ sysctl_register_oid(oidp);
+}
+
+void
+sysctl_enable_oid(struct sysctl_oid *oidp)
+{
+
+ SYSCTL_ASSERT_WLOCKED();
+ if ((oidp->oid_kind & CTLTYPE) == CTLTYPE_NODE) {
+ KASSERT((oidp->oid_kind & CTLFLAG_DORMANT) == 0,
+ ("sysctl node is marked as dormant"));
+ return;
+ }
+ KASSERT((oidp->oid_kind & CTLFLAG_DORMANT) != 0,
+ ("enabling already enabled sysctl oid"));
+ oidp->oid_kind &= ~CTLFLAG_DORMANT;
+}
+
+void
sysctl_unregister_oid(struct sysctl_oid *oidp)
{
struct sysctl_oid *p;
@@ -1057,7 +1088,7 @@ sysctl_sysctl_next_ls(struct sysctl_oid_list *lsp, int
*next = oidp->oid_number;
*oidpp = oidp;
- if (oidp->oid_kind & CTLFLAG_SKIP)
+ if ((oidp->oid_kind & (CTLFLAG_SKIP | CTLFLAG_DORMANT)) != 0)
continue;
if (!namelen) {
@@ -1878,6 +1909,8 @@ sysctl_find_oid(int *name, u_int namelen, struct sysct
}
lsp = SYSCTL_CHILDREN(oid);
} else if (indx == namelen) {
+ if ((oid->oid_kind & CTLFLAG_DORMANT) != 0)
+ return (ENOENT);
*noid = oid;
if (nindx != NULL)
*nindx = indx;
Modified: head/sys/sys/sysctl.h
==============================================================================
--- head/sys/sys/sysctl.h Thu Oct 5 12:29:34 2017 (r324310)
+++ head/sys/sys/sysctl.h Thu Oct 5 12:32:14 2017 (r324311)
@@ -83,6 +83,7 @@ struct ctlname {
#define CTLFLAG_RD 0x80000000 /* Allow reads of variable */
#define CTLFLAG_WR 0x40000000 /* Allow writes to the variable
*/
#define CTLFLAG_RW (CTLFLAG_RD|CTLFLAG_WR)
+#define CTLFLAG_DORMANT 0x20000000 /* This sysctl is not active
yet */
#define CTLFLAG_ANYBODY 0x10000000 /* All users can set this var */
#define CTLFLAG_SECURE 0x08000000 /* Permit set only if
securelevel<=0 */
#define CTLFLAG_PRISON 0x04000000 /* Prisoned roots can fiddle */
@@ -219,6 +220,8 @@ int sysctl_dpcpu_quad(SYSCTL_HANDLER_ARGS);
* These functions are used to add/remove an oid from the mib.
*/
void sysctl_register_oid(struct sysctl_oid *oidp);
+void sysctl_register_disabled_oid(struct sysctl_oid *oidp);
+void sysctl_enable_oid(struct sysctl_oid *oidp);
void sysctl_unregister_oid(struct sysctl_oid *oidp);
/* Declare a static oid to allow child oids to be added to it. */
_______________________________________________
svn-src-all@freebsd.org mailing list
https://lists.freebsd.org/mailman/listinfo/svn-src-all
To unsubscribe, send any mail to "svn-src-all-unsubscr...@freebsd.org"
