RE: [PATCH] fix hrtimer_enqueue_reprogram race

From: Leonid Shatz
Date: Tue Feb 05 2013 - 05:55:22 EST


The explanation were submitted as possible scenario which could explain how
the bug in kernel could happen and it does not mean that serious designer
could do exactly that. As I said before, it's also possible that a race
between hrtimer_cancel and hrtimer_start can trigger the bug. The idea is to
have kernel more robust. There are already locks used inside hrtimer code,
so why should users of the hrtimer add another layer of locks and get
involved in the intricacy of which cases are protected by internal hrtimer
lock and which are not?

Leonid Shatz

> -----Original Message-----
> From: Thomas Gleixner [mailto:tglx@xxxxxxxxxxxxx]
> Sent: Tuesday, February 05, 2013 12:44 PM
> To: Izik Eidus
> Cc: Andrea Arcangeli; linux-kernel@xxxxxxxxxxxxxxx; leonid Shatz
> Subject: Re: [PATCH] fix hrtimer_enqueue_reprogram race
>
> On Mon, 4 Feb 2013, Izik Eidus wrote:
>
> > From: leonid Shatz <leonid.shatz@xxxxxxxxxxxxxxxxxx>
> >
> > it seems like hrtimer_enqueue_reprogram contain a race which could
> > result in timer.base switch during unlock/lock sequence.
> >
> > See the code at __hrtimer_start_range_ns where it calls
> > hrtimer_enqueue_reprogram. The later is releasing lock protecting the
> > timer base for a short time and timer base switch can occur from a
> > different CPU thread. Later when __hrtimer_start_range_ns calls
> > unlock_hrtimer_base, a base switch could have happened and this causes
> > the bug
> >
> > Try to start the same hrtimer from two different threads in kernel
> > running each one on a different CPU. Eventually one of the calls will
> > cause timer base switch while another thread is not expecting it.
>
> Aside of the bug in the hrtimer code being a real one, writing code which
> fiddles with the same resource (hrtimer) unserialized is broken on its
own.
>
> > This can happen in virtualized environment where one thread can be
> > delayed by lower hypervisor, and due to time delay a different CPU is
> > taking care of missed timer start and runs the timer start logic on its
own.
>
> Without noticing that something else already takes care of it? So you're
> saying that the code in question relies on magic serialization in the
hrtimer
> code. Doesn't look like a brilliant design.
>
> Thanks,
>
> tglx

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