Subject: Re: kauth and sched_{get,set}param
To: <>
From: Elad Efrat <elad@NetBSD.org>
List: current-users
Date: 01/31/2008 16:41:39
This is a multi-part message in MIME format.
--------------030603010809070103040103
Content-Type: text/plain; charset=ISO-8859-1; format=flowed
Content-Transfer-Encoding: 7bit
Hey,
I've attached a patch, but if you can test it first (I don't have
schedctl here yet) it'd be great. Also, I'd appreciate review on
the locking as I changed the order in which things are done to first
grab the process pointer.
Hope this helps,
-e.
--------------030603010809070103040103
Content-Type: text/plain;
name="sys_sched.c.diff"
Content-Transfer-Encoding: 7bit
Content-Disposition: inline;
filename="sys_sched.c.diff"
Index: sys_sched.c
===================================================================
RCS file: /cvsroot/src/sys/kern/sys_sched.c,v
retrieving revision 1.9
diff -u -p -r1.9 sys_sched.c
--- sys_sched.c 31 Jan 2008 01:21:17 -0000 1.9
+++ sys_sched.c 31 Jan 2008 14:38:59 -0000
@@ -117,15 +117,29 @@ sys__sched_setparam(struct lwp *l, const
pri_t pri;
int error;
- /* Available only for super-user */
- if (kauth_authorize_process(l->l_cred, KAUTH_PROCESS_SCHEDULER,
- KAUTH_ARG(KAUTH_REQ_PROCESS_SCHEDULER_SETPARAM), NULL, NULL, NULL))
- return EPERM;
+ if (SCARG(uap, pid) != 0) {
+ /* Find the process */
+ p = p_find(SCARG(uap, pid), PFIND_UNLOCK_FAIL);
+ if (p == NULL)
+ return ESRCH;
+ mutex_enter(&p->p_smutex);
+ mutex_exit(&proclist_lock);
+ /* Disallow modification of system processes */
+ if (p->p_flag & PK_SYSTEM) {
+ mutex_exit(&p->p_smutex);
+ return EPERM;
+ }
+ } else {
+ /* Use the calling process */
+ p = l->l_proc;
+ mutex_enter(&p->p_smutex);
+ }
/* Get the parameters from the user-space */
sp = kmem_zalloc(sizeof(struct sched_param), KM_SLEEP);
error = copyin(SCARG(uap, params), sp, sizeof(struct sched_param));
if (error) {
+ mutex_exit(&p->p_smutex);
kmem_free(sp, sizeof(struct sched_param));
return error;
}
@@ -133,6 +147,13 @@ sys__sched_setparam(struct lwp *l, const
policy = sp->sched_class;
kmem_free(sp, sizeof(struct sched_param));
+ /* Available only for super-user */
+ if (kauth_authorize_process(l->l_cred, KAUTH_PROCESS_SCHEDULER, p,
+ KAUTH_ARG(KAUTH_REQ_PROCESS_SCHEDULER_SETPARAM), NULL, NULL)) {
+ mutex_exit(&p->p_smutex);
+ return EPERM;
+ }
+
/* If no parameters specified, just return (this should not happen) */
if (pri == PRI_NONE && policy == SCHED_NONE)
return 0;
@@ -145,24 +166,6 @@ sys__sched_setparam(struct lwp *l, const
if (pri != PRI_NONE && (pri < SCHED_PRI_MIN || pri > SCHED_PRI_MAX))
return EINVAL;
- if (SCARG(uap, pid) != 0) {
- /* Find the process */
- p = p_find(SCARG(uap, pid), PFIND_UNLOCK_FAIL);
- if (p == NULL)
- return ESRCH;
- mutex_enter(&p->p_smutex);
- mutex_exit(&proclist_lock);
- /* Disallow modification of system processes */
- if (p->p_flag & PK_SYSTEM) {
- mutex_exit(&p->p_smutex);
- return EPERM;
- }
- } else {
- /* Use the calling process */
- p = l->l_proc;
- mutex_enter(&p->p_smutex);
- }
-
/* Find the LWP(s) */
lcnt = 0;
lid = SCARG(uap, lid);
@@ -212,12 +215,6 @@ sys__sched_getparam(struct lwp *l, const
lwpid_t lid;
int error;
- if (kauth_authorize_process(l->l_cred, KAUTH_PROCESS_SCHEDULER,
- KAUTH_ARG(KAUTH_REQ_PROCESS_SCHEDULER_GETPARAM), NULL, NULL, NULL))
- return EPERM;
-
- sp = kmem_zalloc(sizeof(struct sched_param), KM_SLEEP);
-
/* If not specified, use the first LWP */
lid = SCARG(uap, lid) == 0 ? 1 : SCARG(uap, lid);
@@ -234,9 +231,16 @@ sys__sched_getparam(struct lwp *l, const
mutex_exit(&p->p_smutex);
}
if (t == NULL) {
- kmem_free(sp, sizeof(struct sched_param));
return ESRCH;
}
+
+ if (kauth_authorize_process(l->l_cred, KAUTH_PROCESS_SCHEDULER,
+ t->l_proc, KAUTH_ARG(KAUTH_REQ_PROCESS_SCHEDULER_GETPARAM), NULL,
+ NULL))
+ return EPERM;
+
+ sp = kmem_zalloc(sizeof(struct sched_param), KM_SLEEP);
+
sp->sched_priority = t->l_priority;
sp->sched_class = t->l_class;
lwp_unlock(t);
@@ -268,7 +272,7 @@ sys__sched_setaffinity(struct lwp *l,
syscallarg(size_t) size;
syscallarg(void *) cpuset;
} */
- cpuset_t *cpuset;
+ cpuset_t *cpuset = NULL;
struct cpu_info *ci = NULL;
struct proc *p;
struct lwp *t;
@@ -277,32 +281,9 @@ sys__sched_setaffinity(struct lwp *l,
u_int lcnt;
int error;
- /* Available only for super-user */
- if (kauth_authorize_process(l->l_cred, KAUTH_PROCESS_SCHEDULER,
- l->l_proc, KAUTH_ARG(KAUTH_REQ_PROCESS_SCHEDULER_SETAFFINITY), NULL,
- NULL))
- return EPERM;
-
if (SCARG(uap, size) <= 0)
return EINVAL;
- /* Allocate the CPU set, and get it from userspace */
- cpuset = kmem_zalloc(sizeof(cpuset_t), KM_SLEEP);
- error = copyin(SCARG(uap, cpuset), cpuset,
- min(SCARG(uap, size), sizeof(cpuset_t)));
- if (error)
- goto error;
-
- /* Look for a CPU in the set */
- for (CPU_INFO_FOREACH(cii, ci))
- if (CPU_ISSET(cpu_index(ci), cpuset))
- break;
- if (ci == NULL) {
- /* Empty set */
- kmem_free(cpuset, sizeof(cpuset_t));
- cpuset = NULL;
- }
-
if (SCARG(uap, pid) != 0) {
/* Find the process */
p = p_find(SCARG(uap, pid), PFIND_UNLOCK_FAIL);
@@ -325,6 +306,35 @@ sys__sched_setaffinity(struct lwp *l,
goto error;
}
+ /* Allocate the CPU set, and get it from userspace */
+ cpuset = kmem_zalloc(sizeof(cpuset_t), KM_SLEEP);
+ error = copyin(SCARG(uap, cpuset), cpuset,
+ min(SCARG(uap, size), sizeof(cpuset_t)));
+ if (error) {
+ mutex_exit(&p->p_smutex);
+ goto error;
+ }
+
+ /* Available only for super-user */
+ if (kauth_authorize_process(l->l_cred, KAUTH_PROCESS_SCHEDULER,
+ l->l_proc, KAUTH_ARG(KAUTH_REQ_PROCESS_SCHEDULER_SETAFFINITY), NULL,
+ NULL)) {
+ kmem_free(cpuset, sizeof(cpuset_t));
+ mutex_exit(&p->p_smutex);
+ error = EPERM;
+ goto error;
+ }
+
+ /* Look for a CPU in the set */
+ for (CPU_INFO_FOREACH(cii, ci))
+ if (CPU_ISSET(cpu_index(ci), cpuset))
+ break;
+ if (ci == NULL) {
+ /* Empty set */
+ kmem_free(cpuset, sizeof(cpuset_t));
+ cpuset = NULL;
+ }
+
/* Find the LWP(s) */
lcnt = 0;
lid = SCARG(uap, lid);
@@ -375,13 +385,6 @@ sys__sched_getaffinity(struct lwp *l,
if (SCARG(uap, size) <= 0)
return EINVAL;
- if (kauth_authorize_process(l->l_cred, KAUTH_PROCESS_SCHEDULER,
- l->l_proc, KAUTH_ARG(KAUTH_REQ_PROCESS_SCHEDULER_GETAFFINITY), NULL,
- NULL))
- return EPERM;
-
- cpuset = kmem_zalloc(sizeof(cpuset_t), KM_SLEEP);
-
/* If not specified, use the first LWP */
lid = SCARG(uap, lid) == 0 ? 1 : SCARG(uap, lid);
@@ -398,9 +401,16 @@ sys__sched_getaffinity(struct lwp *l,
mutex_exit(&p->p_smutex);
}
if (t == NULL) {
- kmem_free(cpuset, sizeof(cpuset_t));
return ESRCH;
}
+
+ if (kauth_authorize_process(l->l_cred, KAUTH_PROCESS_SCHEDULER,
+ t->l_proc, KAUTH_ARG(KAUTH_REQ_PROCESS_SCHEDULER_GETAFFINITY), NULL,
+ NULL))
+ return EPERM;
+
+ cpuset = kmem_zalloc(sizeof(cpuset_t), KM_SLEEP);
+
if (t->l_flag & LW_AFFINITY)
memcpy(cpuset, &t->l_affinity, sizeof(cpuset_t));
lwp_unlock(t);
--------------030603010809070103040103--