netdev - Re: [PATCH v2] net: stmicro: stmmac: do not grab a mutex in irq off section

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: <20140321.133537.1329677087354635241.davem@davemloft.net>
Date:	Fri, 21 Mar 2014 13:35:37 -0400 (EDT)
From:	David Miller <davem@...emloft.net>
To:	bigeasy@...utronix.de
Cc:	netdev@...r.kernel.org, peppe.cavallaro@...com
Subject: Re: [PATCH v2] net: stmicro: stmmac: do not grab a mutex in irq
 off section

From: Sebastian Andrzej Siewior <bigeasy@...utronix.de>
Date: Thu, 20 Mar 2014 17:06:05 +0100

> |libphy: stmmac-0:01 - Link is Up - 1000/Full
> |BUG: sleeping function called from invalid context at kernel/locking/mutex.c:616
> |in_atomic(): 1, irqs_disabled(): 128, pid: 52, name: kworker/1:1
> |4 locks held by kworker/1:1/52:
> |
> | #0:  (events_power_efficient){.+.+..}, at: [<800383a4>] process_one_work+0x128/0x450
> | #1:  ((&(&dev->state_queue)->work)){+.+...}, at: [<800383a4>] process_one_work+0x128/0x450
> | #2:  (&dev->lock#2){+.+...}, at: [<8025d9a4>] phy_state_machine+0x1c/0x39c
> | #3:  (&(&priv->lock)->rlock){+.....}, at: [<802661b4>] stmmac_adjust_link+0x34/0x228
> |CPU: 1 PID: 52 Comm: kworker/1:1 Not tainted 3.13.0-dirty #24
> |Workqueue: events_power_efficient phy_state_machine
> |[<8034eff4>] (mutex_lock_nested+0x28/0x434) from [<8025f35c>] (mdiobus_read+0x44/0x70)
> |[<8025f35c>] (mdiobus_read+0x44/0x70) from [<8025ea40>] (genphy_update_link+0x18/0x50)
> |[<8025ea40>] (genphy_update_link+0x18/0x50) from [<8025ea84>] (genphy_read_status+0xc/0x180)
> |[<8025ea84>] (genphy_read_status+0xc/0x180) from [<8025cbd0>] (phy_init_eee+0x4c/0x2ac)
> |[<8025cbd0>] (phy_init_eee+0x4c/0x2ac) from [<8026516c>] (stmmac_eee_init+0x40/0x104)
> |[<8026516c>] (stmmac_eee_init+0x40/0x104) from [<80266298>] (stmmac_adjust_link+0x118/0x228)
> |[<80266298>] (stmmac_adjust_link+0x118/0x228) from [<8025dbc4>] (phy_state_machine+0x23c/0x39c)
> |[<8025dbc4>] (phy_state_machine+0x23c/0x39c) from [<80038420>] (process_one_work+0x1a4/0x450)
> |stmmac: Energy-Efficient Ethernet initialized
> 
> stmmac_eee_init() is called in other places without this spin lock.
> In stmmac_adjust_link() I just moved the unlock prior phy_print_status()
> as it does not look like this lock protects stmmac_eee_init() (except it
> ensures that does not get called twice).
> For the resume case I moved stmmac_eee_init() out of stmmac_hw_setup()
> and added it to both callers after the lock is dropped.
> DaveM pointed out a problem with two concurent calls to
> stmmac_eee_init() which would be bad. For that there is this
> eee_init_lock mutex now.
> 
> Cc: peppe.cavallaro@...com
> Signed-off-by: Sebastian Andrzej Siewior <bigeasy@...utronix.de>

We can't let the state machine re-enter stmmac_adjust_link() while we are invoking
stmmac_eee_init().

It calls phy_init_eee() which expects a stable set of PHY state, including the
duplex and connection settings, which can change if we don't hold a lock across
all of this.
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html