netdev - Re: [PATCH 2/5] rds: ib: replace spin_lock_irq with 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:   Sun, 12 Mar 2017 10:33:05 +0800
From:   Yanjun Zhu <yanjun.zhu@...cle.com>
To:     Santosh Shilimkar <santosh.shilimkar@...cle.com>,
        netdev@...r.kernel.org, linux-rdma@...r.kernel.org,
        rds-devel@....oracle.com
Subject: Re: [PATCH 2/5] rds: ib: replace spin_lock_irq with spin_lock_irqsave

Sorry. I have no test case to show some issue.
But from Linux Kernel Development Second Edition by Robert Love.

Use spin_lock_irq is dangerous since spin_unlock_irq unconditionally 
enables interrupts.

We can assume the following scenario:

--->the interrupt is disabled.

         spin_lock_irq(lock_ptr);   <---this will disable interrupt again
         list_del(&ic->ib_node);
         spin_unlock_irq(lock_ptr); <---this will enable interrupt

---->the interrupt is enabled.

our code change the state of interrupt. This will make potential risk.
But spin_lock_irqsave/spin_unlock_irqrestore will not make potential risk.

Zhu Yanjun
On 2017/3/10 0:50, Santosh Shilimkar wrote:
> On 3/8/2017 11:26 PM, Zhu Yanjun wrote:
>> It is difficult to make sure the state of the interrupt when this
>> function is called. As such, it is safer to use spin_lock_irqsave
>> than spin_lock_irq.
>>
> There is no reason to hold irqs  and as such the code path is
> safe from irq context. I don't see need of this change unless
> you have test case which showed some issue.
>
> Regards,
> Santosh
>