Skip to contentSkip to conversation

kprobes: fix race in aggregate kprobe registration

PATCH In review Ananth N Mavinakayanahalli 3 participants 5 messages Dec 6, 2005 – Dec 7, 2005 kprobes

lore
Share & tools
lore

Ananth N Mavinakayanahalli

author
#0 lore raw

PATCH 1/1 kprobes: fix race in aggregate kprobe registration

From: Ananth N Mavinakayanahalli <[email protected]>

When registering multiple kprobes at the same address, we leave a small window where the kprobe hlist will not contain a reference to the registered kprobe, leading to potentially, a system crash if the breakpoint is hit on another processor.

Patch below changes the order of hlist updation to make sure that there is always a reference to the kprobe at the location.

Signed-off-by Ananth N Mavinakayanahalli <[email protected]>
Acked-by Prasanna S Panchamukhi <[email protected]>
Acked-by Anil S Keshavamurthy <[email protected]>

---

 kernel/kprobes.c |    2 +-
 1 files changed, 1 insertion(+), 1 deletion(-)
kernel/kprobes.c +1 -1
Index: linux-2.6.15-rc5/kernel/kprobes.c
===================================================================
@@ -400,9 +400,9 @@ static inline void add_aggr_kprobe(struc
 	list_add_rcu(&p->list, &ap->list);
 
 	INIT_HLIST_NODE(&ap->hlist);
-	hlist_del_rcu(&p->hlist);
 	hlist_add_head_rcu(&ap->hlist,
 		&kprobe_table[hash_ptr(ap->addr, KPROBE_HASH_BITS)]);
+	hlist_del_rcu(&p->hlist);
 }
 
 /*
#1 lore raw
Quoted 9 lines · Ananth N Mavinakayanahalli

From: Ananth N Mavinakayanahalli <[email protected]>

When registering multiple kprobes at the same address, we leave a small window where the kprobe hlist will not contain a reference to the registered kprobe, leading to potentially, a system crash if the breakpoint is hit on another processor.

Patch below changes the order of hlist updation to make sure that there is always a reference to the kprobe at the location.

Hi Ananth,
How do you like this patch? Here the old entry
will be replace with the new entry automically.

Signed-off-by Anil S Keshavamurthy <[email protected]>
 kernel/kprobes.c |    5 +----
 1 files changed, 1 insertion(+), 4 deletions(-)
kernel/kprobes.c +1 -4
Index: linux-2.6.15-rc5-mm1/kernel/kprobes.c
===================================================================
@@ -399,10 +399,7 @@ static inline void add_aggr_kprobe(struc
 	INIT_LIST_HEAD(&ap->list);
 	list_add_rcu(&p->list, &ap->list);
 
-	INIT_HLIST_NODE(&ap->hlist);
-	hlist_del_rcu(&p->hlist);
-	hlist_add_head_rcu(&ap->hlist,
-		&kprobe_table[hash_ptr(ap->addr, KPROBE_HASH_BITS)]);
+	hlist_replace_rcu(&p->hlist, &ap->hlist);
 }
 
 /*
author Acked-by
#2 lore raw
Quoted 8 lines · Keshavamurthy Anil S
Quoted 7 lines · Ananth N Mavinakayanahalli

From: Ananth N Mavinakayanahalli <[email protected]>

When registering multiple kprobes at the same address, we leave a small window where the kprobe hlist will not contain a reference to the registered kprobe, leading to potentially, a system crash if the breakpoint is hit on another processor.

Quoted 6 lines
Quoted 2 lines

Patch below changes the order of hlist updation to make sure that there is always a reference to the kprobe at the location.

Hi Ananth,
How do you like this patch? Here the old entry
will be replace with the new entry automically.

Your patch looks better.

Andrew,
Anil's patch depends on the list.h updates currently in -mm

Quoted 1 line
Signed-off-by Anil S Keshavamurthy <[email protected]>
Acked-by Ananth N Mavinakayanahalli <[email protected]>
Quoted 20 lines
 kernel/kprobes.c |    5 +----
 1 files changed, 1 insertion(+), 4 deletions(-)

Index: linux-2.6.15-rc5-mm1/kernel/kprobes.c
===================================================================

kernel/kprobes.c +1 -4
@@ -399,10 +399,7 @@ static inline void add_aggr_kprobe(struc
 	INIT_LIST_HEAD(&ap->list);
 	list_add_rcu(&p->list, &ap->list);
 
-	INIT_HLIST_NODE(&ap->hlist);
-	hlist_del_rcu(&p->hlist);
-	hlist_add_head_rcu(&ap->hlist,
-		&kprobe_table[hash_ptr(ap->addr, KPROBE_HASH_BITS)]);
+	hlist_replace_rcu(&p->hlist, &ap->hlist);
 }
 
 /*
#3 lore raw
Quoted 5 lines · Ananth N Mavinakayanahalli
Quoted 4 lines

Hi Ananth,
How do you like this patch? Here the old entry
will be replace with the new entry automically.

Quoted 7 lines

Your patch looks better.

Andrew,
Anil's patch depends on the list.h updates currently in -mm

Quoted 1 line
Signed-off-by Anil S Keshavamurthy <[email protected]>
Acked-by Ananth N Mavinakayanahalli <[email protected]>

Am feeling particularly uncreative. Please resend with a changelog.

#4 lore raw

PATCH 1/1 kprobes: fix race in aggregate kprobe registration

In reply to Ananth N Mavinakayanahalli

When registering multiple kprobes at the same address, we leave a small window where the kprobe hlist will not contain a reference to the registered kprobe, leading to potentially, a system crash if the breakpoint is hit on another processor.

Patch below now automically relpace the old kprobe with the new kprobe from the hash list.

Signed-off-by Anil S Keshavamurthy <[email protected]>
Acked-by Ananth N Mavinakayanahalli <[email protected]>
 kernel/kprobes.c |    5 +----
 1 files changed, 1 insertion(+), 4 deletions(-)
kernel/kprobes.c +1 -4
Index: linux-2.6.15-rc5-mm1/kernel/kprobes.c
===================================================================
@@ -399,10 +399,7 @@ static inline void add_aggr_kprobe(struc
 	INIT_LIST_HEAD(&ap->list);
 	list_add_rcu(&p->list, &ap->list);
 
-	INIT_HLIST_NODE(&ap->hlist);
-	hlist_del_rcu(&p->hlist);
-	hlist_add_head_rcu(&ap->hlist,
-		&kprobe_table[hash_ptr(ap->addr, KPROBE_HASH_BITS)]);
+	hlist_replace_rcu(&p->hlist, &ap->hlist);
 }
 
 /*