netdev - Re: [PATCH v2] net: davicom: dm9000: Avoid spinlock recursion during dm9000

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: <20170607.143844.682492319776197931.davem@davemloft.net>
Date:   Wed, 07 Jun 2017 14:38:44 -0400 (EDT)
From:   David Miller <davem@...emloft.net>
To:     liu.xiang6@....com.cn
Cc:     netdev@...r.kernel.org, linux-kernel@...r.kernel.org,
        liuxiang1999@...il.com
Subject: Re: [PATCH v2] net: davicom: dm9000: Avoid spinlock recursion
 during dm9000_timeout routine

From: Liu Xiang <liu.xiang6@....com.cn>
Date: Tue,  6 Jun 2017 22:17:06 +0800

> On the DM9000B, dm9000_phy_write() is called after the main spinlock
> is held, during the dm9000_timeout() routine. Spinlock recursion
> occurs because the main spinlock is requested again in
> dm9000_phy_write(). So spinlock should be avoided in phy operation
> during the dm9000_timeout() routine.
> 
> ---
> v2:
>    dm9000_phy_write_reg is extracted from dm9000_phy_write, with no lock,
>    do the real phy operation.

I told you during your first submission that this is racy.

Conditional locking in multithreaded driver almost never works
properly.

Once we enter the timeout, another asynchronous thread of control (an
interrupt, timer, whatever) can try to do a PHY operation and skip the
locking because the timeout boolean is set.

That's not right.

You will need to find a more correct solution to this locking problem.

Thanks.