linux-kernel - Re: Linux 4.9.6 ( Restore IO-APIC irq_chip retrigger callback , breaks my box )

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <alpine.DEB.2.20.1702130912260.3619@nanos>
Date:   Mon, 13 Feb 2017 09:47:15 +0100 (CET)
From:   Thomas Gleixner <tglx@...utronix.de>
To:     Mike Galbraith <efault@....de>
cc:     Gabriel C <nix.or.die@...il.com>, Borislav Petkov <bp@...en8.de>,
        Linus Torvalds <torvalds@...ux-foundation.org>,
        Greg KH <gregkh@...uxfoundation.org>,
        Linux Kernel Mailing List <linux-kernel@...r.kernel.org>,
        Andrew Morton <akpm@...ux-foundation.org>,
        stable <stable@...r.kernel.org>, lwn@....net,
        Jiri Slaby <jslaby@...e.cz>,
        Ruslan Ruslichenko <rruslich@...co.com>
Subject: Re: Linux 4.9.6 ( Restore IO-APIC irq_chip retrigger callback ,
 breaks my box )

On Mon, 13 Feb 2017, Mike Galbraith wrote:
>  kernel/time/tick-broadcast.c |    5 +++--
>  1 file changed, 3 insertions(+), 2 deletions(-)
> 
> --- a/kernel/time/tick-broadcast.c
> +++ b/kernel/time/tick-broadcast.c
> @@ -357,6 +357,7 @@ void tick_broadcast_control(enum tick_br
>  	struct clock_event_device *bc, *dev;
>  	struct tick_device *td;
>  	int cpu, bc_stopped;
> +	unsigned long flags;
>  
>  	td = this_cpu_ptr(&tick_cpu_device);
>  	dev = td->evtdev;
> @@ -370,7 +371,7 @@ void tick_broadcast_control(enum tick_br
>  	if (!tick_device_is_functional(dev))
>  		return;
>  
> -	raw_spin_lock(&tick_broadcast_lock);
> +	raw_spin_lock_irqsave(&tick_broadcast_lock, flags);
>  	cpu = smp_processor_id();
>  	bc = tick_broadcast_device.evtdev;
>  	bc_stopped = cpumask_empty(tick_broadcast_mask);
> @@ -420,7 +421,7 @@ void tick_broadcast_control(enum tick_br
>  				tick_broadcast_setup_oneshot(bc);
>  		}
>  	}
> -	raw_spin_unlock(&tick_broadcast_lock);
> +	raw_spin_unlock_irqrestore(&tick_broadcast_lock, flags);

That cures the lockdep splat, but the comment above
tick_broadcast_control() says:

* Called with interrupts disabled, so clockevents_lock is not
* required here because the local clock event device cannot go away
* under us.

So if we want to relax the calling convention, then we need to take the
lock early.  Otherwise it's unsafe to fiddle with the local clock event
device.

The calling convention was broken with the following commit:

    29d7bbada98e intel_idle: Remove superfluous SMP fuction call

So we could fix it at the call site, but making the core more robust is the
better solution.

I'll fix it up.

Thanks,

	tglx