Re: [test patch] seccomp: add code to disable TSC when enabling seccomp

[andrea@cpushare.com]

On Thu, Jul 13, 2006 at 04:11:12PM -0400, Chuck Ebbert wrote:
> Is the below patch acceptable in generic code, or should some arch
> helper function hide it?  It lets i386 / x86_64 add TIF_NOTSC
> independently.  
>
> Also, what prevents this flag from being set on a running process?
> If that happens the CPU state and flag could get out of sync and
> this could cause problems because of the way the current code tests
> the flag.

Yes, there could be a tiny race where if the controller and seccomp
tasks run on two different CPUs: the seccomp task may write to the
pipe, and then read, but the read may not actually stop anywhere,
because the second CPU may have enabled seccomp and answered faster
than the first cpu. So there's tiny window for the TSC not to be
disabled synchronously at the start of the seccomp computations (and
if there are multiple seccomp tasks running the new ones could let the
old ones run a timeslice with the tsc enabled). The inverse isn't
possible because the SECCOMP/TSC bits cannot be cleared anywhere. In
short the only problem is that it's not a guarantee that the tsc is
always permanently disabled with seccomp in SMP.

To fix the tiny window if it's the current task writing to self, we
should also update the cr4 before returning from base.c. If it was a
different task it's more complicated (we would need to send a forced
sigstop, and wait the task->state to change, but then we go into the
ptrace parallelism I truly don't want to deal with in any way in
seccomp context). The whole point of seccomp is to be simple. So my
suggestion is either we ignore the tiny window, or we do it only from
the current task. If I've to deal with any sigstop then I could use
ptrace or utrace in the first place ;).

I generally preferred to be the controller task that fires up seccomp
(the controller tasks did a number of checks that everything was going
ok before firing it up), but if we forbid other tasks to fire up
seccomp, then perhaps there's no more reason to leave it a /proc
interface. I guess we could move it to a prctl which would probably
waste a few less bytes, so it gets even more friendly.

Thanks Chunk!