netdev - Re: [PATCH net] sky2: Do not deadlock on sky2_hw

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: <20170525220520.GA4706@electric-eye.fr.zoreil.com>
Date:   Fri, 26 May 2017 00:05:20 +0200
From:   Francois Romieu <romieu@...zoreil.com>
To:     David Miller <davem@...emloft.net>
Cc:     jemele@...il.com, netdev@...r.kernel.org, jemele@...gle.com,
        mlindner@...vell.com, stephen@...workplumber.org,
        linux-kernel@...r.kernel.org
Subject: Re: [PATCH net] sky2: Do not deadlock on sky2_hw_down

David Miller <davem@...emloft.net> :
> From: Joshua Emele <jemele@...il.com>
> Date: Wed, 24 May 2017 15:43:18 -0700
[...]
> > The sky2_hw_down uses sky2_tx_complete to free pending frames stuck in
> > the HW queue. Because sky2_hw_down can be called from a process context,
> > the call to u64_stats_update_begin can result in deadlock.
> > 
> > Because the statistics do not require update as part of the sky2_hw_down
> > sequence, prevent the update to avoid the deadlock.
> 
> I disagree.  Taking the interface down should cause events in the
> statistics to be lost.
>
> You're going to have to find a way to fix this without eliding
> the stats increments.

NAPI processing is already disabled at this stage in the device close()
path (and sky2_netpoll() uses napi_schedule).

It's possible to add a conditionnal bh or irq disabling instruction to
silent the warning but it should not be needed at all.

-- 
Ueimor