[prev in list] [next in list] [prev in thread] [next in thread] 

List:       linux-rt-users
Subject:    Re: [RT PATCH 3/3] hrtimer: Prevent using uninitialized spin_lock in hrtimer_grab_expiry_lock()
From:       Julien Grall <julien.grall () arm ! com>
Date:       2019-08-22 10:59:10
Message-ID: 6f637e70-9a7b-47fd-08b0-82b6494d3ae1 () arm ! com
[Download RAW message or body]

Hi Thomas,

Thank you for the review.

On 21/08/2019 15:02, Thomas Gleixner wrote:
> On Wed, 21 Aug 2019, Julien Grall wrote:
> 
>> migration_base is used as a placeholder when an hrtimer is switching
>> between base (see switch_hrtimer_timer_base). It is possible
>> theoritically possible to have timer->base equal to migration_base.
>>
>> Even if it is a placeholder, it would pass all the current check in
>> hrtimer_grab_expiry_lock() leading to use softirq_expiry_lock
>> uninitialized.
>>
>> This is can be prevented by checking whether the base is equal to
>> the placeholder (i.e. migration_base).
> 
> That's a lame argument. The point is that it does not make sense to do that
> on migration base, but not for the reason you are giving (uninitialized
> lock).

Fair point, I will update the commit message.

> 
> If base == migration_base then there is no point to lock soft_expiry_lock
> simply because the timer is not executing the callback in soft irq context
> and the whole lock/unlock dance can be avoided.
> 
> But, yes. Good catch.

Do you want me to resend the series or can I just provide an update to the 
commit message here?

Cheers,

-- 
Julien Grall
[prev in list] [next in list] [prev in thread] [next in thread]