linux-kernel - Re: [PATCH 2/2] Add a thread cpu time implementation to vDSO

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: <4EE69F02.2090805@fb.com>
Date:	Mon, 12 Dec 2011 16:40:34 -0800
From:	Arun Sharma <asharma@...com>
To:	Andrew Lutomirski <luto@....edu>
CC:	<linux-kernel@...r.kernel.org>, Kumar Sundararajan <kumar@...com>,
	Peter Zijlstra <peterz@...radead.org>,
	Ingo Molnar <mingo@...e.hu>,
	Thomas Gleixner <tglx@...utronix.de>,
	john stultz <johnstul@...ibm.com>
Subject: Re: [PATCH 2/2] Add a thread cpu time implementation to vDSO

On 12/12/11 3:01 PM, Andrew Lutomirski wrote:

>> The timing based attacks depend on the granularity of timestamps. I feel
>> what's available here is too coarse grained to be useful. Happy to
>> disable the code at compile time for those cases. Are there
>> CONFIG_HIGH_SECURITY type of options I could use for this purpose?
>
> It allows anyone to detect with very high precision when context
> switches happen on another CPU.  This sounds a little dangerous.  I
> don't know if a config option is the right choice.

Minor nit: attacker gets to see sum_exec_runtime - sched_clock(), not 
sched_clock() directly. It might still be equally damaging (assuming the 
attacker can work out sum_exec_runtime by observing the VVAR page).

Either way, I think it's important to get to the bottom of this. 
Conversely, we might want to consider enabling things like this only 
under CONFIG_I_TRUST_STUFF_RUNNING_ON_MY_MACHINE.

>> Yes - this should be a separate patch. gcc-4.4 likes to get rid of the
>> instruction in __do_thread_cpu_time without the asm volatile (in spite of
>> the memory clobber).
>>
>
> gcc 4.4 caught a genuine bug in your code.  You ignored the return
> value (which is an output constraint), so you weren't using any
> outputs and gcc omitted the apparently useless code.  The right fix is
> to check the return value.  (And to consider adding the missing
> volatile.)

Yes - there are two bugs here.

>
>>
>>
>>>> +       if (vp->tsc_unstable) {
>>>> +               struct timespec ts;
>>>> +               vdso_fallback_gettime(CLOCK_THREAD_CPUTIME_ID,&ts);
>>>> +               return timespec_to_ns(&ts);
>>>> +       }
>>>
>>>
>>> Yuck -- another flag indicating whether we're using the tsc.
>>
>>
>> I renamed it to thread_cputime_disabled to deal with NR_CPUS>  64.
>>
>
> Still yuck.  IMO this should always work.
>

Yes - but boot time VVAR page allocation is going to take me some time 
to implement. In the meanwhile I was merely trying to ensure that 
compilation doesn't break for unsupported configs and the app doesn't 
get SIGILL when running on old 64 bit CPUs.

[..]
> Sorry, I was unclear.  gtod_data contains vclock_mode, which will tell
> you whether the tsc is usable.  I have a buggy computer (I'm typing
> this email on it) that has a TSC that is only useful per-process.  I
> don't think it's worth supporting this particular case, since it's a
> bios bug and needs fixing by the vendor.  It's hopefully rare.

Ah yes. Will make this change as well.

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