lkml.org 
[lkml]   [2013]   [Nov]   [12]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
From
SubjectRe: [PATCH 02/14] sched: add extended scheduling interface.
On Thu,  7 Nov 2013 14:43:36 +0100
Juri Lelli <juri.lelli@gmail.com> wrote:


> +static int
> +do_sched_setscheduler2(pid_t pid, int policy,
> + struct sched_param2 __user *param2)
> +{
> + struct sched_param2 lparam2;
> + struct task_struct *p;
> + int retval;
> +
> + if (!param2 || pid < 0)
> + return -EINVAL;
> +
> + memset(&lparam2, 0, sizeof(struct sched_param2));
> + if (copy_from_user(&lparam2, param2, sizeof(struct sched_param2)))
> + return -EFAULT;

Why the memset() before the copy_from_user()? We are copying
sizeof(sched_param2) anyway, and should overwrite anything that was on
the stack. I'm not aware of any possible leak from copying from
userspace. I could understand it if we were copying to userspace.

do_sched_setscheduler() doesn't do that either.

> +
> + rcu_read_lock();
> + retval = -ESRCH;
> + p = find_process_by_pid(pid);
> + if (p != NULL)
> + retval = sched_setscheduler2(p, policy, &lparam2);
> + rcu_read_unlock();
> +
> + return retval;
> +}
> +
> /**
> * sys_sched_setscheduler - set/change the scheduler policy and RT priority
> * @pid: the pid in question.
> @@ -3514,6 +3553,21 @@ SYSCALL_DEFINE3(sched_setscheduler, pid_t, pid, int, policy,
> }
>
> /**
> + * sys_sched_setscheduler2 - same as above, but with extended sched_param
> + * @pid: the pid in question.
> + * @policy: new policy (could use extended sched_param).
> + * @param: structure containg the extended parameters.
> + */
> +SYSCALL_DEFINE3(sched_setscheduler2, pid_t, pid, int, policy,
> + struct sched_param2 __user *, param2)
> +{
> + if (policy < 0)
> + return -EINVAL;
> +
> + return do_sched_setscheduler2(pid, policy, param2);
> +}
> +
> +/**
> * sys_sched_setparam - set/change the RT priority of a thread
> * @pid: the pid in question.
> * @param: structure containing the new RT priority.
> @@ -3526,6 +3580,17 @@ SYSCALL_DEFINE2(sched_setparam, pid_t, pid, struct sched_param __user *, param)
> }
>
> /**
> + * sys_sched_setparam2 - same as above, but with extended sched_param
> + * @pid: the pid in question.
> + * @param2: structure containing the extended parameters.
> + */
> +SYSCALL_DEFINE2(sched_setparam2, pid_t, pid,
> + struct sched_param2 __user *, param2)
> +{
> + return do_sched_setscheduler2(pid, -1, param2);
> +}
> +
> +/**
> * sys_sched_getscheduler - get the policy (scheduling class) of a thread
> * @pid: the pid in question.
> *
> @@ -3595,6 +3660,45 @@ out_unlock:
> return retval;
> }
>
> +/**
> + * sys_sched_getparam2 - same as above, but with extended sched_param
> + * @pid: the pid in question.
> + * @param2: structure containing the extended parameters.
> + */
> +SYSCALL_DEFINE2(sched_getparam2, pid_t, pid,
> + struct sched_param2 __user *, param2)
> +{
> + struct sched_param2 lp;
> + struct task_struct *p;
> + int retval;
> +
> + if (!param2 || pid < 0)
> + return -EINVAL;
> +
> + rcu_read_lock();
> + p = find_process_by_pid(pid);
> + retval = -ESRCH;
> + if (!p)
> + goto out_unlock;
> +
> + retval = security_task_getscheduler(p);
> + if (retval)
> + goto out_unlock;
> +
> + lp.sched_priority = p->rt_priority;
> + rcu_read_unlock();
> +

OK, now we are missing the memset(). This does leak info, as lp never
was set to zero, it just contains anything on the stack, and the only
value you updated was sched_priority. We just copied to user memory
from the kernel stack.

-- Steve



> + retval = copy_to_user(param2, &lp,
> + sizeof(struct sched_param2)) ? -EFAULT : 0;
> +
> + return retval;
> +
> +out_unlock:
> + rcu_read_unlock();
> + return retval;
> +
> +}
> +
> long sched_setaffinity(pid_t pid, const struct cpumask *in_mask)
> {
> cpumask_var_t cpus_allowed, new_mask;



\
 
 \ /
  Last update: 2013-11-12 19:21    [W:0.411 / U:0.448 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site