lkml.org 
[lkml]   [2011]   [Dec]   [12]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
From
SubjectRe: [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


\
 
 \ /
  Last update: 2011-12-13 01:43    [W:0.053 / U:0.140 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site