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]

Date:	Mon, 29 Oct 2012 14:18:42 +0100
From:	Vincent Guittot <vincent.guittot@...aro.org>
To:	Santosh Shilimkar <santosh.shilimkar@...com>
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

On 24 October 2012 17:21, Santosh Shilimkar <santosh.shilimkar@...com> wrote:
> $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.

My goal is to have a lock free mechanism because I don't want to lock
another CPU while reading its statistic

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