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

[Arun Sharma]

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@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/