Re: [PATCH?] uprobes: change uprobe_write_opcode() to modify the page directly

[Linus Torvalds]

On Tue, Dec 3, 2013 at 10:49 AM, Oleg Nesterov <oleg@xxxxxxxxxx> wrote:
>
> See the patch below. For review only

Looks completely broken. Where do you guarantee that it's just a single page?

Yes, on x86, UPROBE_SWBP_INSN_SIZE is a single byte. But quite
frankly, on x86, exactly *because* it's a single byte, I don't
understand why we don't just write the damn thing with a single
"put_user()", and stop with all the idiotic games. No need to
invalidate caches, even, because if you overwrite the first byte of an
instruction, it all "just works". Either the instruction decoding gets
the old one, or it gets the new one. We already rely on that for the
kernel bp instruction replacement.

And on non-x86, UPROBE_SWBP_INSN_SIZE is not necessarily 1, so it
could cross a page boundary. Yes, many architectures will have
alignment constraints, but I don't see this testing it.

Whatever. I think that code is bad, and you should feel bad. But hey,
I think it was pretty bad before too.

            Linus
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/