lkml.org 
[lkml]   [2007]   [Jul]   [17]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
Patch in this message
/
Date
From
SubjectRe: [PATCH] posix-timer: fix deletion race

On Tue, 17 Jul 2007, Thomas Gleixner wrote:

> Jeremy Katz experienced a posix-timer related bug on 2.6.14. This is
> caused by a subtle race, which is there since the original posix timer
> commit and persists until today.
>
> timer_delete does:
> lock_timer();
> timer->it_process = NULL;
> unlock_timer();
> release_posix_timer();
>
> timer->it_process is checked in lock_timer() to prevent access to a
> timer, which is on the way to be deleted, but the check happens after
> idr_lock is dropped. This allows release_posix_timer() to delete the
> timer before the lock code can check the timer:
>
> CPU 0 CPU 1
> lock_timer();
> timer->it_process = NULL;
> unlock_timer();
> lock_timer()
> spin_lock(idr_lock);
> timer = idr_find();
> spin_lock(timer->lock);
> spin_unlock(idr_lock);
> release_posix_timer();
> spin_lock(idr_lock);
> idr_remove(timer);
> spin_unlock(idr_lock);
> free_timer(timer);
> if (timer->......)
>
> Change the locking to prevent this.

I tried the patch with my test case, but still see the issue.
Here's my explanation of the double free race:
CPU 0 CPU 1
sys_timer_delete():
lock_timer();
...
unlock_timer(); itimer_delete()
release_posix_timer(): spin_lock_irqsave(timer...)
... ...
sigqueue_free() unlock_timer()
... sigqueue_free()
BUG_ON(!(q->flags...


the timer fires during deletion:
CPU 0 CPU 1
sys_timer_delete():
lock_timer();
...
unlock_timer(); posix_timer_fn()
release_posix_timer(): spin_lock_irqsave(timer...)
... ...
sigqueue_free() posix_timer_event()
... ...
send_sigqueue()
BUG_ON(!(q->flags

Currently, this appears to fix the problem. The system survived 19 hours
before I rebooted to try other things, while the I usually see at least
one of the BUGs within 15 minutes.


--- linux-2.6.14-cgl.original/kernel/posix-timers.c 2007-07-11 16:06:47.000000000 -0700
+++ linux-2.6.14-cgl/kernel/posix-timers.c 2007-07-16 15:25:43.000000000 -0700
@@ -339,7 +339,9 @@ static int posix_timer_fn(void *data)
int si_private = 0;
int ret = HRTIMER_NORESTART;

- spin_lock_irqsave(&timr->it_lock, flags);
+ if(lock_timer(timr->it_id, &flags) == NULL) {
+ return ret;
+ }

if (timr->it.real.interval.tv64 != 0)
si_private = ++timr->it_requeue_pending;
@@ -411,8 +413,10 @@ static void release_posix_timer(struct k
{
if (it_id_set) {
unsigned long flags;
+ /* unmangle timer id */
+ int it_id = tmr->it_id & INT_MAX;
spin_lock_irqsave(&idr_lock, flags);
- idr_remove(&posix_timers_id, tmr->it_id);
+ idr_remove(&posix_timers_id, it_id);
spin_unlock_irqrestore(&idr_lock, flags);
}
sigqueue_free(tmr->sigq);
@@ -545,8 +549,11 @@ sys_timer_create(const clockid_t which_c
*/

out:
- if (error)
+ if (error) {
+ /* mangle the timer id for the release */
+ new_timer->it_id |= ~INT_MAX;
release_posix_timer(new_timer, it_id_set);
+ }

return error;
}
@@ -822,6 +829,8 @@ retry_delete:
put_task_struct(timer->it_process);
timer->it_process = NULL;
}
+ /* mangle the timer ID to prevent anyone else from finding it */
+ timer->it_id |= ~INT_MAX;
unlock_timer(timer, flags);
release_posix_timer(timer, IT_ID_SET);
return 0;
@@ -835,7 +844,9 @@ static inline void itimer_delete(struct
unsigned long flags;

retry_delete:
- spin_lock_irqsave(&timer->it_lock, flags);
+ /* timer already deleted? */
+ if (lock_timer(timer->it_id, &flags) == NULL)
+ return;

if (timer_delete_hook(timer) == TIMER_RETRY) {
unlock_timer(timer, flags);
@@ -851,6 +862,8 @@ retry_delete:
put_task_struct(timer->it_process);
timer->it_process = NULL;
}
+ /* mangle the timer ID to prevent anyone else from finding it */
+ timer->it_id |= ~INT_MAX;
unlock_timer(timer, flags);
release_posix_timer(timer, IT_ID_SET);
}
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
\
 
 \ /
  Last update: 2007-07-17 20:43    [W:0.081 / U:7.332 seconds]
©2003-2018 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site