linux-kernel - Re: [PATCH v4 19/23] kprobes: Remove kretprobe hash

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]

Date:   Fri, 28 Aug 2020 22:29:12 +0200
From:   Peter Zijlstra <peterz@...radead.org>
To:     "Eddy_Wu@...ndmicro.com" <Eddy_Wu@...ndmicro.com>
Cc:     Masami Hiramatsu <mhiramat@...nel.org>,
        "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH v4 19/23] kprobes: Remove kretprobe hash

On Fri, Aug 28, 2020 at 07:32:11PM +0000, Eddy_Wu@...ndmicro.com wrote:
> 
> > -----Original Message-----
> > From: Masami Hiramatsu <mhiramat@...nel.org>
> >
> > @@ -1311,24 +1257,23 @@ void kprobe_busy_end(void)
> >  void kprobe_flush_task(struct task_struct *tk)
> >  {
> >         struct kretprobe_instance *ri;
> > +       struct llist_node *node;
> >
> > +       /* Early boot, not yet initialized. */
> >         if (unlikely(!kprobes_initialized))
> >                 return;
> >
> >         kprobe_busy_begin();
> >
> > +       node = current->kretprobe_instances.first;
> > +       current->kretprobe_instances.first = NULL;
> 
> I think we are flushing tk instead of current here.
> After fixing this to tk, the NULL pointer deference is gone!

Ah, very good! I can indeed confirm that that cures things.

---
Index: linux-2.6/kernel/kprobes.c
===================================================================
--- linux-2.6.orig/kernel/kprobes.c
+++ linux-2.6/kernel/kprobes.c
@@ -1275,9 +1275,7 @@ void kprobe_flush_task(struct task_struc

 	kprobe_busy_begin();

-	node = current->kretprobe_instances.first;
-	current->kretprobe_instances.first = NULL;
-
+	node == __llist_del_all(&tk->kretprobe_instances);
 	while (node) {
 		ri = container_of(node, struct kretprobe_instance, llist);
 		node = node->next;
@@ -1871,6 +1869,7 @@ unsigned long __kretprobe_trampoline_han
 	struct llist_node *first, *node;
 	struct kretprobe *rp;

+	/* Find all nodes for this frame. */
 	first = node = current->kretprobe_instances.first;
 	while (node) {
 		ri = container_of(node, struct kretprobe_instance, llist);
@@ -1900,7 +1899,7 @@ unsigned long __kretprobe_trampoline_han
 	while (first) {
 		ri = container_of(first, struct kretprobe_instance, llist);
 		rp = get_kretprobe(ri);
-		node = first->next;
+		first = first->next;

 		if (rp && rp->handler) {
 			__this_cpu_write(current_kprobe, &rp->kp);
@@ -1910,8 +1909,6 @@ unsigned long __kretprobe_trampoline_han
 		}

 		recycle_rp_inst(ri);
-
-		first = node;
 	}

 	return (unsigned long)correct_ret_addr;
Index: linux-2.6/include/linux/llist.h
===================================================================
--- linux-2.6.orig/include/linux/llist.h
+++ linux-2.6/include/linux/llist.h
@@ -237,6 +237,14 @@ static inline struct llist_node *llist_d
 	return xchg(&head->first, NULL);
 }

+static inline struct llist_node *__llist_del_all(struct llist_head *head)
+{
+	struct llist_node *first = head->first;
+
+	head->first = NULL;
+	return first;
+}
+
 extern struct llist_node *llist_del_first(struct llist_head *head);

 struct llist_node *llist_reverse_order(struct llist_node *head);