From 73c40187fdcad79a960c9ae1458724c37e64d943 Mon Sep 17 00:00:00 2001 From: Jeff Roberson Date: Wed, 5 Mar 2008 01:49:20 +0000 Subject: [PATCH] - Verify that when a user supplies a mask that is bigger than the kernel mask none of the upper bits are set. - Be more careful about enforcing the boundaries of masks and child sets. - Introduce a few more CPU_* macros for implementing these tests. - Change the cpusetsize argument to be bytes rather than bits to match other apis. Sponsored by: Nokia --- sys/kern/kern_cpuset.c | 106 ++++++++++++++++++++++++++++++---------- sys/sys/cpuset.h | 22 +++++++++ usr.bin/cpuset/cpuset.c | 15 +++--- 3 files changed, 108 insertions(+), 35 deletions(-) diff --git a/sys/kern/kern_cpuset.c b/sys/kern/kern_cpuset.c index b262dc12593b..73fc1f48bef4 100644 --- a/sys/kern/kern_cpuset.c +++ b/sys/kern/kern_cpuset.c @@ -188,26 +188,23 @@ static int _cpuset_create(struct cpuset *set, struct cpuset *parent, cpuset_t *mask, cpusetid_t id) { - int error; - error = 0; + if (!CPU_OVERLAP(&parent->cs_mask, mask)) + return (EDEADLK); CPU_COPY(mask, &set->cs_mask); LIST_INIT(&set->cs_children); refcount_init(&set->cs_ref, 1); set->cs_flags = 0; mtx_lock_spin(&cpuset_lock); CPU_AND(mask, &parent->cs_mask); - if (!CPU_EMPTY(mask)) { - set->cs_id = id; - set->cs_parent = cpuset_ref(parent); - LIST_INSERT_HEAD(&parent->cs_children, set, cs_siblings); - if (set->cs_id != CPUSET_INVALID) - LIST_INSERT_HEAD(&cpuset_ids, set, cs_link); - } else - error = EDEADLK; + set->cs_id = id; + set->cs_parent = cpuset_ref(parent); + LIST_INSERT_HEAD(&parent->cs_children, set, cs_siblings); + if (set->cs_id != CPUSET_INVALID) + LIST_INSERT_HEAD(&cpuset_ids, set, cs_link); mtx_unlock_spin(&cpuset_lock); - return (error); + return (0); } /* @@ -250,11 +247,11 @@ cpuset_testupdate(struct cpuset *set, cpuset_t *mask) mtx_assert(&cpuset_lock, MA_OWNED); if (set->cs_flags & CPU_SET_RDONLY) return (EPERM); - error = 0; + if (!CPU_OVERLAP(&set->cs_mask, mask)) + return (EDEADLK); CPU_COPY(&set->cs_mask, &newmask); CPU_AND(&newmask, mask); - if (CPU_EMPTY(&newmask)) - return (EDEADLK); + error = 0; LIST_FOREACH(nset, &set->cs_children, cs_siblings) if ((error = cpuset_testupdate(nset, &newmask)) != 0) break; @@ -285,11 +282,19 @@ cpuset_update(struct cpuset *set, cpuset_t *mask) static int cpuset_modify(struct cpuset *set, cpuset_t *mask) { + struct cpuset *root; int error; error = suser(curthread); if (error) return (error); + /* + * Verify that we have access to this set of + * cpus. + */ + root = set->cs_parent; + if (root && !CPU_SUBSET(&root->cs_mask, mask)) + return (EINVAL); mtx_lock_spin(&cpuset_lock); error = cpuset_testupdate(set, mask); if (error) @@ -310,12 +315,10 @@ static struct cpuset * cpuset_root(struct cpuset *set) { - mtx_lock_spin(&cpuset_lock); for (; set->cs_parent != NULL; set = set->cs_parent) if (set->cs_flags & CPU_SET_ROOT) break; cpuset_ref(set); - mtx_unlock_spin(&cpuset_lock); return (set); } @@ -329,11 +332,9 @@ static struct cpuset * cpuset_base(struct cpuset *set) { - mtx_lock_spin(&cpuset_lock); if (set->cs_id == CPUSET_INVALID) set = set->cs_parent; cpuset_ref(set); - mtx_unlock_spin(&cpuset_lock); return (set); } @@ -435,6 +436,8 @@ cpuset_shadow(struct cpuset *set, struct cpuset *fset, cpuset_t *mask) parent = set->cs_parent; else parent = set; + if (!CPU_SUBSET(&parent->cs_mask, mask)) + return (EINVAL); return (_cpuset_create(fset, parent, mask, CPUSET_INVALID)); } @@ -456,6 +459,7 @@ cpuset_setproc(pid_t pid, struct cpuset *set, cpuset_t *mask) { struct setlist freelist; struct setlist droplist; + struct cpuset *tdset; struct cpuset *nset; struct thread *td; struct proc *p; @@ -491,13 +495,41 @@ cpuset_setproc(pid_t pid, struct cpuset *set, cpuset_t *mask) PROC_SLOCK_ASSERT(p, MA_OWNED); /* * Now that the appropriate locks are held and we have enough cpusets, - * replace each thread's cpuset while using deferred release. We - * must do this because the PROC_SLOCK has to be held while traversing - * the thread list and this limits the type of operations allowed. + * make sure the operation will succeed before applying changes. The + * proc lock prevents td_cpuset from changing between calls. */ error = 0; FOREACH_THREAD_IN_PROC(p, td) { - struct cpuset *tdset; + thread_lock(td); + tdset = td->td_cpuset; + /* + * Verify that a new mask doesn't specify cpus outside of + * the set the thread is a member of. + */ + if (mask) { + if (tdset->cs_id == CPUSET_INVALID) + tdset = tdset->cs_parent; + if (!CPU_SUBSET(&tdset->cs_mask, mask)) + error = EINVAL; + /* + * Verify that a new set won't leave an existing thread + * mask without a cpu to run on. It can, however, restrict + * the set. + */ + } else if (tdset->cs_id == CPUSET_INVALID) { + if (!CPU_OVERLAP(&set->cs_mask, &tdset->cs_mask)) + error = EINVAL; + } + thread_unlock(td); + if (error) + goto unlock_out; + } + /* + * Replace each thread's cpuset while using deferred release. We + * must do this because the PROC_SLOCK has to be held while traversing + * the thread list and this limits the type of operations allowed. + */ + FOREACH_THREAD_IN_PROC(p, td) { thread_lock(td); /* * If we presently have an anonymous set or are applying a @@ -528,6 +560,7 @@ cpuset_setproc(pid_t pid, struct cpuset *set, cpuset_t *mask) sched_affinity(td); thread_unlock(td); } +unlock_out: PROC_SUNLOCK(p); PROC_UNLOCK(p); out: @@ -763,9 +796,10 @@ cpuset_getaffinity(struct thread *td, struct cpuset_getaffinity_args *uap) int error; int size; - if (uap->cpusetsize < CPU_SETSIZE || uap->cpusetsize > CPU_MAXSIZE) + if (uap->cpusetsize < sizeof(cpuset_t) || + uap->cpusetsize * NBBY > CPU_MAXSIZE) return (ERANGE); - size = uap->cpusetsize / NBBY; + size = uap->cpusetsize; mask = malloc(size, M_TEMP, M_WAITOK | M_ZERO); error = cpuset_which(uap->which, uap->id, &p, &ttd, &set); if (error) @@ -846,12 +880,30 @@ cpuset_setaffinity(struct thread *td, struct cpuset_setaffinity_args *uap) cpuset_t *mask; int error; - if (uap->cpusetsize < CPU_SETSIZE || uap->cpusetsize > CPU_MAXSIZE) + if (uap->cpusetsize < sizeof(cpuset_t) || + uap->cpusetsize * NBBY > CPU_MAXSIZE) return (ERANGE); - mask = malloc(uap->cpusetsize / NBBY, M_TEMP, M_WAITOK | M_ZERO); - error = copyin(uap->mask, mask, uap->cpusetsize / NBBY); + mask = malloc(uap->cpusetsize, M_TEMP, M_WAITOK | M_ZERO); + error = copyin(uap->mask, mask, uap->cpusetsize); if (error) goto out; + /* + * Verify that no high bits are set. + */ + if (uap->cpusetsize > sizeof(cpuset_t)) { + char *end; + char *cp; + + end = cp = (char *)&mask->__bits; + end += uap->cpusetsize; + cp += sizeof(cpuset_t); + while (cp != end) + if (*cp++ != 0) { + error = EINVAL; + goto out; + } + + } switch (uap->level) { case CPU_LEVEL_ROOT: case CPU_LEVEL_CPUSET: diff --git a/sys/sys/cpuset.h b/sys/sys/cpuset.h index 301f96c1680b..0788d46e5e61 100644 --- a/sys/sys/cpuset.h +++ b/sys/sys/cpuset.h @@ -57,6 +57,7 @@ typedef struct _cpuset { (p)->__bits[__i] = 0; \ } while (0) +/* Is p empty. */ #define CPU_EMPTY(p) __extension__ ({ \ __size_t __i; \ for (__i = 0; __i < _NCPUWORDS; __i++) \ @@ -65,6 +66,27 @@ typedef struct _cpuset { __i == _NCPUWORDS; \ }) +/* Is c a subset of p. */ +#define CPU_SUBSET(p, c) __extension__ ({ \ + __size_t __i; \ + for (__i = 0; __i < _NCPUWORDS; __i++) \ + if (((c)->__bits[__i] & \ + (p)->__bits[__i]) != \ + (c)->__bits[__i]) \ + break; \ + __i == _NCPUWORDS; \ +}) + +/* Are there any common bits between b & c? */ +#define CPU_OVERLAP(p, c) __extension__ ({ \ + __size_t __i; \ + for (__i = 0; __i < _NCPUWORDS; __i++) \ + if (((c)->__bits[__i] & \ + (p)->__bits[__i]) != 0) \ + break; \ + __i != _NCPUWORDS; \ +}) + #define CPU_OR(d, s) do { \ __size_t __i; \ for (__i = 0; __i < _NCPUWORDS; __i++) \ diff --git a/usr.bin/cpuset/cpuset.c b/usr.bin/cpuset/cpuset.c index 699e096ebd0a..23c4b791c5a6 100644 --- a/usr.bin/cpuset/cpuset.c +++ b/usr.bin/cpuset/cpuset.c @@ -128,7 +128,7 @@ parselist(char *list, cpuset_t *mask) } return; parserr: - errx(EXIT_FAILURE, "Malformed cpu list %s", list); + errx(EXIT_FAILURE, "Malformed cpu-list %s", list); } static void @@ -157,8 +157,7 @@ printaffinity(void) { cpuset_t mask; - if (cpuset_getaffinity(level, which, id, CPU_SETSIZE, - &mask) != 0) + if (cpuset_getaffinity(level, which, id, sizeof(mask), &mask) != 0) err(EXIT_FAILURE, "getaffinity"); printf("%s %jd%s mask: ", whichnames[which], (intmax_t)id, levelnames[level]); @@ -274,7 +273,7 @@ main(int argc, char *argv[]) } if (lflag) { if (cpuset_setaffinity(level, which, -1, - CPU_SETSIZE, &mask) != 0) + sizeof(mask), &mask) != 0) err(EXIT_FAILURE, "setaffinity"); } errno = 0; @@ -304,7 +303,7 @@ main(int argc, char *argv[]) id = pid; } if (lflag) { - if (cpuset_setaffinity(level, which, id, CPU_SETSIZE, + if (cpuset_setaffinity(level, which, id, sizeof(mask), &mask) != 0) err(EXIT_FAILURE, "setaffinity"); } @@ -317,11 +316,11 @@ usage(void) { fprintf(stderr, - "usage: cpuset [-l cpu list] [-i | -s setid] cmd ...\n"); + "usage: cpuset [-l cpu-list] [-i | -s setid] cmd ...\n"); fprintf(stderr, - " cpuset [-l cpu list] [-s setid] -p pid\n"); + " cpuset [-l cpu-list] [-s setid] -p pid\n"); fprintf(stderr, - " cpuset [-cr] [-l cpu list] [-p pid | -t tid | -s setid]\n"); + " cpuset [-cr] [-l cpu-list] [-p pid | -t tid | -s setid]\n"); fprintf(stderr, " cpuset [-cgir] [-p pid | -t tid | -s setid]\n"); exit(1);