Push down Giant inside sysctl. Also add some more assertions to the code.

In the existing code we didn't really enforce that callers hold Giant
before calling userland_sysctl(), even though there is no guarantee it
is safe. Fix this by just placing Giant locks around the call to the oid
handler. This also means we only pick up Giant for a very short period
of time. Maybe we should add MPSAFE flags to sysctl or phase it out all
together.

I've also added SYSCTL_LOCK_ASSERT(). We have to make sure sysctl_root()
and name2oid() are called with the sysctl lock held.

Reviewed by:	Jille Timmermans <jille quis cx>

2 files changed, 6 insertions, 17 deletions

diff --git a/sys/compat/freebsd32/freebsd32_misc.c b/sys/compat/freebsd32/freebsd32_misc.c
index 2569ead..83fd67f 100644
--- a/sys/compat/freebsd32/freebsd32_misc.c
+++ b/sys/compat/freebsd32/freebsd32_misc.c
@@ -2019,7 +2019,6 @@ freebsd32_sysctl(struct thread *td, struct freebsd32_sysctl_args *uap)
  	error = copyin(uap->name, name, uap->namelen * sizeof(int));
  	if (error)
 		return (error);
-	mtx_lock(&Giant);
 	if (uap->oldlenp)
 		oldlen = fuword32(uap->oldlenp);
 	else
@@ -2028,12 +2027,10 @@ freebsd32_sysctl(struct thread *td, struct freebsd32_sysctl_args *uap)
 		uap->old, &oldlen, 1,
 		uap->new, uap->newlen, &j, SCTL_MASK32);
 	if (error && error != ENOMEM)
-		goto done2;
+		return (error);
 	if (uap->oldlenp)
 		suword32(uap->oldlenp, j);
-done2:
-	mtx_unlock(&Giant);
-	return (error);
+	return (0);
 }
 
 int
diff --git a/sys/compat/linux/linux_misc.c b/sys/compat/linux/linux_misc.c
index 93f4297..bdbb5dd 100644
--- a/sys/compat/linux/linux_misc.c
+++ b/sys/compat/linux/linux_misc.c
@@ -1682,7 +1682,6 @@ int
 linux_sethostname(struct thread *td, struct linux_sethostname_args *args)
 {
 	int name[2];
-	int error;
 
 #ifdef DEBUG
 	if (ldebug(sethostname))
@@ -1691,18 +1690,14 @@ linux_sethostname(struct thread *td, struct linux_sethostname_args *args)
 
 	name[0] = CTL_KERN;
 	name[1] = KERN_HOSTNAME;
-	mtx_lock(&Giant);
-	error = userland_sysctl(td, name, 2, 0, 0, 0, args->hostname,
-	    args->len, 0, 0);
-	mtx_unlock(&Giant);
-	return (error);
+	return (userland_sysctl(td, name, 2, 0, 0, 0, args->hostname,
+	    args->len, 0, 0));
 }
 
 int
 linux_setdomainname(struct thread *td, struct linux_setdomainname_args *args)
 {
 	int name[2];
-	int error;
 
 #ifdef DEBUG
 	if (ldebug(setdomainname))
@@ -1711,11 +1706,8 @@ linux_setdomainname(struct thread *td, struct linux_setdomainname_args *args)
 
 	name[0] = CTL_KERN;
 	name[1] = KERN_NISDOMAINNAME;
-	mtx_lock(&Giant);
-	error = userland_sysctl(td, name, 2, 0, 0, 0, args->name,
-	    args->len, 0, 0);
-	mtx_unlock(&Giant);
-	return (error);
+	return (userland_sysctl(td, name, 2, 0, 0, 0, args->name,
+	    args->len, 0, 0));
 }
 
 int