Date: Tue, 10 Mar 2009 17:28:23 +0000 (UTC) From: John Baldwin <jhb@FreeBSD.org> To: src-committers@freebsd.org, svn-src-all@freebsd.org, svn-src-stable@freebsd.org, svn-src-stable-7@freebsd.org Subject: svn commit: r189634 - in stable/7/sys: . compat/freebsd32 contrib/pf dev/cxgb i386/ibcs2 kern Message-ID: <200903101728.n2AHSNFH068714@svn.freebsd.org>
next in thread | raw e-mail | index | archive | help
Author: jhb Date: Tue Mar 10 17:28:23 2009 New Revision: 189634 URL: http://svn.freebsd.org/changeset/base/189634 Log: MFC: Push down Giant inside sysctl. Modified: stable/7/sys/ (props changed) stable/7/sys/compat/freebsd32/freebsd32_misc.c stable/7/sys/contrib/pf/ (props changed) stable/7/sys/dev/cxgb/ (props changed) stable/7/sys/i386/ibcs2/ibcs2_sysi86.c stable/7/sys/kern/kern_sysctl.c stable/7/sys/kern/kern_xxx.c Modified: stable/7/sys/compat/freebsd32/freebsd32_misc.c ============================================================================== --- stable/7/sys/compat/freebsd32/freebsd32_misc.c Tue Mar 10 17:19:45 2009 (r189633) +++ stable/7/sys/compat/freebsd32/freebsd32_misc.c Tue Mar 10 17:28:23 2009 (r189634) @@ -1966,7 +1966,6 @@ freebsd32_sysctl(struct thread *td, stru error = copyin(uap->name, name, uap->namelen * sizeof(int)); if (error) return (error); - mtx_lock(&Giant); if (uap->oldlenp) oldlen = fuword32(uap->oldlenp); else @@ -1975,12 +1974,10 @@ freebsd32_sysctl(struct thread *td, stru 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 Modified: stable/7/sys/i386/ibcs2/ibcs2_sysi86.c ============================================================================== --- stable/7/sys/i386/ibcs2/ibcs2_sysi86.c Tue Mar 10 17:19:45 2009 (r189633) +++ stable/7/sys/i386/ibcs2/ibcs2_sysi86.c Tue Mar 10 17:28:23 2009 (r189634) @@ -74,15 +74,11 @@ ibcs2_sysi86(struct thread *td, struct i case SETNAME: { /* set hostname given string w/ len <= 7 chars */ int name[2]; - int error; name[0] = CTL_KERN; name[1] = KERN_HOSTNAME; - mtx_lock(&Giant); - error = userland_sysctl(td, name, 2, 0, 0, 0, - args->arg, 7, 0, 0); - mtx_unlock(&Giant); - return (error); + return (userland_sysctl(td, name, 2, 0, 0, 0, + args->arg, 7, 0, 0)); } case SI86_MEM: /* size of physical memory */ Modified: stable/7/sys/kern/kern_sysctl.c ============================================================================== --- stable/7/sys/kern/kern_sysctl.c Tue Mar 10 17:19:45 2009 (r189633) +++ stable/7/sys/kern/kern_sysctl.c Tue Mar 10 17:28:23 2009 (r189634) @@ -70,6 +70,7 @@ static struct sx sysctllock; #define SYSCTL_LOCK() sx_xlock(&sysctllock) #define SYSCTL_UNLOCK() sx_xunlock(&sysctllock) +#define SYSCTL_LOCK_ASSERT() sx_assert(&sysctllock, SX_XLOCKED) #define SYSCTL_INIT() sx_init(&sysctllock, "sysctl lock") static int sysctl_root(SYSCTL_HANDLER_ARGS); @@ -666,6 +667,8 @@ name2oid (char *name, int *oid, int *len struct sysctl_oid_list *lsp = &sysctl__children; char *p; + SYSCTL_LOCK_ASSERT(); + if (!*name) return (ENOENT); @@ -722,6 +725,8 @@ sysctl_sysctl_name2oid(SYSCTL_HANDLER_AR int error, oid[CTL_MAXNAME], len; struct sysctl_oid *op = 0; + SYSCTL_LOCK_ASSERT(); + if (!req->newlen) return (ENOENT); if (req->newlen >= MAXPATHLEN) /* XXX arbitrary, undocumented */ @@ -1066,14 +1071,12 @@ kernel_sysctl(struct thread *td, int *na req.lock = REQ_LOCKED; SYSCTL_LOCK(); - error = sysctl_root(0, name, namelen, &req); + SYSCTL_UNLOCK(); if (req.lock == REQ_WIRED && req.validlen > 0) vsunlock(req.oldptr, req.validlen); - SYSCTL_UNLOCK(); - if (error && error != ENOMEM) return (error); @@ -1098,6 +1101,11 @@ kernel_sysctlbyname(struct thread *td, c oid[1] = 3; /* name2oid */ oidlen = sizeof(oid); + /* + * XXX: Prone to a possible race condition between lookup and + * execution? Maybe put locking around it? + */ + error = kernel_sysctl(td, oid, 2, oid, &oidlen, (void *)name, strlen(name), &plen, flags); if (error) @@ -1250,6 +1258,8 @@ sysctl_root(SYSCTL_HANDLER_ARGS) struct sysctl_oid *oid; int error, indx, lvl; + SYSCTL_LOCK_ASSERT(); + error = sysctl_find_oid(arg1, arg2, &oid, &indx, req); if (error) return (error); @@ -1304,7 +1314,11 @@ sysctl_root(SYSCTL_HANDLER_ARGS) if (error != 0) return (error); #endif + + /* XXX: Handlers are not guaranteed to be Giant safe! */ + mtx_lock(&Giant); error = oid->oid_handler(oid, arg1, arg2, req); + mtx_unlock(&Giant); return (error); } @@ -1332,20 +1346,16 @@ __sysctl(struct thread *td, struct sysct if (error) return (error); - mtx_lock(&Giant); - error = userland_sysctl(td, name, uap->namelen, uap->old, uap->oldlenp, 0, uap->new, uap->newlen, &j, 0); if (error && error != ENOMEM) - goto done2; + return (error); if (uap->oldlenp) { int i = copyout(&j, uap->oldlenp, sizeof(j)); if (i) - error = i; + return (i); } -done2: - mtx_unlock(&Giant); return (error); } @@ -1405,11 +1415,11 @@ userland_sysctl(struct thread *td, int * uio_yield(); } + SYSCTL_UNLOCK(); + if (req.lock == REQ_WIRED && req.validlen > 0) vsunlock(req.oldptr, req.validlen); - SYSCTL_UNLOCK(); - if (error && error != ENOMEM) return (error); @@ -1421,217 +1431,3 @@ userland_sysctl(struct thread *td, int * } return (error); } - -#ifdef COMPAT_43 -#include <sys/socket.h> -#include <vm/vm_param.h> - -#define KINFO_PROC (0<<8) -#define KINFO_RT (1<<8) -#define KINFO_VNODE (2<<8) -#define KINFO_FILE (3<<8) -#define KINFO_METER (4<<8) -#define KINFO_LOADAVG (5<<8) -#define KINFO_CLOCKRATE (6<<8) - -/* Non-standard BSDI extension - only present on their 4.3 net-2 releases */ -#define KINFO_BSDI_SYSINFO (101<<8) - -/* - * XXX this is bloat, but I hope it's better here than on the potentially - * limited kernel stack... -Peter - */ - -static struct { - int bsdi_machine; /* "i386" on BSD/386 */ -/* ^^^ this is an offset to the string, relative to the struct start */ - char *pad0; - long pad1; - long pad2; - long pad3; - u_long pad4; - u_long pad5; - u_long pad6; - - int bsdi_ostype; /* "BSD/386" on BSD/386 */ - int bsdi_osrelease; /* "1.1" on BSD/386 */ - long pad7; - long pad8; - char *pad9; - - long pad10; - long pad11; - int pad12; - long pad13; - quad_t pad14; - long pad15; - - struct timeval pad16; - /* we dont set this, because BSDI's uname used gethostname() instead */ - int bsdi_hostname; /* hostname on BSD/386 */ - - /* the actual string data is appended here */ - -} bsdi_si; - -/* - * this data is appended to the end of the bsdi_si structure during copyout. - * The "char *" offsets are relative to the base of the bsdi_si struct. - * This contains "FreeBSD\02.0-BUILT-nnnnnn\0i386\0", and these strings - * should not exceed the length of the buffer here... (or else!! :-) - */ -static char bsdi_strings[80]; /* It had better be less than this! */ - -#ifndef _SYS_SYSPROTO_H_ -struct getkerninfo_args { - int op; - char *where; - size_t *size; - int arg; -}; -#endif -int -ogetkerninfo(struct thread *td, struct getkerninfo_args *uap) -{ - int error, name[6]; - size_t size; - u_int needed = 0; - - mtx_lock(&Giant); - - switch (uap->op & 0xff00) { - - case KINFO_RT: - name[0] = CTL_NET; - name[1] = PF_ROUTE; - name[2] = 0; - name[3] = (uap->op & 0xff0000) >> 16; - name[4] = uap->op & 0xff; - name[5] = uap->arg; - error = userland_sysctl(td, name, 6, uap->where, uap->size, - 0, 0, 0, &size, 0); - break; - - case KINFO_VNODE: - name[0] = CTL_KERN; - name[1] = KERN_VNODE; - error = userland_sysctl(td, name, 2, uap->where, uap->size, - 0, 0, 0, &size, 0); - break; - - case KINFO_PROC: - name[0] = CTL_KERN; - name[1] = KERN_PROC; - name[2] = uap->op & 0xff; - name[3] = uap->arg; - error = userland_sysctl(td, name, 4, uap->where, uap->size, - 0, 0, 0, &size, 0); - break; - - case KINFO_FILE: - name[0] = CTL_KERN; - name[1] = KERN_FILE; - error = userland_sysctl(td, name, 2, uap->where, uap->size, - 0, 0, 0, &size, 0); - break; - - case KINFO_METER: - name[0] = CTL_VM; - name[1] = VM_TOTAL; - error = userland_sysctl(td, name, 2, uap->where, uap->size, - 0, 0, 0, &size, 0); - break; - - case KINFO_LOADAVG: - name[0] = CTL_VM; - name[1] = VM_LOADAVG; - error = userland_sysctl(td, name, 2, uap->where, uap->size, - 0, 0, 0, &size, 0); - break; - - case KINFO_CLOCKRATE: - name[0] = CTL_KERN; - name[1] = KERN_CLOCKRATE; - error = userland_sysctl(td, name, 2, uap->where, uap->size, - 0, 0, 0, &size, 0); - break; - - case KINFO_BSDI_SYSINFO: { - /* - * this is pretty crude, but it's just enough for uname() - * from BSDI's 1.x libc to work. - * - * *size gives the size of the buffer before the call, and - * the amount of data copied after a successful call. - * If successful, the return value is the amount of data - * available, which can be larger than *size. - * - * BSDI's 2.x product apparently fails with ENOMEM if *size - * is too small. - */ - - u_int left; - char *s; - - bzero((char *)&bsdi_si, sizeof(bsdi_si)); - bzero(bsdi_strings, sizeof(bsdi_strings)); - - s = bsdi_strings; - - bsdi_si.bsdi_ostype = (s - bsdi_strings) + sizeof(bsdi_si); - strcpy(s, ostype); - s += strlen(s) + 1; - - bsdi_si.bsdi_osrelease = (s - bsdi_strings) + sizeof(bsdi_si); - strcpy(s, osrelease); - s += strlen(s) + 1; - - bsdi_si.bsdi_machine = (s - bsdi_strings) + sizeof(bsdi_si); - strcpy(s, machine); - s += strlen(s) + 1; - - needed = sizeof(bsdi_si) + (s - bsdi_strings); - - if ((uap->where == NULL) || (uap->size == NULL)) { - /* process is asking how much buffer to supply.. */ - size = needed; - error = 0; - break; - } - - if ((error = copyin(uap->size, &size, sizeof(size))) != 0) - break; - - /* if too much buffer supplied, trim it down */ - if (size > needed) - size = needed; - - /* how much of the buffer is remaining */ - left = size; - - if ((error = copyout((char *)&bsdi_si, uap->where, left)) != 0) - break; - - /* is there any point in continuing? */ - if (left > sizeof(bsdi_si)) { - left -= sizeof(bsdi_si); - error = copyout(&bsdi_strings, - uap->where + sizeof(bsdi_si), left); - } - break; - } - - default: - error = EOPNOTSUPP; - break; - } - if (error == 0) { - td->td_retval[0] = needed ? needed : size; - if (uap->size) { - error = copyout(&size, uap->size, sizeof(size)); - } - } - mtx_unlock(&Giant); - return (error); -} -#endif /* COMPAT_43 */ Modified: stable/7/sys/kern/kern_xxx.c ============================================================================== --- stable/7/sys/kern/kern_xxx.c Tue Mar 10 17:19:45 2009 (r189633) +++ stable/7/sys/kern/kern_xxx.c Tue Mar 10 17:28:23 2009 (r189634) @@ -42,9 +42,11 @@ __FBSDID("$FreeBSD$"); #include <sys/proc.h> #include <sys/lock.h> #include <sys/mutex.h> +#include <sys/socket.h> #include <sys/sysctl.h> #include <sys/utsname.h> +#include <vm/vm_param.h> #if defined(COMPAT_43) @@ -61,16 +63,12 @@ ogethostname(td, uap) struct gethostname_args *uap; { int name[2]; - int error; size_t len = uap->len; name[0] = CTL_KERN; name[1] = KERN_HOSTNAME; - mtx_lock(&Giant); - error = userland_sysctl(td, name, 2, uap->hostname, &len, - 1, 0, 0, 0, 0); - mtx_unlock(&Giant); - return(error); + return (userland_sysctl(td, name, 2, uap->hostname, &len, + 1, 0, 0, 0, 0)); } #ifndef _SYS_SYSPROTO_H_ @@ -86,15 +84,11 @@ osethostname(td, uap) register struct sethostname_args *uap; { int name[2]; - int error; name[0] = CTL_KERN; name[1] = KERN_HOSTNAME; - mtx_lock(&Giant); - error = userland_sysctl(td, name, 2, 0, 0, 0, uap->hostname, - uap->len, 0, 0); - mtx_unlock(&Giant); - return (error); + return (userland_sysctl(td, name, 2, 0, 0, 0, uap->hostname, + uap->len, 0, 0)); } #ifndef _SYS_SYSPROTO_H_ @@ -145,6 +139,212 @@ oquota(td, uap) return (ENOSYS); } + +#define KINFO_PROC (0<<8) +#define KINFO_RT (1<<8) +#define KINFO_VNODE (2<<8) +#define KINFO_FILE (3<<8) +#define KINFO_METER (4<<8) +#define KINFO_LOADAVG (5<<8) +#define KINFO_CLOCKRATE (6<<8) + +/* Non-standard BSDI extension - only present on their 4.3 net-2 releases */ +#define KINFO_BSDI_SYSINFO (101<<8) + +/* + * XXX this is bloat, but I hope it's better here than on the potentially + * limited kernel stack... -Peter + */ + +static struct { + int bsdi_machine; /* "i386" on BSD/386 */ +/* ^^^ this is an offset to the string, relative to the struct start */ + char *pad0; + long pad1; + long pad2; + long pad3; + u_long pad4; + u_long pad5; + u_long pad6; + + int bsdi_ostype; /* "BSD/386" on BSD/386 */ + int bsdi_osrelease; /* "1.1" on BSD/386 */ + long pad7; + long pad8; + char *pad9; + + long pad10; + long pad11; + int pad12; + long pad13; + quad_t pad14; + long pad15; + + struct timeval pad16; + /* we dont set this, because BSDI's uname used gethostname() instead */ + int bsdi_hostname; /* hostname on BSD/386 */ + + /* the actual string data is appended here */ + +} bsdi_si; + +/* + * this data is appended to the end of the bsdi_si structure during copyout. + * The "char *" offsets are relative to the base of the bsdi_si struct. + * This contains "FreeBSD\02.0-BUILT-nnnnnn\0i386\0", and these strings + * should not exceed the length of the buffer here... (or else!! :-) + */ +static char bsdi_strings[80]; /* It had better be less than this! */ + +#ifndef _SYS_SYSPROTO_H_ +struct getkerninfo_args { + int op; + char *where; + size_t *size; + int arg; +}; +#endif +int +ogetkerninfo(struct thread *td, struct getkerninfo_args *uap) +{ + int error, name[6]; + size_t size; + u_int needed = 0; + + switch (uap->op & 0xff00) { + + case KINFO_RT: + name[0] = CTL_NET; + name[1] = PF_ROUTE; + name[2] = 0; + name[3] = (uap->op & 0xff0000) >> 16; + name[4] = uap->op & 0xff; + name[5] = uap->arg; + error = userland_sysctl(td, name, 6, uap->where, uap->size, + 0, 0, 0, &size, 0); + break; + + case KINFO_VNODE: + name[0] = CTL_KERN; + name[1] = KERN_VNODE; + error = userland_sysctl(td, name, 2, uap->where, uap->size, + 0, 0, 0, &size, 0); + break; + + case KINFO_PROC: + name[0] = CTL_KERN; + name[1] = KERN_PROC; + name[2] = uap->op & 0xff; + name[3] = uap->arg; + error = userland_sysctl(td, name, 4, uap->where, uap->size, + 0, 0, 0, &size, 0); + break; + + case KINFO_FILE: + name[0] = CTL_KERN; + name[1] = KERN_FILE; + error = userland_sysctl(td, name, 2, uap->where, uap->size, + 0, 0, 0, &size, 0); + break; + + case KINFO_METER: + name[0] = CTL_VM; + name[1] = VM_TOTAL; + error = userland_sysctl(td, name, 2, uap->where, uap->size, + 0, 0, 0, &size, 0); + break; + + case KINFO_LOADAVG: + name[0] = CTL_VM; + name[1] = VM_LOADAVG; + error = userland_sysctl(td, name, 2, uap->where, uap->size, + 0, 0, 0, &size, 0); + break; + + case KINFO_CLOCKRATE: + name[0] = CTL_KERN; + name[1] = KERN_CLOCKRATE; + error = userland_sysctl(td, name, 2, uap->where, uap->size, + 0, 0, 0, &size, 0); + break; + + case KINFO_BSDI_SYSINFO: { + /* + * this is pretty crude, but it's just enough for uname() + * from BSDI's 1.x libc to work. + * + * *size gives the size of the buffer before the call, and + * the amount of data copied after a successful call. + * If successful, the return value is the amount of data + * available, which can be larger than *size. + * + * BSDI's 2.x product apparently fails with ENOMEM if *size + * is too small. + */ + + u_int left; + char *s; + + bzero((char *)&bsdi_si, sizeof(bsdi_si)); + bzero(bsdi_strings, sizeof(bsdi_strings)); + + s = bsdi_strings; + + bsdi_si.bsdi_ostype = (s - bsdi_strings) + sizeof(bsdi_si); + strcpy(s, ostype); + s += strlen(s) + 1; + + bsdi_si.bsdi_osrelease = (s - bsdi_strings) + sizeof(bsdi_si); + strcpy(s, osrelease); + s += strlen(s) + 1; + + bsdi_si.bsdi_machine = (s - bsdi_strings) + sizeof(bsdi_si); + strcpy(s, machine); + s += strlen(s) + 1; + + needed = sizeof(bsdi_si) + (s - bsdi_strings); + + if ((uap->where == NULL) || (uap->size == NULL)) { + /* process is asking how much buffer to supply.. */ + size = needed; + error = 0; + break; + } + + if ((error = copyin(uap->size, &size, sizeof(size))) != 0) + break; + + /* if too much buffer supplied, trim it down */ + if (size > needed) + size = needed; + + /* how much of the buffer is remaining */ + left = size; + + if ((error = copyout((char *)&bsdi_si, uap->where, left)) != 0) + break; + + /* is there any point in continuing? */ + if (left > sizeof(bsdi_si)) { + left -= sizeof(bsdi_si); + error = copyout(&bsdi_strings, + uap->where + sizeof(bsdi_si), left); + } + break; + } + + default: + error = EOPNOTSUPP; + break; + } + if (error == 0) { + td->td_retval[0] = needed ? needed : size; + if (uap->size) { + error = copyout(&size, uap->size, sizeof(size)); + } + } + return (error); +} #endif /* COMPAT_43 */ /* @@ -173,11 +373,10 @@ uname(td, uap) name[0] = CTL_KERN; name[1] = KERN_OSTYPE; len = sizeof (uap->name->sysname); - mtx_lock(&Giant); error = userland_sysctl(td, name, 2, uap->name->sysname, &len, 1, 0, 0, 0, 0); if (error) - goto done2; + return (error); subyte( uap->name->sysname + sizeof(uap->name->sysname) - 1, 0); name[1] = KERN_HOSTNAME; @@ -185,7 +384,7 @@ uname(td, uap) error = userland_sysctl(td, name, 2, uap->name->nodename, &len, 1, 0, 0, 0, 0); if (error) - goto done2; + return (error); subyte( uap->name->nodename + sizeof(uap->name->nodename) - 1, 0); name[1] = KERN_OSRELEASE; @@ -193,7 +392,7 @@ uname(td, uap) error = userland_sysctl(td, name, 2, uap->name->release, &len, 1, 0, 0, 0, 0); if (error) - goto done2; + return (error); subyte( uap->name->release + sizeof(uap->name->release) - 1, 0); /* @@ -202,7 +401,7 @@ uname(td, uap) error = userland_sysctl(td, name, 2, uap->name->version, &len, 1, 0, 0, 0, 0); if (error) - goto done2; + return (error); subyte( uap->name->version + sizeof(uap->name->version) - 1, 0); */ @@ -214,11 +413,11 @@ uname(td, uap) for(us = uap->name->version; *s && *s != ':'; s++) { error = subyte( us++, *s); if (error) - goto done2; + return (error); } error = subyte( us++, 0); if (error) - goto done2; + return (error); name[0] = CTL_HW; name[1] = HW_MACHINE; @@ -226,11 +425,9 @@ uname(td, uap) error = userland_sysctl(td, name, 2, uap->name->machine, &len, 1, 0, 0, 0, 0); if (error) - goto done2; + return (error); subyte( uap->name->machine + sizeof(uap->name->machine) - 1, 0); -done2: - mtx_unlock(&Giant); - return (error); + return (0); } #ifndef _SYS_SYSPROTO_H_
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?200903101728.n2AHSNFH068714>