|
|
Subscribe / Log in / New account

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

The article only shows the x86 specific helpers and is missing the arch agnostic primitives. However, those are required to understand my concerns. Let me quote them here for completeness:

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.


to post comments

The "rare write" mechanism

Posted Jun 2, 2017 14:07 UTC (Fri) by nurh (guest, #114270) [Link]

I did misread the intent of your LKML post, and I apologize. We've corrected the error. Thank you for the clarification.


Copyright © 2025, Eklektix, Inc.
Comments and public postings are copyrighted by their creators.
Linux is a registered trademark of Linus Torvalds