public inbox for linux-kernel@vger.kernel.org
 help / color / mirror / Atom feed
* [1/6 PATCH] Kprobes : Prevent possible race conditions generic changes
@ 2005-07-07 10:10 Prasanna S Panchamukhi
  2005-07-07 10:11 ` [2/6 PATCH] Kprobes : Prevent possible race conditions i386 changes Prasanna S Panchamukhi
  2005-07-07 10:25 ` [1/6 PATCH] Kprobes : Prevent possible race conditions generic changes Andrew Morton
  0 siblings, 2 replies; 19+ messages in thread
From: Prasanna S Panchamukhi @ 2005-07-07 10:10 UTC (permalink / raw)
  To: akpm, Andi Kleen, David S. Miller, systemtap; +Cc: linux-kernel

Hi,

Please provide your feedback on this kprobes patch set.

Thanks
Prasanna


There are possible race conditions if probes are placed on routines within the
kprobes files and routines used by the kprobes. For example if you put probe on
get_kprobe() routines, the system can hang while inserting probes on
any routine such as do_fork(). Because while inserting probes on do_fork(),
register_kprobes() routine grabs the kprobes spin lock and executes get_kprobe()
routine and to handle probe of get_kprobe(), kprobes_handler() gets executed 
and tries to grab kprobes spin lock, and spins forever. This patch avoids such
possible race conditions by preventing probes on routines within the kprobes
file and routines used by kprobes.

I have modified the patches as per Andi Kleen's suggestion to move kprobes
routines and other routines used by kprobes to a seperate section .kprobes.text.
Also moved page fault and exception handlers to .kprobes.text section.
These patches have been tested on i386, x86_64 and ppc64 architectures,
also compiled on ia64 and sparc64 architectures.

Signed-off-by: Prasanna S Panchamukhi <prasanna@in.ibm.com>


---

 linux-2.6.13-rc1-mm1-prasanna/include/asm-generic/vmlinux.lds.h |    5 
 linux-2.6.13-rc1-mm1-prasanna/include/linux/kprobes.h           |    4 
 linux-2.6.13-rc1-mm1-prasanna/kernel/kprobes.c                  |   69 ++++++----
 3 files changed, 51 insertions(+), 27 deletions(-)

diff -puN kernel/kprobes.c~kprobes-exclude-functions-generic kernel/kprobes.c
--- linux-2.6.13-rc1-mm1/kernel/kprobes.c~kprobes-exclude-functions-generic	2005-07-06 18:51:16.000000000 +0530
+++ linux-2.6.13-rc1-mm1-prasanna/kernel/kprobes.c	2005-07-06 18:51:45.000000000 +0530
@@ -72,7 +72,7 @@ static struct hlist_head kprobe_insn_pag
  * 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 *get_insn_slot(void)
+kprobe_opcode_t * __kprobes get_insn_slot(void)
 {
 	struct kprobe_insn_page *kip;
 	struct hlist_node *pos;
@@ -117,7 +117,7 @@ kprobe_opcode_t *get_insn_slot(void)
 	return kip->insns;
 }
 
