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:0.416 / U:1.072 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site