cheri_revoke() is too disruptive

[markjdb]

To scan threads' register files, we use thread_single(SINGLE_BOUNDARY), which has the side effect of interrupting system calls like nanosleep(). This breaks some applications, e.g., will-it-scale, which in some mode will have a bunch of threads executing some work loop while another thread prints a status message once a second. cheri_revoke() interrupts this sleep(1) and effectively makes the program useless.

thread_single() is simple but it's a heavy hammer. I propose the following instead: for each thread in the proc,

• if the thread is off-CPU, lock the thread and revoke its register file then and there,
• otherwise schedule an AST which causes each thread to revoke its own register file, then decrement a counter and wake up the cheri_revoke() caller once finished
• sleep waiting for ASTs to drain

[markjdb]

I suspect we might need threads to block in an AST handler while we revoke hoarders. But, maybe we can safely schedule ASTs, then revoke hoarders, then drain ASTs, to let some of this happen in parallel.

I also wonder if we should be revoking each thread's td_retval field as a part of this...?

[jrtc27]

All the threads still need to be blocked once their register files are revoked until the generation counter gets flipped though, right? Which maybe they can do as part of the AST... But what about kernel hoarders? How do we ensure that, once a thread's register file is blocked, its generation counter flipped and it gets scheduled, you can't then retrieve to-be-revoked capabilities from hoarders?

[markjdb]

All the threads still need to be blocked once their register files are revoked until the generation counter gets flipped though, right? Which maybe they can do as part of the AST... But what about kernel hoarders? How do we ensure that, once a thread's register file is blocked, its generation counter flipped and it gets scheduled, you can't then retrieve to-be-revoked capabilities from hoarders?

Ignoring sleeping threads for the moment, I think the flow needs to be something like:

1. schedule ASTs on target threads
2. wait until target threads have blocked in their AST handler
3. revoke capabilities from kernel hoarders
4. increment the generation counter
5. signal blocked threads to revoke their registers (or have them do this before blocking) and return to userspace

You're right, we do need them to block while we increment the generation counter and revoke hoarders.

I am not sure how to handle sleeping threads then. We can perhaps prevent them from running after a wakeup until after step 4 is done. But how do we know that there aren't any to-be-revoked capabilities somewhere on the sleeping thread's kernel stack?

[jrtc27]

Isn't that one of the reasons we use thread_single? It ensures all threads are at the user-kernel boundary, so none of the kernel stack should be holding onto user capabilities.

[markjdb]

Isn't that one of the reasons we use thread_single? It ensures all threads are at the user-kernel boundary, so none of the kernel stack should be holding onto user capabilities.

Presumably yes. So, I think to fix underlying the problem we'd need to scan the kernel stack of sleeping threads, rather than interrupt them. (Probably with some optimization to avoid scanning a stack more than once if the thread hasn't run on a CPU since the last scan.)

[jrtc27]

Having a capability change value sounds rather exciting for the kernel. In particular, for if (cheri_tag_get(ptr_from_user)) { ... } patterns.

[jrtc27]

You'd also need to scan every trapframe for that thread, not just the userspace one. But I guess those are all on the kernel stack...

[markjdb]

Having a capability change value sounds rather exciting for the kernel. In particular, for if (cheri_tag_get(ptr_from_user)) { ... } patterns.

Yeah. I'd naively hope that the {...} is quite unlikely to contain an interruptible sleep.

[nwf]

scan the kernel stack

Oh boy is that going to be exciting! I know thread_single() is a very heavy hammer, but all the alternatives seemed scary.

of sleeping threads

Everybody associated with this address space who's further into the kernel than the syscall boundary is going to need to get back to the syscall boundary or have their stacks scanned one way or another.

I guess the good news is that userspace revocation deletes only caps to userspace addresses and most accesses through userspace pointers are prepared to deal with faults, CHERI or otherwise...

(Probably with some optimization to avoid scanning a stack more than once if the thread hasn't run on a CPU since the last scan.)

Alas, every epoch brings with it different revoked pointers, so I think it really needs to be an "every time, all the time" thing.

(While you're thinking about threads and revocation and letting some threads start running again before others, I think that ptrace() can use thread register files as hoards?)

[markjdb]

scan the kernel stack

Oh boy is that going to be exciting! I know thread_single() is a very heavy hammer, but all the alternatives seemed scary.

Yeah, certainly the current approach is much easier to deal with. I wonder if we could instead try to transparently restart interrupt system calls instead. Calling thread_single() causes interruptible sleeps to return ERESTART, but various system calls convert this to EINTR since they assume that the interruption is due to a signal.

I also note that in upstream FreeBSD, ptrace(PT_ATTACH)/SIGSTOP were recently changed to avoid bouncing the target to the user/kernel boundary, specifically because it ends up being too disruptive in the same way that thread_single() is here.

of sleeping threads

Everybody associated with this address space who's further into the kernel than the syscall boundary is going to need to get back to the syscall boundary or have their stacks scanned one way or another.

I guess the good news is that userspace revocation deletes only caps to userspace addresses and most accesses through userspace pointers are prepared to deal with faults, CHERI or otherwise...

(Probably with some optimization to avoid scanning a stack more than once if the thread hasn't run on a CPU since the last scan.)

Alas, every epoch brings with it different revoked pointers, so I think it really needs to be an "every time, all the time" thing.

Yes, that wasn't well-thought out. I suppose we could avoid repeated scans only if the first one reveals that the stack contains no user capabilities at all, but that's probably never going to be the case for a thread in a system call.

(While you're thinking about threads and revocation and letting some threads start running again before others, I think that ptrace() can use thread register files as hoards?)

I think I'm convinced that the rendezvous needs to be mostly synchronous, i.e., all threads in the process should be sleeping while we revoke from hoarders.

[brooksdavis]

I'm not convinced this is useful or implementable in practice, but for many system calls you can tell if it's "safe" to leave them in the kernel by examining their trap frame. In particular, I think syscalls with no pointer arguments or pointer arguments that don't point to things with pointers in them.

Take nanosleep as one example. If the pointer arguments would be revoked (yikes!) then you have to interrupt it and the caller deserves what they get. Otherwise it seems fine to scrub the stack frame while leaving the thread alone so long as you can prevent it from racing the return to userspace. Logistically this all sounds hard to get right and you have to trust the syscall implementation, but I nonetheless wonder if it could work.

If it could work, would it be useful given that many things can't be handled this way? At a stretch you might be able to find a way to make writev work, but ioctl is going to have to be forced to the user/kernel boundary no matter what.

[nwf]

Belatedly: rather than attempting to zorch pointers on kernel stacks and deal with the consequences, you might take them as a sign that the call in progress needs to complete or be aborted (that is, get back to the syscall boundary one way or the other).