linux-kernel - Re: [PATCH] sched/deadline: overrun could happen in start_hrtick

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]

Message-ID: <53BBD4A7.5050701@huawei.com>
Date:	Tue, 8 Jul 2014 19:23:19 +0800
From:	"xiaofeng.yan" <xiaofeng.yan@...wei.com>
To:	Peter Zijlstra <peterz@...radead.org>
CC:	<mingo@...hat.com>, <juri.lelli@...il.com>,
	<linux-kernel@...r.kernel.org>, <xiaofeng.yan2012@...il.com>
Subject: Re: [PATCH] sched/deadline: overrun could happen in start_hrtick_dl

On 2014/7/8 17:33, Peter Zijlstra wrote:
> On Tue, Jul 08, 2014 at 08:53:27AM +0000, xiaofeng.yan wrote:
>>   static void start_hrtick_dl(struct rq *rq, struct task_struct *p)
>>   {
>> -	s64 delta = p->dl.dl_runtime - p->dl.runtime;
>> -
>> -	if (delta > 10000)
>> -		hrtick_start(rq, p->dl.runtime);
>> +	delta = max_t(s64, 10000LL, delta);
>> +	hrtick_start(rq, delta);
>>   }
> no, no, no. I said to unify the test.

I understand your idea after reading the next patch. This is good solution.
I will test it with your patch.

> ---
>   kernel/sched/core.c     | 9 ++++++++-
>   kernel/sched/deadline.c | 3 +--
>   kernel/sched/fair.c     | 7 -------
>   3 files changed, 9 insertions(+), 10 deletions(-)
>
> diff --git a/kernel/sched/core.c b/kernel/sched/core.c
> index e1a2f31bb0cb..c7b8a6fbac66 100644
> --- a/kernel/sched/core.c
> +++ b/kernel/sched/core.c
> @@ -444,7 +444,14 @@ static void __hrtick_start(void *arg)
>   void hrtick_start(struct rq *rq, u64 delay)
>   {
>   	struct hrtimer *timer = &rq->hrtick_timer;
> -	ktime_t time = ktime_add_ns(timer->base->get_time(), delay);
> +	ktime_t time;
> +
> +	/*
> +	 * Don't schedule slices shorter than 10000ns, that just
> +	 * doesn't make sense and can cause timer DoS.
> +	 */
> +	delta = max_t(s64, delta, 10000LL);

transfer the argument delta to delay

> +	time = ktime_add_ns(timer->base->get_time(), delay);
>   
>   	hrtimer_set_expires(timer, time);
>   
> diff --git a/kernel/sched/deadline.c b/kernel/sched/deadline.c
> index fc4f98b1258f..e1e24eea8061 100644
> --- a/kernel/sched/deadline.c
> +++ b/kernel/sched/deadline.c
> @@ -999,8 +999,7 @@ static void start_hrtick_dl(struct rq *rq, struct task_struct *p)
>   {
>   	s64 delta = p->dl.dl_runtime - p->dl.runtime;
>   
> -	if (delta > 10000)
> -		hrtick_start(rq, p->dl.runtime);
> +	hrtick_start(rq, p->dl.runtime);
>   }
>   #endif
>   
> diff --git a/kernel/sched/fair.c b/kernel/sched/fair.c
> index 923fe32db6b3..713c58d2a7b0 100644
> --- a/kernel/sched/fair.c
> +++ b/kernel/sched/fair.c
> @@ -3901,13 +3901,6 @@ static void hrtick_start_fair(struct rq *rq, struct task_struct *p)
>   			return;
>   		}
>   
> -		/*
> -		 * Don't schedule slices shorter than 10000ns, that just
> -		 * doesn't make sense. Rely on vruntime for fairness.
> -		 */
> -		if (rq->curr != p)
> -			delta = max_t(s64, 10000LL, delta);
> -
>   		hrtick_start(rq, delta);
>   	}
>   }


--
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/