linux-kernel - Re: [RFC 12/12][PATCH] SCHED_DEADLINE: modified sched_*

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:	Wed, 13 Jan 2010 11:27:32 +0100
From:	Raistlin <raistlin@...ux.it>
To:	Peter Zijlstra <peterz@...radead.org>
Cc:	linux-kernel <linux-kernel@...r.kernel.org>,
	michael trimarchi <michael@...dence.eu.com>,
	Fabio Checconi <fabio@...dalf.sssup.it>,
	Ingo Molnar <mingo@...e.hu>,
	Thomas Gleixner <tglx@...utronix.de>,
	Dhaval Giani <dhaval.giani@...il.com>,
	Johan Eker <johan.eker@...csson.com>,
	"p.faure" <p.faure@...tech.ch>,
	Chris Friesen <cfriesen@...tel.com>,
	Steven Rostedt <rostedt@...dmis.org>,
	Henrik Austad <henrik@...tad.us>,
	Frederic Weisbecker <fweisbec@...il.com>,
	Darren Hart <darren@...art.com>,
	Sven-Thorsten Dietrich <sven@...bigcorporation.com>,
	Bjoern Brandenburg <bbb@...unc.edu>,
	Tommaso Cucinotta <tommaso.cucinotta@...up.it>,
	"giuseppe.lipari" <giuseppe.lipari@...up.it>,
	Juri Lelli <juri.lelli@...il.com>
Subject: Re: [RFC 12/12][PATCH] SCHED_DEADLINE: modified sched_*_ex API

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@...ga.net /
dario.faggioli@...ber.org

Download attachment "signature.asc" of type "application/pgp-signature" (198 bytes)