On Sat, Aug 25, 2018 at 9:21 PM, Andy Lutomirski <luto@xxxxxxxxxx> wrote: > On Sat, Aug 25, 2018 at 7:23 PM, Masami Hiramatsu <mhiramat@xxxxxxxxxx> wrote: >> On Fri, 24 Aug 2018 21:23:26 -0700 >> Andy Lutomirski <luto@xxxxxxxxxx> wrote: >>> Couldn't text_poke() use kmap_atomic()? Or, even better, just change CR3? >> >> No, since kmap_atomic() is only for x86_32 and highmem support kernel. >> In x86-64, it seems that returns just a page address. That is not >> good for text_poke, since it needs to make a writable alias for RO >> code page. Hmm, maybe, can we mimic copy_oldmem_page(), it uses ioremap_cache? >> > > I just re-read text_poke(). It's, um, horrible. Not only is the > implementation overcomplicated and probably buggy, but it's SLOOOOOW. > It's totally the wrong API -- poking one instruction at a time > basically can't be efficient on x86. The API should either poke lots > of instructions at once or should be text_poke_begin(); ...; > text_poke_end();. > > Anyway, the attached patch seems to boot. Linus, Kees, etc: is this > too scary of an approach? With the patch applied, text_poke() is a > fantastic exploit target. On the other hand, even without the patch > applied, text_poke() is every bit as juicy. I tried to convince Ingo to use this method for doing "write rarely" and he soundly rejected it. :) I've always liked this because AFAICT, it's local to the CPU. I had proposed it in https://git.kernel.org/pub/scm/linux/kernel/git/kees/linux.git/commit/?h=kspp/write-rarely&id=9ab0cb2618ebbc51f830ceaa06b7d2182fe1a52d With that, text_poke() mostly becomes: rare_write_begin() memcpy(addr, opcode, len); rare_write_end() As for juiciness, if an attacker already has execution control, they can do more interesting things than text_poke(). But regardless, yes, it's always made me uncomfortable. :) -Kees -- Kees Cook Pixel Security
