netdev - Re: [PATCH net] net: dsa: fix lockdep warning

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: <20190217175539.GI5968@lunn.ch>
Date:   Sun, 17 Feb 2019 18:55:39 +0100
From:   Andrew Lunn <andrew@...n.ch>
To:     Russell King <rmk+kernel@...linux.org.uk>
Cc:     Florian Fainelli <f.fainelli@...il.com>,
        Vivien Didelot <vivien.didelot@...il.com>,
        Heiner Kallweit <hkallweit1@...il.com>,
        "David S. Miller" <davem@...emloft.net>, netdev@...r.kernel.org
Subject: Re: [PATCH net] net: dsa: fix lockdep warning

On Sun, Feb 17, 2019 at 04:27:32PM +0000, Russell King wrote:
> ======================================================
> WARNING: possible circular locking dependency detected
> 4.20.0+ #302 Not tainted
> ------------------------------------------------------
> systemd-udevd/160 is trying to acquire lock:
> edea6080 (&chip->reg_lock){+.+.}, at: __setup_irq+0x640/0x704
> 
> but task is already holding lock:
> edff0340 (&desc->request_mutex){+.+.}, at: __setup_irq+0xa0/0x704
> 
> which lock already depends on the new lock.
> 
> the existing dependency chain (in reverse order) is:
> 
> -> #1 (&desc->request_mutex){+.+.}:
>        mutex_lock_nested+0x1c/0x24
>        __setup_irq+0xa0/0x704
>        request_threaded_irq+0xd0/0x150
>        mv88e6xxx_probe+0x41c/0x694 [mv88e6xxx]

> -> #0 (&chip->reg_lock){+.+.}:
>        __mutex_lock+0x50/0x8b8
>        mutex_lock_nested+0x1c/0x24
>        __setup_irq+0x640/0x704
>        request_threaded_irq+0xd0/0x150
>        mv88e6xxx_g2_irq_setup+0xcc/0x1b4 [mv88e6xxx]
>        mv88e6xxx_probe+0x44c/0x694 [mv88e6xxx]
>        mdio_probe+0x2c/0x54
> 
> other info that might help us debug this:
> 
>  Possible unsafe locking scenario:
> 
>        CPU0                    CPU1
>        ----                    ----
>   lock(&desc->request_mutex);
>                                lock(&chip->reg_lock);
>                                lock(&desc->request_mutex);
>   lock(&chip->reg_lock);

Hi Russell

I failed to reproduce it on a Freescale system. Which made me take a
closer look at the above. This is a false positive.

In #1 we are requesting the GPIO interrupt. In #2 we are requesting
the chained interrupt from the mv88e6xxx global 1 interrupt handler.
So these are different desc->request_mutex. The Freescale VF610 GPIO
driver uses gpiochip_irqchip_add(), which creates a lock class for the
GPIO. The marvell gpio-mvebu driver does not create a lock class.  So
when i test on Freescale, lockdep can tell they are different mutex,
but on clearfog it cannot.

So i think the real fix is probably two fold, although just doing one
is sufficient:

1) Add lock classes to gpio-mvebu, by call irq_set_lockdep_class()
2) Add lock classes to chip.c global 1, by calling irq_set_lockdep_class()

There is probably more value in 1) since the mvebu gpio driver is much
more widely used than the mv88e6xxx driver.

     Andrew