lkml.org 
[lkml]   [2013]   [Nov]   [13]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    Patch in this message
    /
    Date
    From
    SubjectRe: [PATCH 02/14] sched: add extended scheduling interface.
    On 11/12/2013 06:32 PM, Steven Rostedt wrote:
    > 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.

    Right! memset() moved:

    @@ -3779,7 +3779,6 @@ do_sched_setscheduler2(pid_t pid, int policy,
    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;

    @@ -3937,6 +3936,8 @@ SYSCALL_DEFINE2(sched_getparam2, pid_t, pid,
    if (!param2 || pid < 0)
    return -EINVAL;

    + memset(&lp, 0, sizeof(struct sched_param2));
    +
    rcu_read_lock();
    p = find_process_by_pid(pid);
    retval = -ESRCH;
    Thanks,

    - Juri

    >
    >> + 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-13 10:41    [W:4.757 / U:0.280 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site