Messages in this thread | | | Date | Wed, 20 Feb 2013 13:23:00 -0800 | From | Andrew Morton <> | Subject | Re: [PATCH] posix-timer: don't call idr_find() w/ negative ID |
| |
On Wed, 20 Feb 2013 13:01:16 -0800 Tejun Heo <tj@kernel.org> wrote:
> Recent idr updates make idr_find() trigger WARN_ON_ONCE() before > returning NULL when a negative ID is specified. Apparently, > posix-timer::__lock_timer() was depending on idr_find() returning NULL > on negative ID, thus triggering the new WARN_ON_ONCE(). Make > __lock_timer() first check whether @timer_id is negative and return > NULL without invoking idr_find() if so. > > Note that the previous code was theoretically broken. idr_find() > masked off the sign bit before performing lookup and if the matching > IDs were in use, it would have returned pointer for the incorrect > entry. > > ... > > --- a/kernel/posix-timers.c > +++ b/kernel/posix-timers.c > @@ -637,6 +637,9 @@ static struct k_itimer *__lock_timer(timer_t timer_id, unsigned long *flags) > { > struct k_itimer *timr; > > + if ((int)timer_id < 0) > + return NULL; > + > rcu_read_lock(); > timr = idr_find(&posix_timers_id, (int)timer_id); > if (timr) {
This is a bit risky - if some arch defines timer_t to be a u64 then we will incorrectly treat 0x0000 0001 ffff ffff as a negative number. (That's a lot of timers!)
A fancy way of avoiding this is
if (timer_id & ((typeof timer_id)1 << (sizeof(timer_id) - 1)))
(approximately ;))
But I think casting to (long) should be good enough?
| |