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] [day] [month] [year] [list]

Message-ID: <20170526084308.091a41cb@xeon-e3>
Date:   Fri, 26 May 2017 08:43:08 -0700
From:   Stephen Hemminger <stephen@...workplumber.org>
To:     David Miller <davem@...emloft.net>
Cc:     jemele@...il.com, jemele@....org, netdev@...r.kernel.org,
        jemele@...gle.com, mlindner@...vell.com,
        linux-kernel@...r.kernel.org
Subject: Re: [PATCH net] sky2: Do not deadlock on sky2_hw_down

On Thu, 25 May 2017 23:58:30 -0400 (EDT)
David Miller <davem@...emloft.net> wrote:

> From: Stephen Hemminger <stephen@...workplumber.org>
> Date: Thu, 25 May 2017 15:05:02 -0700
> 
> > Ok, the issue is that lockdep is being stupid and thinking that
> > seqcount's behave like locks.  
> 
> Well.. they do.  That's why they have that annotation.

Your right, but it has some lock like properties because
the seqcount's assumption about odd and even values.

Lockdep is reporting that accessing a sequence count with softirq disabled
and in another case with softirq enabled is a problem. 

Potential race theoretical race:

 seqcount_begin()             ++count
  ...
		---> IRQ
                --->   Soft IRQ
                                    seqcount_begin()  ++count
					update stats
				    seqcount_end() ++count
      Anything doing seqcount read during this softirq 
      will spin thinking still in critical section

But for this case of statistics, there is nothing reading statistics in softirq
context so it can't happen. Lockdep needs to be smarter to know that.
Simplest way to shut this up is to just disable softirq during the cleanup.