utrace: fix double free re __rcu_process_callbacks()
PATCH 1/1 utrace: fix double free re __rcu_process_callbacks()
The following patch fixes double free manifesting itself as crash in __rcu_process_callbasks():
http://marc.info/?l=linux-kernel&m=117518764517017&w=2
https://bugzilla.redhat.com/bugzilla/show_bug.cgi?id=229112
The problem is with check_dead_utrace() conditionally scheduling "struct utrace" for freeing but not cleaning struct task_struct::utrace pointer leaving it reachable:
tsk->utrace_flags = flags; if (flags) spin_unlock(&utrace->lock); else rcu_utrace_free(utrace);
OTOH, utrace_release_task() first clears ->utrace pointer, then frees struct utrace itself:
Roland inserted some debugging into 2.6.21-rc6-mm1 so that aforementined double free couldn't be reproduced without seeing
BUG at kernel/utrace.c:176 first. It triggers if one struct utrace were passed to rcu_utrace_free() second time.
With patch applied I no longer see¹ BUG message and double frees on 2-way P3, 8-way ia64, Core 2 Duo boxes. Testcase is at the first link.
I _think_ it adds leak if utrace_reap() takes branch without freeing but, well, I hope Roland will give me some clue on how to fix it too.
---
kernel/utrace.c | 6 +----- 1 file changed, 1 insertion(+), 5 deletions(-)
¹ But I see whole can of other bugs! I think they were already lurking
but weren't easily reproducable without hitting double-free first. FWIW, it's
BUG_ON(!list_empty(&tsk->ptracees)); oops at the beginning of remove_engine() NULL ->report_quiesce call which is absent in ptrace utrace ops BUG_ON(tracehook_check_released(p));
kernel/utrace.c +1 -5
@@ -205,7 +205,6 @@ utrace_clear_tsk(struct task_struct *tskif (utrace->u.live.signal ==NULL ) {task_lock (tsk);if (likely (tsk->utrace !=NULL )) { -rcu_assign_pointer (tsk->utrace,NULL ); tsk->utrace_flags &=UTRACE_ACTION_NOREAP ; }task_unlock (tsk);
@@ -305,10 +304,7 @@ check_dead_utrace(struct task_struct *ts } tsk->utrace_flags = flags; -if (flags) -spin_unlock (&utrace->lock); -else -rcu_utrace_free (utrace); +spin_unlock (&utrace->lock);/* * Now we're finished updating the utrace state.
Roland,
can you please help with it?
current utrace state is far from being stable,
RHEL5 and -mm kernels can be quite easily crashed with some of the exploits we collected so far.
Alexey can help you with any information needed - call traces, test cases, but without your help we can't fix it all ourselfes :/
Thanks,
Kirill
Quoted 71 lines · Alexey Dobriyan
The following patch fixes double free manifesting itself as crash in
__rcu_process_callbasks():
http://marc.info/?l=linux-kernel&m=117518764517017&w=2
https://bugzilla.redhat.com/bugzilla/show_bug.cgi?id=229112The problem is with
check_dead_utrace()conditionally scheduling "struct utrace" for freeing but not cleaning structtask_struct::utracepointer leaving it reachable:tsk->utrace_flags = flags; if (flags) spin_unlock(&utrace->lock); else rcu_utrace_free(utrace);OTOH,
utrace_release_task()first clears ->utrace pointer, then frees struct utrace itself:Roland inserted some debugging into 2.6.21-rc6-mm1 so that aforementined double free couldn't be reproduced without seeing
BUG atkernel/utrace.c:176first. It triggers if one struct utrace were passed torcu_utrace_free()second time.With patch applied I no longer see¹ BUG message and double frees on 2-way P3, 8-way ia64, Core 2 Duo boxes. Testcase is at the first link.
I _think_ it adds leak if
utrace_reap()takes branch without freeing but, well, I hope Roland will give me some clue on how to fix it too.Signed-off-by Alexey Dobriyan <[email protected]>---
kernel/utrace.c | 6 +----- 1 file changed, 1 insertion(+), 5 deletions(-)¹ But I see whole can of other bugs! I think they were already lurking
but weren't easily reproducable without hitting double-free first. FWIW, it'sBUG_ON(!list_empty(&tsk->ptracees)); oops at the beginning of remove_engine() NULL ->report_quiesce call which is absent in ptrace utrace ops BUG_ON(tracehook_check_released(p));kernel/utrace.c +1
-5@@ -205,7 +205,6 @@ utrace_clear_tsk(struct task_struct *tskif (utrace->u.live.signal ==NULL ) {task_lock (tsk);if (likely (tsk->utrace !=NULL )) { -rcu_assign_pointer (tsk->utrace,NULL ); tsk->utrace_flags &=UTRACE_ACTION_NOREAP ; }task_unlock (tsk);@@ -305,10 +304,7 @@ check_dead_utrace(struct task_struct *ts } tsk->utrace_flags = flags; -if (flags) -spin_unlock (&utrace->lock); -else -rcu_utrace_free (utrace); +spin_unlock (&utrace->lock);/* * Now we're finished updating the utrace state. _______________________________________________
Devel mailing list
[email protected]
https://openvz.org/mailman/listinfo/devel
Quoted 4 lines · Alexey Dobriyan
¹ But I see whole can of other bugs! I think they were already lurking
but weren't easily reproducable without hitting double-free first. FWIW, it's
BUG_ON(!list_empty(&tsk->ptracees));
mmm, pretty easily reproduced with:
while true; do killall -9 expl_ptratt 2>/dev/null; killall -9 exe 2>/dev/null; sleep 2; done
vs
while true; do ./expl_ptratt; done