The "rare write" mechanism
The "rare write" mechanism
Posted Jun 1, 2017 21:39 UTC (Thu) by minipli (guest, #69735)Parent article: The "rare write" mechanism
static __always_inline rare_write_begin(void)
{
preempt_disable();
local_irq_disable();
barrier();
__arch_rare_write_begin();
barrier();
}
static __always_inline rare_write_end(void)
{
barrier();
__arch_rare_write_end();
barrier();
local_irq_enable();
preempt_enable_no_resched();
}
As can be seen, the rare_write_begin() primitive disables preemption and IRQ delivery on the local CPU. (The latter is a bug, actually, as the code could be called from a section that already had interrupts disabled (think of spin_lock_irqsave() et al.) and would wrongly re-enable them in rare_write_end(). But that's just another flaw of that code I don't want to focus on too much.) The point is, the local_irq_disable() doesn't prevent NMIs from interrupting a rare_write_begin()...rare_write_end() block. The NMI handler would run with CR0.WP disabled and, in turn, would allow an attacker to exploit this situation as mechanisms like COW won't work any more. So I raised that concern, as mentioned in the article, but I didn't question the implementation in PaX! In fact, well, let me just quote my email here as well:
"""
Well, doesn't look good to me. NMIs will still be able to interrupt
this code and will run with CR0.WP = 0.
Shouldn't you instead question yourself why PaX can do it "just" with
preempt_disable() instead?!
"""
As I'm not a native speaker my understanding of the second sentence might differ from yours, but, what I wanted to say was: PaX's implementation doesn't need to disable interrupts, it does handle the NMI case (and IRQ case, for that matter) perfectly fine and has a working COW semantic even then. I wasn't questioning PaX's implementation, I was questioning Kees' proposal. In fact, my question was asking *him* why PaX can have such a simple implementation of the rare_write_begin() helper (called pax_open_kernel() in the PaX patch) that is correct while his is not. (Hint: He'd missed a few hunks while copy-paste-bike-shedding.) But that led nowhere, just to Thomas arguing CR0.WP is a bad arch hack to begin with and that it shouldn't be used at all. I didn't quite agree, though.
Posted Jun 2, 2017 14:07 UTC (Fri)
by nurh (guest, #114270)
[Link]
The "rare write" mechanism