x86, ptrace, mm: fix double-free on race

Ptrace_detach() races with __ptrace_unlink() if the traced task is reaped while detaching. This might cause a double-free of the BTS buffer. Change the ptrace_detach() path to only do the memory accounting in ptrace_bts_detach() and leave the buffer free to ptrace_bts_untrace() which will be called from __ptrace_unlink(). The fix follows a proposal from Oleg Nesterov. Reported-by: Oleg Nesterov <oleg@redhat.com> Signed-off-by: Markus Metzger <markus.t.metzger@intel.com> Signed-off-by: Ingo Molnar <mingo@elte.hu>

x86, ptrace, mm: fix double-free on race
Ptrace_detach() races with __ptrace_unlink() if the traced task is reaped while detaching. This might cause a double-free of the BTS buffer. Change the ptrace_detach() path to only do the memory accounting in ptrace_bts_detach() and leave the buffer free to ptrace_bts_untrace() which will be called from __ptrace_unlink(). The fix follows a proposal from Oleg Nesterov. Reported-by: Oleg Nesterov <oleg@redhat.com> Signed-off-by: Markus Metzger <markus.t.metzger@intel.com> Signed-off-by: Ingo Molnar <mingo@elte.hu>
Markus Metzger · Ingo Molnar
1 parent 06eb23b1ba
Showing 3 changed files with 17 additions and 7 deletions Side-by-side Diff
arch/x86/kernel/ptrace.c
include/linux/mm.h
mm/mlock.c
@@ -810,12 +810,16 @@
  
 static void ptrace_bts_detach(struct task_struct *child)
 {
-	if (unlikely(child->bts)) {
-		ds_release_bts(child->bts);
-		child->bts = NULL;
-
-		ptrace_bts_free_buffer(child);
-	}
+	/*
+	 * Ptrace_detach() races with ptrace_untrace() in case
+	 * the child dies and is reaped by another thread.
+	 *
+	 * We only do the memory accounting at this point and
+	 * leave the buffer deallocation and the bts tracer
+	 * release to ptrace_bts_untrace() which will be called
+	 * later on with tasklist_lock held.
+	 */
+	release_locked_buffer(child->bts_buffer, child->bts_size);
 }
 #else
 static inline void ptrace_bts_fork(struct task_struct *tsk) {}
@@ -1305,6 +1305,7 @@
  
 extern void *alloc_locked_buffer(size_t size);
 extern void free_locked_buffer(void *buffer, size_t size);
+extern void release_locked_buffer(void *buffer, size_t size);
 #endif /* __KERNEL__ */
 #endif /* _LINUX_MM_H */
@@ -657,7 +657,7 @@
 	return buffer;
 }
  
-void free_locked_buffer(void *buffer, size_t size)
+void release_locked_buffer(void *buffer, size_t size)
 {
 	unsigned long pgsz = PAGE_ALIGN(size) >> PAGE_SHIFT;
  
@@ -667,6 +667,11 @@
 	current->mm->locked_vm -= pgsz;
  
 	up_write(&current->mm->mmap_sem);
+}
+
+void free_locked_buffer(void *buffer, size_t size)
+{
+	release_locked_buffer(buffer, size);
  
 	kfree(buffer);
 }
...	...	@@ -810,12 +810,16 @@
810	810
811	811	static void ptrace_bts_detach(struct task_struct *child)
812	812	{
813		- if (unlikely(child->bts)) {
814		- ds_release_bts(child->bts);
815		- child->bts = NULL;
816		-
817		- ptrace_bts_free_buffer(child);
818		- }
	813	+ /*
	814	+ * Ptrace_detach() races with ptrace_untrace() in case
	815	+ * the child dies and is reaped by another thread.
	816	+ *
	817	+ * We only do the memory accounting at this point and
	818	+ * leave the buffer deallocation and the bts tracer
	819	+ * release to ptrace_bts_untrace() which will be called
	820	+ * later on with tasklist_lock held.
	821	+ */
	822	+ release_locked_buffer(child->bts_buffer, child->bts_size);
819	823	}
820	824	#else
821	825	static inline void ptrace_bts_fork(struct task_struct *tsk) {}
...	...	@@ -1305,6 +1305,7 @@
1305	1305
1306	1306	extern void *alloc_locked_buffer(size_t size);
1307	1307	extern void free_locked_buffer(void *buffer, size_t size);
	1308	+extern void release_locked_buffer(void *buffer, size_t size);
1308	1309	#endif /* __KERNEL__ */
1309	1310	#endif /* _LINUX_MM_H */
...	...	@@ -657,7 +657,7 @@
657	657	return buffer;
658	658	}
659	659
660		-void free_locked_buffer(void *buffer, size_t size)
	660	+void release_locked_buffer(void *buffer, size_t size)
661	661	{
662	662	unsigned long pgsz = PAGE_ALIGN(size) >> PAGE_SHIFT;
663	663
...	...	@@ -667,6 +667,11 @@
667	667	current->mm->locked_vm -= pgsz;
668	668
669	669	up_write(&current->mm->mmap_sem);
	670	+}
	671	+
	672	+void free_locked_buffer(void *buffer, size_t size)
	673	+{
	674	+ release_locked_buffer(buffer, size);
670	675
671	676	kfree(buffer);
672	677	}