Skip to content

Dodgy CLG pmap semantics #2610

Description

@jrtc27

pmap_caploadgen_next bumps the pmap's uclg, but despite being called for the current pmap does not update the hardware state on the current CPU. That only gets updated in pmap_activate, and it has the unusual property that the fast path there also updates the hardware state, unlike everything else about the pmap that's context switched, which is kept up-to-date. kern_cheri_revoke is the only caller of pmap_caploadgen_next, and immediately calls pmap_activate afterwards, which (a) is a strong sign that pmap_caploadgen_next should just do the update (b) is particularly odd because it's trying to switch to the current thread's pmap, which would normally be an entirely redundant thing to do. Moreover, this expectation that pmap_activate not be needed when not context switching is present in vmspace_switch_aio, which skips calling it if the vmspace pointers are the same.

Despite all this oddity, in the single-threaded world view that the current caprevoke code has, where it uses thread_single to ensure it's the only thread in the process running, this seems ok, as no other thread will be using that pmap. However, once you introduce things like AIO and, more importantly, the async revoker, or even coprocesses, you end up with other processes (kernel, or perhaps userspace) sharing the same vmspace as the userspace process requesting revocation, and suddenly this "defer until next context switch then pretend we're doing one for the current thread" behaviour breaks down, as if any of those processes are running we'll bump the epoch without updating their current CPUs' hardware state (and aside from the obvious "running with the wrong epoch" problems can end up tripping the assertion in pmap_caploadgen_update that the pmap and hardware generation are consistent).

c0c6dd4 adds a workaround for this to make vmspace_switch_aio perform a similar seemingly-redundant pmap_activate as kern_cheri_revoke. This should actually fix the problem for the async revoker, since when starting a revocation pass we should know there is no other one in progress, just that the kproc might not yet have got round to noticing the work queue is empty (and therefore if another work request for the same vmspace comes in it will vmspace_switch_aio to it without any other such call in between). For AIO it's probably quite broken as, although we do some hoarding of pending AIOs, I don't know that we do anything about in-progress AIOs, so we may actually be able to get into the truly concurrent case. Similarly for coprocesses you could have all kinds of things running in the same vmspace. Either we need a thread_single variant that applies to the entire vmspace, or we need to be smarter about how we handle concurrency here (e.g. we can do things like IPI all CPUs using the pmap in question to sync the generation bit), possibly (probably?) both. AIOs certainly need something to ensure we don't allow in-progress AIOs to hoard capabilities in kernel stacks etc.

Metadata

Metadata

Labels

No labels
No labels

Type

No type

Projects

No projects

Milestone

No milestone

Relationships

None yet

Development

No branches or pull requests

Issue actions