-void free_insn_slot(kprobe_opcode_t *slot)
+void __kprobes free_insn_slot(kprobe_opcode_t *slot)
 {
 	struct kprobe_insn_page *kip;
 	struct hlist_node *pos;
@@ -152,20 +152,20 @@ void free_insn_slot(kprobe_opcode_t *slo
 }
 
 /* Locks kprobe: irqs must be disabled */
-void lock_kprobes(void)
+void __kprobes lock_kprobes(void)
 {
 	spin_lock(&kprobe_lock);
 	kprobe_cpu = smp_processor_id();
 }
 
-void unlock_kprobes(void)
+void __kprobes unlock_kprobes(void)
 {
 	kprobe_cpu = NR_CPUS;
 	spin_unlock(&kprobe_lock);
 }
 
 /* You have to be holding the kprobe_lock */
-struct kprobe *get_kprobe(void *addr)
+struct kprobe * __kprobes get_kprobe(void *addr)
 {
 	struct hlist_head *head;
 	struct hlist_node *node;
@@ -183,7 +183,7 @@ struct kprobe *get_kprobe(void *addr)
  * Aggregate handlers for multiple kprobes support - these handlers
  * take care of invoking the individual kprobe handlers on p->list
  */
-static int aggr_pre_handler(struct kprobe *p, struct pt_regs *regs)
+static int __kprobes aggr_pre_handler(struct kprobe *p, struct pt_regs *regs)
 {
 	struct kprobe *kp;
 
@@ -198,8 +198,8 @@ static int aggr_pre_handler(struct kprob
 	return 0;
 }
 
-static void aggr_post_handler(struct kprobe *p, struct pt_regs *regs,
-			      unsigned long flags)
+static void __kprobes aggr_post_handler(struct kprobe *p, struct pt_regs *regs,
+					unsigned long flags)
 {
 	struct kprobe *kp;
 
@@ -213,8 +213,8 @@ static void aggr_post_handler(struct kpr
 	return;
 }
 
-static int aggr_fault_handler(struct kprobe *p, struct pt_regs *regs,
-			      int trapnr)
+static int __kprobes aggr_fault_handler(struct kprobe *p, struct pt_regs *regs,
+					int trapnr)
 {
 	/*
 	 * if we faulted "during" the execution of a user specified
@@ -227,7 +227,7 @@ static int aggr_fault_handler(struct kpr
 	return 0;
 }
 
-static int aggr_break_handler(struct kprobe *p, struct pt_regs *regs)
+static int __kprobes aggr_break_handler(struct kprobe *p, struct pt_regs *regs)
 {
 	struct kprobe *kp = curr_kprobe;
 	if (curr_kprobe && kp->break_handler) {
@@ -240,7 +240,7 @@ static int aggr_break_handler(struct kpr
 	return 0;
 }
 
-struct kretprobe_instance *get_free_rp_inst(struct kretprobe *rp)
+struct kretprobe_instance * __kprobes get_free_rp_inst(struct kretprobe *rp)
 {
 	struct hlist_node *node;
 	struct kretprobe_instance *ri;
@@ -249,7 +249,8 @@ struct kretprobe_instance *get_free_rp_i
 	return NULL;
 }
 
-static struct kretprobe_instance *get_used_rp_inst(struct kretprobe *rp)
+static struct kretprobe_instance * __kprobes get_used_rp_inst(struct kretprobe
+							      *rp)
 {
 	struct hlist_node *node;
 	struct kretprobe_instance *ri;
@@ -258,7 +259,7 @@ static struct kretprobe_instance *get_us
 	return NULL;
 }
 
-void add_rp_inst(struct kretprobe_instance *ri)
+void __kprobes add_rp_inst(struct kretprobe_instance *ri)
 {
 	/*
 	 * Remove rp inst off the free list -
@@ -276,7 +277,7 @@ void add_rp_inst(struct kretprobe_instan
 	hlist_add_head(&ri->uflist, &ri->rp->used_instances);
 }
 
-void recycle_rp_inst(struct kretprobe_instance *ri)
+void __kprobes recycle_rp_inst(struct kretprobe_instance *ri)
 {
 	/* remove rp inst off the rprobe_inst_table */
 	hlist_del(&ri->hlist);
@@ -291,7 +292,7 @@ void recycle_rp_inst(struct kretprobe_in
 		kfree(ri);
 }
 
-struct hlist_head * kretprobe_inst_table_head(struct task_struct *tsk)
+struct hlist_head * __kprobes kretprobe_inst_table_head(struct task_struct *tsk)
 {
 	return &kretprobe_inst_table[hash_ptr(tsk, KPROBE_HASH_BITS)];
 }
@@ -302,7 +303,7 @@ struct hlist_head * kretprobe_inst_table
  * instances associated with this task. These left over instances represent
  * probed functions that have been called but will never return.
  */
-void kprobe_flush_task(struct task_struct *tk)
+void __kprobes kprobe_flush_task(struct task_struct *tk)
 {
         struct kretprobe_instance *ri;
         struct hlist_head *head;
@@ -322,7 +323,8 @@ void kprobe_flush_task(struct task_struc
  * This kprobe pre_handler is registered with every kretprobe. When probe
  * hits it will set up the return probe.
  */
-static int pre_handler_kretprobe(struct kprobe *p, struct pt_regs *regs)
+static int __kprobes pre_handler_kretprobe(struct kprobe *p,
+					   struct pt_regs *regs)
 {
 	struct kretprobe *rp = container_of(p, struct kretprobe, kp);
 
@@ -353,7 +355,7 @@ static inline void copy_kprobe(struct kp
 * Add the new probe to old_p->list. Fail if this is the
 * second jprobe at the address - two jprobes can't coexist
 */
-static int add_new_kprobe(struct kprobe *old_p, struct kprobe *p)
+static int __kprobes add_new_kprobe(struct kprobe *old_p, struct kprobe *p)
 {
         struct kprobe *kp;
 
@@ -395,7 +397,8 @@ static inline void add_aggr_kprobe(struc
  * the intricacies
  * TODO: Move kcalloc outside the spinlock
  */
-static int register_aggr_kprobe(struct kprobe *old_p, struct kprobe *p)
+static int __kprobes register_aggr_kprobe(struct kprobe *old_p,
+					  struct kprobe *p)
 {
 	int ret = 0;
 	struct kprobe *ap;
@@ -434,12 +437,24 @@ static inline void cleanup_aggr_kprobe(s
 		spin_unlock_irqrestore(&kprobe_lock, flags);
 }
 
-int register_kprobe(struct kprobe *p)
+static int __kprobes in_kprobes_functions(unsigned long addr)
+{
+	/* Linker adds these: start and end of __kprobes functions */
+	extern char __kprobes_text_start[], __kprobes_text_end[];
+	if (addr >= (unsigned long)__kprobes_text_start
+		&& addr < (unsigned long)__kprobes_text_end)
+		return -EINVAL;
+	return 0;
+}
+
+int __kprobes register_kprobe(struct kprobe *p)
 {
 	int ret = 0;
 	unsigned long flags = 0;
 	struct kprobe *old_p;
 
+	if ((ret = in_kprobes_functions((unsigned long) p->addr)) !=0)
+		return ret;
 	if ((ret = arch_prepare_kprobe(p)) != 0) {
 		goto rm_kprobe;
 	}
@@ -466,7 +481,7 @@ rm_kprobe:
 	return ret;
 }
 
-void unregister_kprobe(struct kprobe *p)
+void __kprobes unregister_kprobe(struct kprobe *p)
 {
 	unsigned long flags;
 	struct kprobe *old_p;
@@ -487,7 +502,7 @@ static struct notifier_block kprobe_exce
 	.priority = 0x7fffffff /* we need to notified first */
 };
 
-int register_jprobe(struct jprobe *jp)
+int __kprobes register_jprobe(struct jprobe *jp)
 {
 	/* Todo: Verify probepoint is a function entry point */
 	jp->kp.pre_handler = setjmp_pre_handler;
@@ -496,14 +511,14 @@ int register_jprobe(struct jprobe *jp)
 	return register_kprobe(&jp->kp);
 }
 
-void unregister_jprobe(struct jprobe *jp)
+void __kprobes unregister_jprobe(struct jprobe *jp)
 {
 	unregister_kprobe(&jp->kp);
 }
 
 #ifdef ARCH_SUPPORTS_KRETPROBES
 
-int register_kretprobe(struct kretprobe *rp)
+int __kprobes register_kretprobe(struct kretprobe *rp)
 {
 	int ret = 0;
 	struct kretprobe_instance *inst;
@@ -540,14 +555,14 @@ int register_kretprobe(struct kretprobe 
 
 #else /* ARCH_SUPPORTS_KRETPROBES */
 
-int register_kretprobe(struct kretprobe *rp)
+int __kprobes register_kretprobe(struct kretprobe *rp)
 {
 	return -ENOSYS;
 }
 
 #endif /* ARCH_SUPPORTS_KRETPROBES */
 
-void unregister_kretprobe(struct kretprobe *rp)
+void __kprobes unregister_kretprobe(struct kretprobe *rp)
 {
 	unsigned long flags;
 	struct kretprobe_instance *ri;
diff -puN include/linux/kprobes.h~kprobes-exclude-functions-generic include/linux/kprobes.h
--- linux-2.6.13-rc1-mm1/include/linux/kprobes.h~kprobes-exclude-functions-generic	2005-07-06 18:51:16.000000000 +0530
+++ linux-2.6.13-rc1-mm1-prasanna/include/linux/kprobes.h	2005-07-06 18:51:16.000000000 +0530
@@ -42,6 +42,10 @@
 #define KPROBE_REENTER		0x00000004
 #define KPROBE_HIT_SSDONE	0x00000008
 
+/* Attach to insert probes on any functions which should be ignored*/
+#define __kprobes	__attribute__((__section__(".kprobes.text")))
+/* Is this address in the __kprobes functions? */
+
 struct kprobe;
 struct pt_regs;
 struct kretprobe;
diff -puN include/asm-generic/vmlinux.lds.h~kprobes-exclude-functions-generic include/asm-generic/vmlinux.lds.h
--- linux-2.6.13-rc1-mm1/include/asm-generic/vmlinux.lds.h~kprobes-exclude-functions-generic	2005-07-06 18:51:16.000000000 +0530
+++ linux-2.6.13-rc1-mm1-prasanna/include/asm-generic/vmlinux.lds.h	2005-07-06 18:51:16.000000000 +0530
@@ -95,3 +95,8 @@
 		VMLINUX_SYMBOL(__lock_text_start) = .;			\
 		*(.spinlock.text)					\
 		VMLINUX_SYMBOL(__lock_text_end) = .;
+
+#define KPROBES_TEXT							\
+		VMLINUX_SYMBOL(__kprobes_text_start) = .;			\
+		*(.kprobes.text)					\
+		VMLINUX_SYMBOL(__kprobes_text_end) = .;

_
-- 

Prasanna S Panchamukhi
Linux Technology Center
India Software Labs, IBM Bangalore
Ph: 91-80-25044636
<prasanna@in.ibm.com>

^ permalink raw reply	[flat|nested] 19+ messages in thread
end of thread, other threads:[~2005-07-08 19:20 UTC | newest]

Thread overview: 19+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2005-07-07 10:10 [1/6 PATCH] Kprobes : Prevent possible race conditions generic changes Prasanna S Panchamukhi
2005-07-07 10:11 ` [2/6 PATCH] Kprobes : Prevent possible race conditions i386 changes Prasanna S Panchamukhi
2005-07-07 10:14   ` [3/6 PATCH] Kprobes : Prevent possible race conditions x86_64 changes Prasanna S Panchamukhi
2005-07-07 10:16     ` [4/6 PATCH] Kprobes : Prevent possible race conditions ppc64 changes Prasanna S Panchamukhi
2005-07-07 10:18       ` [5/6 PATCH] Kprobes : Prevent possible race conditions ia64 changes Prasanna S Panchamukhi
2005-07-07 10:20         ` [6/6 PATCH] Kprobes : Prevent possible race conditions sparc64 changes Prasanna S Panchamukhi
2005-07-08  2:06         ` [5/6 PATCH] Kprobes : Prevent possible race conditions ia64 changes Keshavamurthy Anil S
2005-07-08 11:10           ` Prasanna S Panchamukhi
2005-07-08 19:01             ` Keshavamurthy Anil S
2005-07-08 19:16               ` Prarit Bhargava
2005-07-08 14:13       ` [4/6 PATCH] Kprobes : Prevent possible race conditions ppc64 changes Ananth N Mavinakayanahalli
2005-07-08 11:07     ` [3/6 PATCH] Kprobes : Prevent possible race conditions x86_64 changes Prasanna S Panchamukhi
2005-07-07 13:19   ` [2/6 PATCH] Kprobes : Prevent possible race conditions i386 changes Andi Kleen
2005-07-08 11:05     ` Prasanna S Panchamukhi
2005-07-07 10:25 ` [1/6 PATCH] Kprobes : Prevent possible race conditions generic changes Andrew Morton
2005-07-07 10:34   ` Andi Kleen
2005-07-07 13:01     ` Prasanna S Panchamukhi
2005-07-08 11:02       ` Prasanna S Panchamukhi
2005-07-07 21:20     ` Vara Prasad

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox