linux-kernel - Re: [PATCH] semaphore: use raw_spin_lock_irq instead of raw_spin_lock

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]

Date:   Thu, 21 Jun 2018 16:20:41 +0200
From:   Juergen Gross <jgross@...e.com>
To:     Ingo Molnar <mingo@...nel.org>,
        Mikulas Patocka <mpatocka@...hat.com>
Cc:     Peter Zijlstra <peterz@...radead.org>,
        Ingo Molnar <mingo@...hat.com>,
        Matthew Wilcox <matthew@....cx>, linux-kernel@...r.kernel.org,
        Thomas Gleixner <tglx@...utronix.de>
Subject: Re: [PATCH] semaphore: use raw_spin_lock_irq instead of
 raw_spin_lock_irqsave

On 21/06/18 16:02, Ingo Molnar wrote:
> 
> * Ingo Molnar <mingo@...nel.org> wrote:
> 
>>
>> * Mikulas Patocka <mpatocka@...hat.com> wrote:
>>
>>> The sleeping functions down, down_interruptible, down_killable and
>>> down_timeout can't be called with interrupts disabled, so we don't have to
>>> save and restore interrupt flag.
>>>
>>> This patch avoids the costly pushf and popf instructions on the semaphore
>>> path.
>>>
>>> Signed-off-by: Mikulas Patocka <mpatocka@...hat.com>
>>>
>>> ---
>>>  kernel/locking/semaphore.c |   21 ++++++++-------------
>>>  1 file changed, 8 insertions(+), 13 deletions(-)
>>
>> I've applied this to the locking tree, I suspect we can do this on the condition 
>> that it doesn't explode in early boot code (which has irqs disabled) and doesn't 
>> generate early boot lockdep splats either.
> 
> Hm, this blew up pretty quick on a pretty regular x86-64 PC white-box, during 
> early bootup:
> 
>   PANIC: early exception 0x08 IP 246:10 error ffffffff811537b2 cr2 0xffff88000240cff8
> 
> and I think it's due to your patch - verifying that now.

I guess local_irq_enable() being called by raw_spin_unlock_irq() makes
its usage in early boot code undesirable.

Maybe it would be possible to use alternatives for that case? They are
applied after enabling interrupts, so pushf and popf instructions could
be patched away.


Juergen