Make sysctl_int() and sysctl_int_lower() mp-safe and unlock KERN_HOSTID.

The only difference between sysctl_int() and sysctl_int_bounded()
is the range check, so sysctl_int() is just sysctl_int_bounded(...,
INT_MIN, INT_MAX). sysctl_int() is not the fast path, so this useless
check is not significant.

Mp-safe sysctl_int() is meaningless for sysctl_int_lower(), so rework it
in the sysctl_int_bounded() style. This time all affected paths are
kernel locked, but this doesn't make sysctl_int_lower() worse.

Change `hostid' type to the type of int. It only stored but never used
within kernel, userland accesses it through sysctl_int(). Nothing
changes, but variable becomes consistent with sysctl_int().

ok bluhm

diff --git a/sys/kern/kern_sysctl.c b/sys/kern/kern_sysctl.c
index 8cb4772..9c4a647 100644 (file)
--- a/sys/kern/kern_sysctl.c
+++ b/sys/kern/kern_sysctl.c
@@ -1,4 +1,4 @@
-/*     $OpenBSD: kern_sysctl.c,v 1.437 2024/08/11 15:10:53 mvs Exp $   */
+/*     $OpenBSD: kern_sysctl.c,v 1.438 2024/08/14 13:54:08 mvs Exp $   */
 /*     $NetBSD: kern_sysctl.c,v 1.17 1996/05/20 17:49:05 mrg Exp $     */
 
 /*-
@@ -306,7 +306,7 @@ char hostname[MAXHOSTNAMELEN];
 int hostnamelen;
 char domainname[MAXHOSTNAMELEN];
 int domainnamelen;
-long hostid;
+int hostid;
 char *disknames = NULL;
 size_t disknameslen;
 struct diskstats *diskstats = NULL;
@@ -507,6 +507,8 @@ kern_sysctl(int *name, u_int namelen, void *oldp, size_t *oldlenp, void *newp,
                return (sysctl_rdstring(oldp, oldlenp, newp, version));
        case KERN_NUMVNODES:  /* XXX numvnodes is a long */
                return (sysctl_rdint(oldp, oldlenp, newp, numvnodes));
+       case KERN_HOSTID:
+               return (sysctl_int(oldp, oldlenp, newp, newlen, &hostid));
        case KERN_CLOCKRATE:
                return (sysctl_clockrate(oldp, oldlenp, newp));
        case KERN_BOOTTIME: {
@@ -585,7 +587,7 @@ int
 kern_sysctl_locked(int *name, u_int namelen, void *oldp, size_t *oldlenp,
     void *newp, size_t newlen, struct proc *p)
 {
-       int error, level, inthostid, stackgap;
+       int error, level, stackgap;
        dev_t dev;
        extern int pool_debug;
 
@@ -623,11 +625,6 @@ kern_sysctl_locked(int *name, u_int namelen, void *oldp, size_t *oldlenp,
                if (newp && !error)
                        domainnamelen = newlen;
                return (error);
-       case KERN_HOSTID:
-               inthostid = hostid;  /* XXX assumes sizeof long <= sizeof int */
-               error =  sysctl_int(oldp, oldlenp, newp, newlen, &inthostid);
-               hostid = inthostid;
-               return (error);
        case KERN_CONSBUF:
                if ((error = suser(p)))
                        return (error);
@@ -1055,17 +1052,36 @@ int
 sysctl_int_lower(void *oldp, size_t *oldlenp, void *newp, size_t newlen,
     int *valp)
 {
-       unsigned int oval = *valp, val = *valp;
+       unsigned int oldval, newval;
        int error;
 
-       if (newp == NULL)
-               return (sysctl_rdint(oldp, oldlenp, newp, val));
+       if (oldp && *oldlenp < sizeof(int))
+               return (ENOMEM);
+       if (newp && newlen != sizeof(int))
+               return (EINVAL);
+       *oldlenp = sizeof(int);
+
+       if (newp) {
+               if ((error = copyin(newp, &newval, sizeof(int))))
+                       return (error);
+               do {
+                       oldval = atomic_load_int(valp);
+                       if (oldval < (unsigned int)newval)
+                               return (EPERM); /* do not allow raising */
+               } while (atomic_cas_uint(valp, oldval, newval) != oldval);
+
+               if (oldp) {
+                       /* new value has been set although user gets error */
+                       if ((error = copyout(&oldval, oldp, sizeof(int))))
+                               return (error);
+               }
+       } else if (oldp) {
+               oldval = atomic_load_int(valp);
+
+               if ((error = copyout(&oldval, oldp, sizeof(int))))
+                       return (error); 
+       }
 
-       if ((error = sysctl_int(oldp, oldlenp, newp, newlen, &val)))
-               return (error);
-       if (val > oval)
-               return (EPERM);         /* do not allow raising */
-       *(unsigned int *)valp = val;
        return (0);
 }
 
@@ -1076,18 +1092,8 @@ sysctl_int_lower(void *oldp, size_t *oldlenp, void *newp, size_t newlen,
 int
 sysctl_int(void *oldp, size_t *oldlenp, void *newp, size_t newlen, int *valp)
 {
-       int error = 0;
-
-       if (oldp && *oldlenp < sizeof(int))
-               return (ENOMEM);
-       if (newp && newlen != sizeof(int))
-               return (EINVAL);
-       *oldlenp = sizeof(int);
-       if (oldp)
-               error = copyout(valp, oldp, sizeof(int));
-       if (error == 0 && newp)
-               error = copyin(newp, valp, sizeof(int));
-       return (error);
+       return (sysctl_int_bounded(oldp, oldlenp, newp, newlen, valp,
+           INT_MIN, INT_MAX));
 }
 
 /*

diff --git a/sys/sys/kernel.h b/sys/sys/kernel.h
index 0147fde..41f4f40 100644 (file)
--- a/sys/sys/kernel.h
+++ b/sys/sys/kernel.h
@@ -1,4 +1,4 @@
-/*     $OpenBSD: kernel.h,v 1.26 2023/03/03 20:16:44 cheloha Exp $     */
+/*     $OpenBSD: kernel.h,v 1.27 2024/08/14 13:54:08 mvs Exp $ */
 /*     $NetBSD: kernel.h,v 1.11 1995/03/03 01:24:16 cgd Exp $  */
 
 /*-
@@ -40,7 +40,7 @@
 /* Global variables for the kernel. */
 
 /* 1.1 */
-extern long hostid;
+extern int hostid;
 extern char hostname[MAXHOSTNAMELEN];
 extern int hostnamelen;
 extern char domainname[MAXHOSTNAMELEN];