Messages in this thread | | | Date | Mon, 12 Dec 2011 16:40:34 -0800 | From | Arun Sharma <> | 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
| |