netdev - Re:Re: [PATCH v3] 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: <72770aae.bb31.162d9036874.Coremail.liuxiang_1999@126.com>
Date:   Wed, 18 Apr 2018 21:48:22 +0800 (CST)
From:   liuxiang <liuxiang_1999@....com>
To:     "David Miller" <davem@...emloft.net>
Cc:     liu.xiang6@....com.cn, netdev@...r.kernel.org,
        linux-kernel@...r.kernel.org
Subject: Re:Re: [PATCH v3] net: davicom: dm9000: Avoid spinlock recursion
 during dm9000_timeout routine

Hi,
Because the timeout task gets the main spinlock and disable the current cpu's irq, 
there is no other task on the same cpu can run, and tasks on the other cpus can not
enter the dm9000_timeout() again. So in the whole dm9000_timeout() routine, 
db->timeout_cpu can not be changed by other tasks. Although smp_processor_id() may change 
after preempt_enable(), these tasks always get the false result when call dm9000_current_in_timeout.
Only the timeout task get the true result. And if there is no timeout, all the tasks that want to 
do asynchronous phy operation get the false result. So I think this can avoid racy.

At 2018-04-16 23:05:01, "David Miller" <davem@...emloft.net> wrote:
>From: Liu Xiang <liu.xiang6@....com.cn>
>Date: Sat, 14 Apr 2018 16:50:34 +0800
>
>> +static bool dm9000_current_in_timeout(struct board_info *db)
>> +{
>> +	bool ret = false;
>> +
>> +	preempt_disable();
>> +	ret = (db->timeout_cpu == smp_processor_id());
>> +	preempt_enable();
>
>This doesn't work.
>
>As soon as you do preempt_enable(), smp_processor_id() can change.