Re: [Devel] [PATCH -mm] utrace: fix double free re __rcu_process_callbacks()

2007-04-24 Thread Alexey Dobriyan 
On Tue, Apr 24, 2007 at 01:10:23PM +0400, Alexey Dobriyan wrote:
> ¹ 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(>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

-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [Devel] [PATCH -mm] utrace: fix double free re __rcu_process_callbacks()

2007-04-24 Thread Kirill Korotaev 
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

Alexey Dobriyan wrote:
> The following patch fixes double free manifesting itself as crash in
> __rcu_process_callbasks():
> http://marc.info/?l=linux-kernel=117518764517017=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(>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.
> 
> 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's
>   BUG_ON(!list_empty(>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));
> 
> --- a/kernel/utrace.c
> +++ b/kernel/utrace.c
> @@ -205,7 +205,6 @@ utrace_clear_tsk(struct task_struct *tsk
>   if (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(>lock);
> - else
> - rcu_utrace_free(utrace);
> + spin_unlock(>lock);
>  
>   /*
>* Now we're finished updating the utrace state.
> 
> ___
> Devel mailing list
> [EMAIL PROTECTED]
> https://openvz.org/mailman/listinfo/devel
> 

-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [Devel] [PATCH -mm] utrace: fix double free re __rcu_process_callbacks()

2007-04-24 Thread Kirill Korotaev 
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

Alexey Dobriyan wrote:
 The following patch fixes double free manifesting itself as crash in
 __rcu_process_callbasks():
 http://marc.info/?l=linux-kernelm=117518764517017w=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.
 
 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'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));
 
 --- a/kernel/utrace.c
 +++ b/kernel/utrace.c
 @@ -205,7 +205,6 @@ utrace_clear_tsk(struct task_struct *tsk
   if (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
 

-
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [Devel] [PATCH -mm] utrace: fix double free re __rcu_process_callbacks()

2007-04-24 Thread Alexey Dobriyan 
On Tue, Apr 24, 2007 at 01:10:23PM +0400, Alexey Dobriyan wrote:
 ¹ 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

-
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/