As I was staring a bit more at sysctl related code this pattern caught my attention. Looks like a few lines can disappear and hopefully code expressivity goes up.
Anybody like this? >From 3b52f9ad743fe9b59316077478d33f77fff9a119 Mon Sep 17 00:00:00 2001 From: Greg Steuck <[email protected]> Date: Sun, 23 Jan 2022 22:05:45 -0800 Subject: [PATCH] Capture a repeated pattern into sysctl_securelevel_int function A few variables in the kernel are only writeable before securelevel is raised. It makes sense to handle them with less code. --- sys/arch/amd64/amd64/machdep.c | 8 ++------ sys/arch/i386/i386/machdep.c | 8 ++------ sys/kern/kern_sysctl.c | 33 +++++++++++++++++++-------------- sys/kern/vfs_subr.c | 5 ++--- sys/netinet/ip_input.c | 7 +------ sys/sys/sysctl.h | 1 + 6 files changed, 27 insertions(+), 35 deletions(-) diff --git a/sys/arch/amd64/amd64/machdep.c b/sys/arch/amd64/amd64/machdep.c index 326f1c10253..4f9585510a1 100644 --- a/sys/arch/amd64/amd64/machdep.c +++ b/sys/arch/amd64/amd64/machdep.c @@ -513,12 +513,8 @@ cpu_sysctl(int *name, u_int namelen, void *oldp, size_t *oldlenp, void *newp, case CPU_CPUVENDOR: return (sysctl_rdstring(oldp, oldlenp, newp, cpu_vendor)); case CPU_KBDRESET: - if (securelevel > 0) - return (sysctl_rdint(oldp, oldlenp, newp, - kbd_reset)); - else - return (sysctl_int(oldp, oldlenp, newp, newlen, - &kbd_reset)); + return (sysctl_securelevel_int(oldp, oldlenp, newp, newlen, + &kbd_reset)); case CPU_ALLOWAPERTURE: if (namelen != 1) return (ENOTDIR); /* overloaded */ diff --git a/sys/arch/i386/i386/machdep.c b/sys/arch/i386/i386/machdep.c index 0d32cd4edd7..56e30fac9df 100644 --- a/sys/arch/i386/i386/machdep.c +++ b/sys/arch/i386/i386/machdep.c @@ -3617,12 +3617,8 @@ cpu_sysctl(int *name, u_int namelen, void *oldp, size_t *oldlenp, void *newp, case CPU_CPUFEATURE: return (sysctl_rdint(oldp, oldlenp, newp, curcpu()->ci_feature_flags)); case CPU_KBDRESET: - if (securelevel > 0) - return (sysctl_rdint(oldp, oldlenp, newp, - kbd_reset)); - else - return (sysctl_int(oldp, oldlenp, newp, newlen, - &kbd_reset)); + return (sysctl_securelevel_int(oldp, oldlenp, newp, newlen, + &kbd_reset)); #if NPCKBC > 0 && NUKBD > 0 case CPU_FORCEUKBD: { diff --git a/sys/kern/kern_sysctl.c b/sys/kern/kern_sysctl.c index e71cdf5897d..fe6a2689365 100644 --- a/sys/kern/kern_sysctl.c +++ b/sys/kern/kern_sysctl.c @@ -473,14 +473,12 @@ kern_sysctl(int *name, u_int namelen, void *oldp, size_t *oldlenp, void *newp, return (0); #if NDT > 0 case KERN_ALLOWDT: - if (securelevel > 0) - return (sysctl_rdint(oldp, oldlenp, newp, allowdt)); - return (sysctl_int(oldp, oldlenp, newp, newlen, &allowdt)); + return (sysctl_securelevel_int(oldp, oldlenp, newp, newlen, + &allowdt)); #endif case KERN_ALLOWKMEM: - if (securelevel > 0) - return (sysctl_rdint(oldp, oldlenp, newp, allowkmem)); - return (sysctl_int(oldp, oldlenp, newp, newlen, &allowkmem)); + return (sysctl_securelevel_int(oldp, oldlenp, newp, newlen, + &allowkmem)); case KERN_HOSTNAME: error = sysctl_tstring(oldp, oldlenp, newp, newlen, hostname, sizeof(hostname)); @@ -757,10 +755,7 @@ hw_sysctl(int *name, u_int namelen, void *oldp, size_t *oldlenp, void *newp, return (sysctl_rdquad(oldp, oldlenp, newp, ptoa((psize_t)physmem - uvmexp.wired))); case HW_ALLOWPOWERDOWN: - if (securelevel > 0) - return (sysctl_rdint(oldp, oldlenp, newp, - allowpowerdown)); - return (sysctl_int(oldp, oldlenp, newp, newlen, + return (sysctl_securelevel_int(oldp, oldlenp, newp, newlen, &allowpowerdown)); #ifdef __HAVE_CPU_TOPOLOGY case HW_SMT: @@ -875,6 +870,18 @@ sysctl_rdint(void *oldp, size_t *oldlenp, void *newp, int val) return (error); } +/* + * Selects between sysctl_rdint and sysctl_int according to securelevel. + */ +int +sysctl_securelevel_int(void *oldp, size_t *oldlenp, void *newp, size_t newlen, + int *valp) +{ + if (securelevel > 0) + return (sysctl_rdint(oldp, oldlenp, newp, *valp)); + return (sysctl_int(oldp, oldlenp, newp, newlen, valp)); +} + /* * Read-only or bounded integer values. */ @@ -2501,11 +2508,9 @@ sysctl_utc_offset(void *oldp, size_t *oldlenp, void *newp, size_t newlen) int adjustment_seconds, error, new_offset_minutes, old_offset_minutes; old_offset_minutes = utc_offset / 60; /* seconds -> minutes */ - if (securelevel > 0) - return sysctl_rdint(oldp, oldlenp, newp, old_offset_minutes); - new_offset_minutes = old_offset_minutes; - error = sysctl_int(oldp, oldlenp, newp, newlen, &new_offset_minutes); + error = sysctl_securelevel_int(oldp, oldlenp, newp, newlen, + &new_offset_minutes); if (error) return error; if (new_offset_minutes < -24 * 60 || new_offset_minutes > 24 * 60) diff --git a/sys/kern/vfs_subr.c b/sys/kern/vfs_subr.c index 43a7cbd4ae9..a60eb0397d7 100644 --- a/sys/kern/vfs_subr.c +++ b/sys/kern/vfs_subr.c @@ -1845,9 +1845,8 @@ fs_posix_sysctl(int *name, u_int namelen, void *oldp, size_t *oldlenp, switch (name[0]) { case FS_POSIX_SETUID: - if (newp && securelevel > 0) - return (EPERM); - return(sysctl_int(oldp, oldlenp, newp, newlen, &suid_clear)); + return (sysctl_securelevel_int(oldp, oldlenp, newp, newlen, + &suid_clear)); default: return (EOPNOTSUPP); } diff --git a/sys/netinet/ip_input.c b/sys/netinet/ip_input.c index 9e7e20af30b..2956f013f9f 100644 --- a/sys/netinet/ip_input.c +++ b/sys/netinet/ip_input.c @@ -1610,13 +1610,8 @@ ip_sysctl(int *name, u_int namelen, void *oldp, size_t *oldlenp, void *newp, switch (name[0]) { case IPCTL_SOURCEROUTE: - /* - * Don't allow this to change in a secure environment. - */ - if (newp && securelevel > 0) - return (EPERM); NET_LOCK(); - error = sysctl_int(oldp, oldlenp, newp, newlen, + error = sysctl_securelevel_int(oldp, oldlenp, newp, newlen, &ip_dosourceroute); NET_UNLOCK(); return (error); diff --git a/sys/sys/sysctl.h b/sys/sys/sysctl.h index 29f82bcece3..b8e9b1d13ac 100644 --- a/sys/sys/sysctl.h +++ b/sys/sys/sysctl.h @@ -1017,6 +1017,7 @@ typedef int (sysctlfn)(int *, u_int, void *, size_t *, void *, size_t, struct pr int sysctl_int_lower(void *, size_t *, void *, size_t, int *); int sysctl_int(void *, size_t *, void *, size_t, int *); int sysctl_rdint(void *, size_t *, void *, int); +int sysctl_securelevel_int(void *, size_t *, void *, size_t, int *); int sysctl_int_bounded(void *, size_t *, void *, size_t, int *, int, int); int sysctl_bounded_arr(const struct sysctl_bounded_args *, u_int, int *, u_int, void *, size_t *, void *, size_t); -- 2.34.1
