Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1578849

Re: [BUGFIX PATCH tip/master V2 0/3] kprobes: Fix a possible deadlock in kretprobe

Show all headers | View raw

On Fri, 10 Feb 2017 01:28:47 +0900
Masami Hiramatsu <mhiramat@kernel.org> wrote:

> Hi,
> 
> Here is 2nd version of the series. I just updated the
> patch description to make it easier to understand
> on arm and arm64, no code change.
> 
> V1 is here:
> http://www.mail-archive.com/linux-kernel@vger.kernel.org/msg1327856.html
> 
> ----
> This series will fix a possible deadlock case in kretprobe
> on x86, arm, arm64. Since kretprobe has been optimized on
> those arch, they have similar possible deadlock issue.
> 
> Problem
> =====
> 
> The deadlock senario is when a user puts 2 kretprobes, 
> one on normal function and one on a function which can be
> called from NMI or FIQ where normal interrupt disabled.
> (we don't recommend it, but possible.) In this case, if
> the kernel hits the 1st kretprobe on a normal function
> return which calls trampoline_handler(), acquire a 
> spinlock on the hash table in kretprobe_hash_lock() and
> disable irqs.
> After that, if NMI(or FIQ on arm/arm64) is occurred and
> the 2nd kretprobe is kicked, it also calls
> trampoline_handler() and tries to acquire the same 
> spinlock (since the hash is based on current task, same
> as the 1st kretprobe), it causes a deadlock on the
> spinlock.
> Note that this is very rare case, but theoretically happens.

Please ignore this series. I found that the kretprobe
already rejected probes in NMI (and FIQ) so this senario
never be true (as far as in_nmi() works in nested interrupt
context.)
Sorry for bother you.

And thank you!

> 
> Reason and Affected Arch
> =====
> 
> Actually, this bug has been introduced by kretprobe-booster,
> which removes a kprobe from return trampoline code, but also
> resets current kprobe, which can be a stopper for the nested
> k(ret)probes. So, currently only x86, arm, and arm64 are
> affected, because other arch have not implemented the
> kretprobe-booster.
> 
> Solution
> =====
> 
> To fix this issue, I introduced a dummy kprobe which is set
> as a current kprobe while holding the kretprobe-hash lock.
> With that, if an NMI/FIQ occurred and 2nd kretprobe's kprobe
> is kicked (to modify the return address, a kprobe is kicked
> when the target function is called), the kprobe (and the 2nd
> kretprobe also) is skipped because it detects there is
> another kprobe is running.
>     
> This reentrance detection and nested kprobe blocker had
> existed when the original kretprobe was implemented by
> using kprobe on trampoline code. This fix just revived it.
> 
> Thank you,
> 
> ---
> 
> Masami Hiramatsu (3):
>       kprobes/x86: Fix a possible deadlock case in kretprobe
>       kprobes/arm64: Fix a possible deadlock case in kretprobe
>       kprobes/arm: Fix a possible deadlock case in kretprobe
> 
> 
>  arch/arm/probes/kprobes/core.c     |   12 ++++++++++--
>  arch/arm64/kernel/probes/kprobes.c |   12 ++++++++++--
>  arch/x86/kernel/kprobes/core.c     |   13 ++++++++++---
>  3 files changed, 30 insertions(+), 7 deletions(-)
> 
> --
> Masami Hiramatsu


-- 
Masami Hiramatsu <mhiramat@kernel.org>

Back to linux.kernel | Previous | Next — Previous in thread | Find similar | Unroll thread

Thread

[BUGFIX PATCH tip/master V2 0/3] kprobes: Fix a possible deadlock in kretprobe Masami Hiramatsu <mhiramat@kernel.org> - 2017-02-09 17:40 +0100
  [BUGFIX PATCH tip/master V2 2/3] kprobes/arm64: Fix a possible deadlock case in kretprobe Masami Hiramatsu <mhiramat@kernel.org> - 2017-02-09 17:40 +0100
  [BUGFIX PATCH tip/master V2 3/3] kprobes/arm: Fix a possible deadlock case in kretprobe Masami Hiramatsu <mhiramat@kernel.org> - 2017-02-09 17:50 +0100
    Re: [BUGFIX PATCH tip/master V2 3/3] kprobes/arm: Fix a possible  deadlock case in kretprobe Russell King - ARM Linux <linux@armlinux.org.uk> - 2017-02-09 18:30 +0100
      Re: [BUGFIX PATCH tip/master V2 3/3] kprobes/arm: Fix a possible  deadlock case in kretprobe Masami Hiramatsu <mhiramat@kernel.org> - 2017-02-10 03:40 +0100
        Re: [BUGFIX PATCH tip/master V2 3/3] kprobes/arm: Fix a possible  deadlock case in kretprobe Masami Hiramatsu <mhiramat@kernel.org> - 2017-02-10 23:40 +0100
          Re: [BUGFIX PATCH tip/master V2 3/3] kprobes/arm: Fix a possible  deadlock case in kretprobe Russell King - ARM Linux <linux@armlinux.org.uk> - 2017-02-11 00:30 +0100
            Re: [BUGFIX PATCH tip/master V2 3/3] kprobes/arm: Fix a possible  deadlock case in kretprobe Masami Hiramatsu <mhiramat@kernel.org> - 2017-02-11 10:30 +0100
  Re: [BUGFIX PATCH tip/master V2 0/3] kprobes: Fix a possible  deadlock in kretprobe Masami Hiramatsu <mhiramat@kernel.org> - 2017-02-11 00:00 +0100

csiph-web