Messages in this thread | | | Date | Wed, 6 Feb 2013 17:10:11 +0100 | From | Oleg Nesterov <> | Subject | Re: [PATCH] posix-cpu-timers: fix nanosleep task_struct leak |
| |
Stanislaw,
First of all, thank you so much. I knew it was a good idea to cc you ;)
And let me repeat that I forgot everything about this code.
On 02/06, Stanislaw Gruszka wrote: > > In do_cpu_nanosleep() we do posix_cpu_timer_create(), but forgot > corresponding posix_cpu_timer_del(), what lead to task_struct leak.
Plus, it seems we can leave the timer on ->cpu_timers list...
> @@ -1403,6 +1403,7 @@ static int do_cpu_nanosleep(const clockid_t which_clock, int flags, > /* > * Our timer fired and was reset. > */ > + posix_cpu_timer_del(&timer); > spin_unlock_irq(&timer.it_lock); > return 0; > } > @@ -1420,9 +1421,17 @@ static int do_cpu_nanosleep(const clockid_t which_clock, int flags, > * We were interrupted by a signal. > */ > sample_to_timespec(which_clock, timer.it.cpu.expires, rqtp); > - posix_cpu_timer_set(&timer, 0, &zero_it, it); > + error = posix_cpu_timer_set(&timer, 0, &zero_it, it); > + if (!error) > + posix_cpu_timer_del(&timer); > spin_unlock_irq(&timer.it_lock); > > + while (error == TIMER_RETRY) { > + spin_lock_irq(&timer.it_lock); > + error = posix_cpu_timer_del(&timer);
It is not clear to me why other posix_cpu_timer_del's above can't fail.. May be you can add a comment.
And I am not sure that TIMER_RETRY is the only error we should worry. And perhaps we need even more posix_cpu_timer_del's?
For example. Suppose that posix_cpu_timer_create() succeeds and does get_task_struct(p). But than p dies, and the first posix_cpu_timer_set() fails with -ESRCH. No?
Oleg.
| |