Received-SPF: pass (google.com: best guess record for domain of linux-kernel-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) client-ip=209.132.180.67;
Date:   Wed, 21 Aug 2019 16:02:54 +0200 (CEST)
From:   Thomas Gleixner <tglx@linutronix.de>
To:     Julien Grall <julien.grall@arm.com>
cc:     linux-rt-users@vger.kernel.org, linux-kernel@vger.kernel.org,
        maz@kernel.org, bigeasy@linutronix.de, rostedt@goodmis.org
Subject: Re: [RT PATCH 3/3] hrtimer: Prevent using uninitialized spin_lock
 in hrtimer_grab_expiry_lock()
In-Reply-To: <20190821092409.13225-4-julien.grall@arm.com>
Message-ID: <alpine.DEB.2.21.1908211557420.2223@nanos.tec.linutronix.de>
References: <20190821092409.13225-1-julien.grall@arm.com> <20190821092409.13225-4-julien.grall@arm.com>
User-Agent: Alpine 2.21 (DEB 202 2017-01-01)
MIME-Version: 1.0
Content-Type: text/plain; charset=US-ASCII
Sender: linux-kernel-owner@vger.kernel.org
Precedence: bulk

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).

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.

Thanks,

	tglx