diff options
author | Markus Metzger <markus.t.metzger@intel.com> | 2009-02-11 09:10:27 -0500 |
---|---|---|
committer | Ingo Molnar <mingo@elte.hu> | 2009-02-11 09:44:20 -0500 |
commit | 9f339e7028e2855717af3193c938f9960ad13b38 (patch) | |
tree | 76e0e9181f4ee2b324742d517518e837d5c250bf | |
parent | 06eb23b1ba39c61ee5d5faeb42a097635693e370 (diff) |
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>
-rw-r--r-- | arch/x86/kernel/ptrace.c | 16 | ||||
-rw-r--r-- | include/linux/mm.h | 1 | ||||
-rw-r--r-- | mm/mlock.c | 7 |
3 files changed, 17 insertions, 7 deletions
diff --git a/arch/x86/kernel/ptrace.c b/arch/x86/kernel/ptrace.c index 0a5df5f82fb9..5a4c23d89892 100644 --- a/arch/x86/kernel/ptrace.c +++ b/arch/x86/kernel/ptrace.c | |||
@@ -810,12 +810,16 @@ static void ptrace_bts_untrace(struct task_struct *child) | |||
810 | 810 | ||
811 | static void ptrace_bts_detach(struct task_struct *child) | 811 | static void ptrace_bts_detach(struct task_struct *child) |
812 | { | 812 | { |
813 | if (unlikely(child->bts)) { | 813 | /* |
814 | ds_release_bts(child->bts); | 814 | * Ptrace_detach() races with ptrace_untrace() in case |
815 | child->bts = NULL; | 815 | * the child dies and is reaped by another thread. |
816 | 816 | * | |
817 | ptrace_bts_free_buffer(child); | 817 | * We only do the memory accounting at this point and |
818 | } | 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 | #else | 824 | #else |
821 | static inline void ptrace_bts_fork(struct task_struct *tsk) {} | 825 | static inline void ptrace_bts_fork(struct task_struct *tsk) {} |
diff --git a/include/linux/mm.h b/include/linux/mm.h index e8ddc98b8405..3d7fb44d7d7e 100644 --- a/include/linux/mm.h +++ b/include/linux/mm.h | |||
@@ -1305,5 +1305,6 @@ void vmemmap_populate_print_last(void); | |||
1305 | 1305 | ||
1306 | extern void *alloc_locked_buffer(size_t size); | 1306 | extern void *alloc_locked_buffer(size_t size); |
1307 | extern void free_locked_buffer(void *buffer, size_t size); | 1307 | extern void free_locked_buffer(void *buffer, size_t size); |
1308 | extern void release_locked_buffer(void *buffer, size_t size); | ||
1308 | #endif /* __KERNEL__ */ | 1309 | #endif /* __KERNEL__ */ |
1309 | #endif /* _LINUX_MM_H */ | 1310 | #endif /* _LINUX_MM_H */ |
diff --git a/mm/mlock.c b/mm/mlock.c index 028ec482fdd4..2b57f7e60390 100644 --- a/mm/mlock.c +++ b/mm/mlock.c | |||
@@ -657,7 +657,7 @@ void *alloc_locked_buffer(size_t size) | |||
657 | return buffer; | 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 | unsigned long pgsz = PAGE_ALIGN(size) >> PAGE_SHIFT; | 662 | unsigned long pgsz = PAGE_ALIGN(size) >> PAGE_SHIFT; |
663 | 663 | ||
@@ -667,6 +667,11 @@ void free_locked_buffer(void *buffer, size_t size) | |||
667 | current->mm->locked_vm -= pgsz; | 667 | current->mm->locked_vm -= pgsz; |
668 | 668 | ||
669 | up_write(¤t->mm->mmap_sem); | 669 | up_write(¤t->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 | kfree(buffer); | 676 | kfree(buffer); |
672 | } | 677 | } |