kprobes: Add kprobe_insn_mutex and cleanup arch_remove_kprobe()
Subsystems (5)
PATCH 2/7 kprobes: Add kprobe_insn_mutex and cleanup arch_remove_kprobe()
Add kprobe_insn_mutex for protecting kprobe_insn_pages hlist, and remove kprobe_mutex from architecture dependent code.
This allows us to call arch_remove_kprobe() (and free_insn_slot) while holding kprobe_mutex.
--- arch/arm/kernel/kprobes.c | 2 -- arch/ia64/kernel/kprobes.c | 8 +++++--- arch/powerpc/kernel/kprobes.c | 7 ++++--- arch/s390/kernel/kprobes.c | 7 ++++--- arch/x86/kernel/kprobes.c | 7 ++++--- include/linux/kprobes.h | 1 - kernel/kprobes.c | 25 +++++++++++++++++++++---- 7 files changed, 38 insertions(+), 19 deletions(-)
arch/arm/kernel/kprobes.c +0 -2
@@ -92,9 +92,7 @@ void __kprobes arch_disarm_kprobe(structvoid __kprobes arch_remove_kprobe (struct kprobe *p) {if (p->ainsn.insn) { -mutex_lock (&kprobe_mutex);free_insn_slot (p->ainsn.insn,0 ); -mutex_unlock (&kprobe_mutex); p->ainsn.insn =NULL ; } }
arch/ia64/kernel/kprobes.c +5 -3
@@ -670,9 +670,11 @@ void __kprobes arch_disarm_kprobe(structvoid __kprobes arch_remove_kprobe (struct kprobe *p) { -mutex_lock (&kprobe_mutex); -free_insn_slot (p->ainsn.insn, p->ainsn.inst_flag &INST_FLAG_BOOSTABLE ); -mutex_unlock (&kprobe_mutex); +if (p->ainsn.insn) { +free_insn_slot (p->ainsn.insn, + p->ainsn.inst_flag &INST_FLAG_BOOSTABLE ); + p->ainsn.insn =NULL ; + } }/* * We are resuming execution after a single step fault, so the pt_regs
arch/powerpc/kernel/kprobes.c +4 -3
@@ -96,9 +96,10 @@ void __kprobes arch_disarm_kprobe(structvoid __kprobes arch_remove_kprobe (struct kprobe *p) { -mutex_lock (&kprobe_mutex); -free_insn_slot (p->ainsn.insn,0 ); -mutex_unlock (&kprobe_mutex); +if (p->ainsn.insn) { +free_insn_slot (p->ainsn.insn,0 ); + p->ainsn.insn =NULL ; + } }static void __kprobes prepare_singlestep (struct kprobe *p,struct pt_regs *regs)
arch/s390/kernel/kprobes.c +4 -3
@@ -218,9 +218,10 @@ void __kprobes arch_disarm_kprobe(structvoid __kprobes arch_remove_kprobe (struct kprobe *p) { -mutex_lock (&kprobe_mutex); -free_insn_slot (p->ainsn.insn,0 ); -mutex_unlock (&kprobe_mutex); +if (p->ainsn.insn) { +free_insn_slot (p->ainsn.insn,0 ); + p->ainsn.insn =NULL ; + } }static void __kprobes prepare_singlestep (struct kprobe *p,struct pt_regs *regs)
arch/x86/kernel/kprobes.c +4 -3
@@ -376,9 +376,10 @@ void __kprobes arch_disarm_kprobe(structvoid __kprobes arch_remove_kprobe (struct kprobe *p) { -mutex_lock (&kprobe_mutex); -free_insn_slot (p->ainsn.insn, (p->ainsn.boostable ==1 )); -mutex_unlock (&kprobe_mutex); +if (p->ainsn.insn) { +free_insn_slot (p->ainsn.insn, (p->ainsn.boostable ==1 )); + p->ainsn.insn =NULL ; + } }static void __kprobes save_previous_kprobe (struct kprobe_ctlblk *kcb)
kernel/kprobes.c +21 -4
@@ -69,7 +69,7 @@ static struct hlist_head kretprobe_inst_/* NOTE: change this value only with kprobe_mutex held */ static bool kprobe_enabled; -DEFINE_MUTEX (kprobe_mutex);/* Protects kprobe_table */ +static DEFINE_MUTEX (kprobe_mutex);/* Protects kprobe_table */ static DEFINE_PER_CPU(struct kprobe *, kprobe_instance) = NULL; static struct { spinlock_t lock ____cacheline_aligned;
@@ -115,6 +115,7 @@ enum kprobe_slot_state { SLOT_USED = 2, }; +static DEFINE_MUTEX (kprobe_insn_mutex);/* Protects kprobe_insn_pages */ static struct hlist_head kprobe_insn_pages;static int kprobe_garbage_slots;static int collect_garbage_slots (void );
@@ -144,10 +145,10 @@ loop_end: }/** -* get_insn_slot() - Find a slot on an executable page for an instruction. +* __get_insn_slot() - Find a slot on an executable page for an instruction. * We allocate an executable page if there's no room on existing ones. */ -kprobe_opcode_t __kprobes *get_insn_slot (void ) +static kprobe_opcode_t __kprobes *__get_insn_slot (void ) {struct kprobe_insn_page *kip;struct hlist_node *pos;
@@ -196,6 +197,15 @@ kprobe_opcode_t __kprobes *get_insn_slotreturn kip->insns; } +kprobe_opcode_t __kprobes *get_insn_slot (void ) +{ +kprobe_opcode_t *ret; +mutex_lock (&kprobe_insn_mutex); + ret =__get_insn_slot (); +mutex_unlock (&kprobe_insn_mutex); +return ret; +} +/* Return 1 if all garbages are collected, otherwise 0. */ static int __kprobes collect_one_slot (struct kprobe_insn_page *kip,int idx) {
@@ -226,9 +236,13 @@ static int __kprobes collect_garbage_slo {struct kprobe_insn_page *kip;struct hlist_node *pos, *next; +int safety;/* Ensure no-one is preepmted on the garbages */ -if (check_safety () !=0 ) +mutex_unlock (&kprobe_insn_mutex); + safety =check_safety (); +mutex_lock (&kprobe_insn_mutex); +if (safety !=0 )return -EAGAIN ;hlist_for_each_entry_safe (kip, pos, next, &kprobe_insn_pages, hlist) {
@@ -251,6 +265,7 @@ void __kprobes free_insn_slot(kprobe_opcstruct kprobe_insn_page *kip;struct hlist_node *pos; +mutex_lock (&kprobe_insn_mutex);hlist_for_each_entry (kip, pos, &kprobe_insn_pages, hlist) {if (kip->insns <= slot && slot < kip->insns + (INSNS_PER_PAGE *MAX_INSN_SIZE )) {
@@ -267,6 +282,8 @@ void __kprobes free_insn_slot(kprobe_opcif (dirty && ++kprobe_garbage_slots >INSNS_PER_PAGE )collect_garbage_slots (); + + mutex_unlock(&kprobe_insn_mutex); }#endif
include/linux/kprobes.h +0 -1
@@ -201,7 +201,6 @@ static inline int init_test_probes(void) }#endif /* CONFIG_KPROBES_SANITY_TEST */ -extern struct mutex kprobe_mutex;extern int arch_prepare_kprobe (struct kprobe *p);extern void arch_arm_kprobe (struct kprobe *p);extern void arch_disarm_kprobe (struct kprobe *p);
Signature · 7 lines
Masami Hiramatsu Software Engineer Hitachi Computer Products (America) Inc. Software Solutions Division e-mail: [email protected]
Quoted 7 lines · Masami Hiramatsu
Add
kprobe_insn_mutexfor protectingkprobe_insn_pageshlist, and removekprobe_mutexfrom architecture dependent code.This allows us to call
arch_remove_kprobe()(andfree_insn_slot) while holdingkprobe_mutex.Signed-off-by Masami Hiramatsu <[email protected]>
Quoted 7 lines · On Tue, 11 Nov 2008 15:56:21 -0500 Masami Hiramatsu <[email protected]> wrote:
Add
kprobe_insn_mutexfor protectingkprobe_insn_pageshlist, and removekprobe_mutexfrom architecture dependent code.This allows us to call
arch_remove_kprobe()(andfree_insn_slot) while holdingkprobe_mutex.Signed-off-by Masami Hiramatsu <[email protected]>
Quoted 8 lines
--- arch/arm/kernel/kprobes.c | 2 -- arch/ia64/kernel/kprobes.c | 8 +++++--- arch/powerpc/kernel/kprobes.c | 7 ++++--- arch/s390/kernel/kprobes.c | 7 ++++--- arch/x86/kernel/kprobes.c | 7 ++++--- include/linux/kprobes.h | 1 - kernel/kprobes.c | 25 +++++++++++++++++++++----
For some reasons sparc64 never had this mutex. You've checked that sparc64 will be OK after this change?
Hi Andrew,
Quoted 19 lines · Andrew Morton
Quoted 7 lines · On Tue, 11 Nov 2008 15:56:21 -0500 Masami Hiramatsu <[email protected]> wrote:
Add
kprobe_insn_mutexfor protectingkprobe_insn_pageshlist, and removekprobe_mutexfrom architecture dependent code.This allows us to call
arch_remove_kprobe()(andfree_insn_slot) while holdingkprobe_mutex.Signed-off-by Masami Hiramatsu <[email protected]>Quoted 8 lines
--- arch/arm/kernel/kprobes.c | 2 -- arch/ia64/kernel/kprobes.c | 8 +++++--- arch/powerpc/kernel/kprobes.c | 7 ++++--- arch/s390/kernel/kprobes.c | 7 ++++--- arch/x86/kernel/kprobes.c | 7 ++++--- include/linux/kprobes.h | 1 - kernel/kprobes.c | 25 +++++++++++++++++++++----
Quoted 2 lines
For some reasons sparc64 never had this mutex. You've checked that sparc64 will be OK after this change?
I think it's OK for those archs.
Actually, this mutex is protecting insn_slot which stores copied instructions into executable memory. Some architectures which doesn't need that special memory have its own buffer in the arch_specific_insn and they don't use insn_slot. On those arch, kprobe_insn_mutex is not used (not defined).
Thank you,
Signature · 7 lines
Masami Hiramatsu Software Engineer Hitachi Computer Products (America) Inc. Software Solutions Division e-mail: [email protected]