linux-kernel - Re: [PATCH -tip v3 7/7] kprobes: Use synchronize_rcu_tasks() for optprobe with CONFIG

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20170928072220.iv5subwl4u4zcqcp@gmail.com>
Date:   Thu, 28 Sep 2017 09:22:20 +0200
From:   Ingo Molnar <mingo@...nel.org>
To:     Masami Hiramatsu <mhiramat@...nel.org>
Cc:     mingo@...hat.com, x86@...nel.org,
        Steven Rostedt <rostedt@...dmis.org>,
        linux-kernel@...r.kernel.org,
        Peter Zijlstra <peterz@...radead.org>,
        Ananth N Mavinakayanahalli <ananth@...ux.vnet.ibm.com>,
        Thomas Gleixner <tglx@...utronix.de>,
        "H . Peter Anvin" <hpa@...or.com>,
        "Paul E . McKenney" <paulmck@...ux.vnet.ibm.com>,
        Alexei Starovoitov <ast@...nel.org>,
        Alexei Starovoitov <ast@...com>
Subject: Re: [PATCH -tip v3 7/7] kprobes: Use synchronize_rcu_tasks() for
 optprobe with CONFIG_PREEMPT


* Masami Hiramatsu <mhiramat@...nel.org> wrote:

> To enable jump optimized probe with CONFIG_PREEMPT, use
> synchronize_rcu_tasks() to wait for all tasks preempted
> on trampoline code back on track.

This sentence does not parse. It's missing a verb, but I'm not sure.

> Since the jump optimized kprobes can replace multiple
> instructions, there can be tasks which are preempted
> on the 2nd (or 3rd) instructions. If the kprobe
> replaces those instructions by a jump instruction,
> when those tasks back to the preempted place, it is
> a middle of the jump instruction and causes a kernel
> panic.


Again, sentence appears to be missing a verb and also an adjective I think.

> To avoid such tragedies in advance, kprobe optimizer
> prepare a detour route using normal kprobe (e.g.
> int3 breakpoint on x86), and wait for the tasks which
> is interrrupted on such place by synchronize_sched()
> when CONFIG_PREEMPT=n.

s/tragedies/mishaps

Part after the first comma does not parse.

Also the way to refer to kprobes is "kprobes" and "normal kprobes".
Use 'kprobe' only when talking about a specific kprobe instance or such.
You use this correctly later on in the changelog ...

> If CONFIG_PREEMPT=y, things be more complicated, because

s/be/are or s/be/get

> such interrupted thread can be preempted (other thread
> can be scheduled in interrupt handler.) So, kprobes

full stop in the wrong place.

> optimizer has to wait for those tasks scheduled normally.

missing verb.

> In this case we can use synchronize_rcu_tasks() which
> ensures that all preempted tasks back on track and
> schedule it.

More careful changelogs please.

> +	 * are done. Because optprobe may modify multiple instructions,
> +	 * there is a chance that the Nth instruction is interrupted. In that
> +	 * case, running interrupt can return to the Nth byte of jump
> +	 * instruction. This can be avoided by waiting for returning of
> +	 * such interrupts, since (until here) the first byte of the optimized
> +	 * probe is already replaced with normal kprobe (sw breakpoint) and
> +	 * all threads which reach to the probed address will hit it and
> +	 * bypass the copied instructions (instead of executing the original.)
> +	 * With CONFIG_PREEMPT, such interrupts can be preepmted. To wait
> +	 * for such thread, we will use synchronize_rcu_tasks() which ensures
> +	 * all preeempted tasks are scheduled normally (not preempted).
> +	 * So we can ensure there is no threads preempted at probed address.

What? Interrupts cannot be preempted.

Also, "To wait for such threads", or "To wait for such a thread".

Thanks,

	Ingo