linux-kernel - Re: Another SCHED_DEADLINE bug (with bisection and possible fix)

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:	Tue, 13 Jan 2015 15:04:36 +0100
From:	Peter Zijlstra <peterz@...radead.org>
To:	Kirill Tkhai <tkhai@...dex.ru>
Cc:	Juri Lelli <juri.lelli@....com>, Luca Abeni <luca.abeni@...tn.it>,
	Ingo Molnar <mingo@...hat.com>,
	"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
	"juri.lelli@...il.com" <juri.lelli@...il.com>
Subject: Re: Another SCHED_DEADLINE bug (with bisection and possible fix)

On Tue, Jan 12, 2015 at 12:26:40PM +0300, Kirill Tkhai wrote:
> > Well, I'm inclined to agree to Luca's viewpoint. We should not change
> > parameters of a throttled task or we may affect other tasks.
> 
> Could you explain your viewpoint more? How does this affects other tasks?

I agree with Juri and Luca here. Luca gave an example that DoS's
SCHED_DEADLINE.

In Luca's example we'd constantly call sched_setattr() on a task, which
results in it never throttling and that will affect other tasks, because
if we're running, they cannot be.

> As I understand, in __setparam_dl() we are sure that there is enough
> dl_bw. In __sched_setscheduler() we call it after dl_overflow() check.

Yes, _but_, as per the above. BW includes the sleep time, if we fail to
sleep its all pointless.

We got the runtime (before the throttle) under the promise that we'd go
sleep. When we change our parameters while being throttled we should
adjust the throttle time accordingly. If say we changed from 30% to 35%
we are allowed to sleep less. Now sleeping more than strictly required
is only detrimental to ourselves, so that is not (too) critical.

However, the other way around, if we change from 35% to 30% we should
now effectively sleep longer (for the same runtime we already consumed),
and we must do this, because admission control will instantly assume the
5% change in bandwidth to be available. If we sleep short, this BW is
not in fact available and we break our guarantees.

> >>>  Anyway, I am fine with every patch that fixes the bug :)
> >>  Deadline class requires root privileges. So, I do not see a problem
> >>  here. Please, see __sched_setscheduler().

Its not a priv issue, not doing this makes it impossible to use
SCHED_DEADLINE in the intended way, even for root.

Say we have a userspace task that evaluates and changes runtime
parameters for other tasks (basically what Luca is doing IIRC), and the
changes keep resetting the sleep time, the whole guarantee system comes
down, rendering the deadline system useless.

> >>  If in the future we allow non-privileged users to increase deadline,
> >>  we will reflect that in __setparam_dl() too.
> >
> > I'd say it is better to implement the right behavior even for root, so
> > that we will find it right when we'll grant access to non root users
> > too. Also, even if root can do everything, we always try not to break
> > guarantees that come with admission control (root or non root that is).

Just so.
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/