Messages in this thread | | | Subject | Re: [RFC 12/12][PATCH] SCHED_DEADLINE: modified sched_*_ex API | From | Raistlin <> | Date | Wed, 13 Jan 2010 11:27:32 +0100 |
| |
On Mon, 2009-12-28 at 16:09 +0100, Peter Zijlstra wrote: > On Fri, 2009-10-16 at 17:48 +0200, Raistlin wrote: > > @@ -6807,9 +6811,10 @@ out_unlock: > > /** > > * sys_sched_getparam - get the DEADLINE task parameters of a thread > > * @pid: the pid in question. > > + * @len: size of data pointed by param_ex. > > * @param_ex: structure containing the new parameters (deadline, runtime, etc.). > > */ > > -SYSCALL_DEFINE2(sched_getparam_ex, pid_t, pid, > > +SYSCALL_DEFINE3(sched_getparam_ex, pid_t, pid, unsigned, len, > > struct sched_param_ex __user *, param_ex) > > { > > struct sched_param_ex lp; > > @@ -6818,6 +6823,8 @@ SYSCALL_DEFINE2(sched_getparam_ex, pid_t, pid, > > > > if (!param_ex || pid < 0) > > return -EINVAL; > > + if (len < sizeof(struct sched_param_ex)) > > + return -EINVAL; > > > > read_lock(&tasklist_lock); > > p = find_process_by_pid(pid); > > This allows len > sizeof(). > Yes...
> > @@ -6837,7 +6844,7 @@ SYSCALL_DEFINE2(sched_getparam_ex, pid_t, pid, > > /* > > * This one might sleep, we cannot do it with a spinlock held ... > > */ > > - retval = copy_to_user(param_ex, &lp, sizeof(*param_ex)) ? -EFAULT : 0; > > + retval = copy_to_user(param_ex, &lp, len) ? -EFAULT : 0; > > > > return retval; > > Which would copy more than lp, resulting in a stack leak, right? > ... And yes again! :-)
This has been done bearing in mind that the _kernel_side_ sched_param_ex --once stabilized-- will never lower its size. I.e., it should always grow and, if/when it does, it should retain the position of existing fields, for the sake of backward compatibility.
In that case, I think, the only possible case we have to face is the one where the "old" userspace program/library uses a version of sched_param_ex which is smaller than the one in the kernel, and what we want is the kernel to fill only the fields existing in the userspace code.
Does all this make sense?
If yes, I guess I just have to flip the inequality in the if() turning it into "if (len > sizeof())" (, then apologize for the glaring bug! :-P) and then I'm done, am I?
Thanks and regards, Dario
-- <<This happens because I choose it to happen!>> (Raistlin Majere) ---------------------------------------------------------------------- Dario Faggioli, ReTiS Lab, Scuola Superiore Sant'Anna, Pisa (Italy)
http://blog.linux.it/raistlin / raistlin@ekiga.net / dario.faggioli@jabber.org [unhandled content-type:application/pgp-signature] | |