On Saturday 01 December 2012 18:29:51 Simon Wunderlich wrote:
If rtnl_trylock() is used to prevent circular dependencies, rtnl_unlock() may destroy this attempt because it first unlocks rtnl_mutex but may lock it again later. The callgraph roughly looks like:
rtnl_unlock() netdev_run_todo() __rtnl_unlock() netdev_wait_allrefs() rtnl_lock() ... __rtnl_unlock()
There are two users which have possible deadlocks and are fixed in this patch: batman-adv and bridge. Their problematic access pattern is the same:
notifier_call handler:
- holds rtnl lock when called
- calls sysfs code at some point (acquiring sysfs locks)
sysfs code:
- holds sysfs lock when called
- calls rtnl_trylock() and rtnl_unlock(), rtnl_unlock() calls rtnl_lock implicitly
Fix this by exporting __rtnl_unlock() to just unlock the mutex without implicitly locking again.
Reported-by: Sven Eckelmann sven@narfation.org Signed-off-by: Simon Wunderlich siwu@hrz.tu-chemnitz.de
Of course, an alternative would be to not lock again after unlocking within rtnl_unlock(), or put the todo handling into the locked section. I'm not familiar enough with this code to know what would be best.
There are others using rtnl_trylock(), but I'm not sure if they are affected.
At least they look like they have a problem in a parallel user scenario involving another lock and locking order (most of them s_active or a device lock). So just to list the places and poke some other users. They can better decide for themself because they know the code.
drivers/infiniband/ulp/ipoib/ipoib_cm.c: if (!rtnl_trylock()) drivers/infiniband/ulp/ipoib/ipoib_vlan.c: if (!rtnl_trylock()) drivers/infiniband/ulp/ipoib/ipoib_vlan.c: if (!rtnl_trylock()) drivers/net/bonding/bond_alb.c: if (!rtnl_trylock()) { drivers/net/bonding/bond_main.c: if (!rtnl_trylock()) { drivers/net/bonding/bond_main.c: if (!rtnl_trylock()) { drivers/net/bonding/bond_main.c: if (!rtnl_trylock()) { drivers/net/bonding/bond_main.c: if (!rtnl_trylock()) { drivers/net/bonding/bond_sysfs.c: if (!rtnl_trylock()) drivers/net/bonding/bond_sysfs.c: if (!rtnl_trylock()) drivers/net/bonding/bond_sysfs.c: if (!rtnl_trylock()) drivers/net/bonding/bond_sysfs.c: if (!rtnl_trylock()) drivers/net/bonding/bond_sysfs.c: if (!rtnl_trylock()) drivers/net/bonding/bond_sysfs.c: if (!rtnl_trylock()) drivers/net/ethernet/chelsio/cxgb3/cxgb3_main.c: if (!rtnl_trylock()) /* synchronize with ifdown */ drivers/s390/net/qeth_l2_main.c: if (rtnl_trylock()) { drivers/s390/net/qeth_l3_main.c: if (rtnl_trylock()) { net/bridge/br_sysfs_br.c: if (!rtnl_trylock()) net/bridge/br_sysfs_if.c: if (!rtnl_trylock()) net/core/net-sysfs.c: if (!rtnl_trylock()) net/core/net-sysfs.c: if (!rtnl_trylock()) net/core/net-sysfs.c: if (!rtnl_trylock()) net/core/net-sysfs.c: if (!rtnl_trylock()) net/core/net-sysfs.c: if (!rtnl_trylock()) net/ipv4/devinet.c: if (!rtnl_trylock()) { net/ipv6/addrconf.c: if (!rtnl_trylock()) net/ipv6/addrconf.c: if (!rtnl_trylock())
Kind regards, Sven