linux-kernel - Re: [RFC 4/6] sched: secure access to other CPU statistics

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: <5088076D.6080208@ti.com>
Date:	Wed, 24 Oct 2012 20:51:17 +0530
From:	Santosh Shilimkar <santosh.shilimkar@...com>
To:	Vincent Guittot <vincent.guittot@...aro.org>
CC:	<linux-kernel@...r.kernel.org>,
	<linux-arm-kernel@...ts.infradead.org>,
	<linaro-dev@...ts.linaro.org>, <peterz@...radead.org>,
	<mingo@...hat.com>, <pjt@...gle.com>, <linux@....linux.org.uk>
Subject: Re: [RFC 4/6] sched: secure access to other CPU statistics

$subject is bit confusing here.

On Sunday 07 October 2012 01:13 PM, Vincent Guittot wrote:
> The atomic update of runnable_avg_sum and runnable_avg_period are ensured
> by their size and the toolchain. But we must ensure to not read an old value
> for one field and a newly updated value for the other field. As we don't
> want to lock other CPU while reading these fields, we read twice each fields
> and check that no change have occured in the middle.
>
> Signed-off-by: Vincent Guittot <vincent.guittot@...aro.org>
> ---
>   kernel/sched/fair.c |   19 +++++++++++++++++--
>   1 file changed, 17 insertions(+), 2 deletions(-)
>
> diff --git a/kernel/sched/fair.c b/kernel/sched/fair.c
> index 8c9d3ed..6df53b5 100644
> --- a/kernel/sched/fair.c
> +++ b/kernel/sched/fair.c
> @@ -3133,13 +3133,28 @@ static int select_idle_sibling(struct task_struct *p, int target)
>   static inline bool is_buddy_busy(int cpu)
>   {
>   	struct rq *rq = cpu_rq(cpu);
> +	volatile u32 *psum = &rq->avg.runnable_avg_sum;
> +	volatile u32 *pperiod = &rq->avg.runnable_avg_period;
> +	u32 sum, new_sum, period, new_period;
> +	int timeout = 10;
So it can be 2 times read or more as well.
> +
> +	while (timeout) {
> +		sum = *psum;
> +		period = *pperiod;
> +		new_sum = *psum;
> +		new_period = *pperiod;
> +
> +		if ((sum == new_sum) && (period == new_period))
> +			break;
> +
> +		timeout--;
> +	}
>
Seems like you did notice incorrect pair getting read
for rq runnable_avg_sum and runnable_avg_period. Seems
like the fix is to update them together under some lock
to avoid such issues.

Regards
Santosh

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