From patchwork Fri Jan 15 18:03:54 2016 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Nikolay Aleksandrov X-Patchwork-Id: 568252 X-Patchwork-Delegate: davem@davemloft.net Return-Path: X-Original-To: patchwork-incoming@ozlabs.org Delivered-To: patchwork-incoming@ozlabs.org Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by ozlabs.org (Postfix) with ESMTP id 84CF5140BED for ; Sat, 16 Jan 2016 05:04:09 +1100 (AEDT) Authentication-Results: ozlabs.org; dkim=pass (2048-bit key; unprotected) header.d=blackwall-org.20150623.gappssmtp.com header.i=@blackwall-org.20150623.gappssmtp.com header.b=CNHmZLl+; dkim-atps=neutral Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1753577AbcAOSED (ORCPT ); Fri, 15 Jan 2016 13:04:03 -0500 Received: from mail-wm0-f67.google.com ([74.125.82.67]:33044 "EHLO mail-wm0-f67.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751018AbcAOSEA (ORCPT ); Fri, 15 Jan 2016 13:04:00 -0500 Received: by mail-wm0-f67.google.com with SMTP id u188so4057892wmu.0 for ; Fri, 15 Jan 2016 10:04:00 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=blackwall-org.20150623.gappssmtp.com; s=20150623; h=from:to:cc:subject:date:message-id; bh=Ix3/pxrE9k743x2gIUEMjcbppPUpQ27X4wEgkFVaYPA=; b=CNHmZLl+K9+dWXauiApd0DD7JkespKyLK4o3zAAqfQE8ueZgOb5pq2LkyfytWH7mAF BRAScpygCcUWlWgq+V6FGbBlS0oF1qcxlfgPW3g1jbo68MbWwm/+bbz/wRJ3IcO8k7kp orB6xaans+tXUxXXqzjyXeIpLADJhqQjUmRTReBgK2d+Cx9mvlCQGcKteKUwigTdW902 zoudHKsCki5IPkOzT+rNNDflpef1UDCffeIZ/0FPtCL8C0K4zQrSlRT7kJSVjmTj3abV bJGJgqbPMZh9bXLOPBCH9W1HT1FVRGN4Gj3vj/2RdN7/Y/z1aHiiQ4p7eHxhNAoNj1IA gV6g== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20130820; h=x-gm-message-state:from:to:cc:subject:date:message-id; bh=Ix3/pxrE9k743x2gIUEMjcbppPUpQ27X4wEgkFVaYPA=; b=IQZEZUM39/CT8GNaieITSF6/lzeWG3D7WA+8aKgeWE4u4gyfU0vK5MlT/hkIw4Z7JB jmxhifg4A6DCBiUA91/faZJRKaIkvXN9gPLMW1xsem7LsTp83DSgitRvfzVm6QBY3b8E Efa9I2LVcvoDU1qqVjpTcT3/qXtRMnUGQ0BUsd2Sa2//uYOTb9hQxyWQ3qmUcPvQ7/eK TVXK2q3oIkxqpLLh1r7PSYrlhtKfCXWTZcIcVpN+7cnvhPUo5GCX8L66ax2D1qOyF0s5 MTX0rAw6IEM1M2NQqn7c5smvQHLeomNrTZshY45G9HZ1GiyQJD7Ys3N0Ca+ocjxyO/ex EMdQ== X-Gm-Message-State: AG10YOTVobVQnOb8hqgHnHAoDJqBknTy43tynW7J62sC21tXxLFpGZVx5JqYOxYWTHl0SQ== X-Received: by 10.28.89.69 with SMTP id n66mr5048569wmb.63.1452881039550; Fri, 15 Jan 2016 10:03:59 -0800 (PST) Received: from debil.localdomain (ip4-62-4-104-109.cust.nbox.cz. [62.4.104.109]) by smtp.gmail.com with ESMTPSA id c185sm3556191wma.5.2016.01.15.10.03.58 (version=TLS1_2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128/128); Fri, 15 Jan 2016 10:03:58 -0800 (PST) From: Nikolay Aleksandrov To: netdev@vger.kernel.org Cc: Nikolay Aleksandrov , Vlad Yasevich , Stephen Hemminger , Bridge list , Andy Gospodarek , Roopa Prabhu Subject: [PATCH net] bridge: fix lockdep addr_list_lock false positive splat Date: Fri, 15 Jan 2016 19:03:54 +0100 Message-Id: <1452881034-19128-1-git-send-email-razor@blackwall.org> X-Mailer: git-send-email 2.4.3 Sender: netdev-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: netdev@vger.kernel.org From: Nikolay Aleksandrov After promisc mode management was introduced a bridge device could do dev_set_promiscuity from its ndo_change_rx_flags() callback which in turn can be called after the bridge's addr_list_lock has been taken (e.g. by dev_uc_add). This causes a false positive lockdep splat because the port interfaces' addr_list_lock is taken when br_manage_promisc() runs after the bridge's addr list lock was already taken. To remove the false positive introduce a custom bridge addr_list_lock class and set it on bridge init. A simple way to reproduce this is with the following: $ brctl addbr br0 $ ip l add l br0 br0.100 type vlan id 100 $ ip l set br0 up $ ip l set br0.100 up $ echo 1 > /sys/class/net/br0/bridge/vlan_filtering $ brctl addif br0 eth0 Splat: [ 43.684325] ============================================= [ 43.684485] [ INFO: possible recursive locking detected ] [ 43.684636] 4.4.0-rc8+ #54 Not tainted [ 43.684755] --------------------------------------------- [ 43.684906] brctl/1187 is trying to acquire lock: [ 43.685047] (_xmit_ETHER){+.....}, at: [] dev_set_rx_mode+0x1e/0x40 [ 43.685460] but task is already holding lock: [ 43.685618] (_xmit_ETHER){+.....}, at: [] dev_uc_add+0x27/0x80 [ 43.686015] other info that might help us debug this: [ 43.686316] Possible unsafe locking scenario: [ 43.686743] CPU0 [ 43.686967] ---- [ 43.687197] lock(_xmit_ETHER); [ 43.687544] lock(_xmit_ETHER); [ 43.687886] *** DEADLOCK *** [ 43.688438] May be due to missing lock nesting notation [ 43.688882] 2 locks held by brctl/1187: [ 43.689134] #0: (rtnl_mutex){+.+.+.}, at: [] rtnl_lock+0x17/0x20 [ 43.689852] #1: (_xmit_ETHER){+.....}, at: [] dev_uc_add+0x27/0x80 [ 43.690575] stack backtrace: [ 43.690970] CPU: 0 PID: 1187 Comm: brctl Not tainted 4.4.0-rc8+ #54 [ 43.691270] Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS 1.8.1-20150318_183358- 04/01/2014 [ 43.691770] ffffffff826a25c0 ffff8800369fb8e0 ffffffff81360ceb ffffffff826a25c0 [ 43.692425] ffff8800369fb9b8 ffffffff810d0466 ffff8800369fb968 ffffffff81537139 [ 43.693071] ffff88003a08c880 0000000000000000 00000000ffffffff 0000000002080020 [ 43.693709] Call Trace: [ 43.693931] [] dump_stack+0x4b/0x70 [ 43.694199] [] __lock_acquire+0x1e46/0x1e90 [ 43.694483] [] ? netlink_broadcast_filtered+0x139/0x3e0 [ 43.694789] [] ? nlmsg_notify+0x5a/0xc0 [ 43.695064] [] lock_acquire+0xe5/0x1f0 [ 43.695340] [] ? dev_set_rx_mode+0x1e/0x40 [ 43.695623] [] _raw_spin_lock_bh+0x45/0x80 [ 43.695901] [] ? dev_set_rx_mode+0x1e/0x40 [ 43.696180] [] dev_set_rx_mode+0x1e/0x40 [ 43.696460] [] dev_set_promiscuity+0x3c/0x50 [ 43.696750] [] br_port_set_promisc+0x25/0x50 [bridge] [ 43.697052] [] br_manage_promisc+0x8a/0xe0 [bridge] [ 43.697348] [] br_dev_change_rx_flags+0x1e/0x20 [bridge] [ 43.697655] [] __dev_set_promiscuity+0x132/0x1f0 [ 43.697943] [] __dev_set_rx_mode+0x82/0x90 [ 43.698223] [] dev_uc_add+0x5e/0x80 [ 43.698498] [] vlan_device_event+0x542/0x650 [8021q] [ 43.698798] [] notifier_call_chain+0x5d/0x80 [ 43.699083] [] raw_notifier_call_chain+0x16/0x20 [ 43.699374] [] call_netdevice_notifiers_info+0x6e/0x80 [ 43.699678] [] call_netdevice_notifiers+0x16/0x20 [ 43.699973] [] br_add_if+0x47e/0x4c0 [bridge] [ 43.700259] [] add_del_if+0x6e/0x80 [bridge] [ 43.700548] [] br_dev_ioctl+0xaf/0xc0 [bridge] [ 43.700836] [] dev_ifsioc+0x30c/0x3c0 [ 43.701106] [] dev_ioctl+0xf9/0x6f0 [ 43.701379] [] ? mntput_no_expire+0x5/0x450 [ 43.701665] [] ? mntput_no_expire+0xae/0x450 [ 43.701947] [] sock_do_ioctl+0x42/0x50 [ 43.702219] [] sock_ioctl+0x1e5/0x290 [ 43.702500] [] do_vfs_ioctl+0x2cb/0x5c0 [ 43.702771] [] SyS_ioctl+0x79/0x90 [ 43.703033] [] entry_SYSCALL_64_fastpath+0x16/0x7a CC: Vlad Yasevich CC: Stephen Hemminger CC: Bridge list CC: Andy Gospodarek CC: Roopa Prabhu Fixes: 2796d0c648c9 ("bridge: Automatically manage port promiscuous mode.") Reported-by: Andy Gospodarek Signed-off-by: Nikolay Aleksandrov --- Note: Maybe we should add different addr_list_lock class names, xmit_ETHER is used both for the queue lock class and addr list. net/bridge/br_device.c | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/net/bridge/br_device.c b/net/bridge/br_device.c index 5e88d3e17546..2c8095a5d824 100644 --- a/net/bridge/br_device.c +++ b/net/bridge/br_device.c @@ -28,6 +28,8 @@ const struct nf_br_ops __rcu *nf_br_ops __read_mostly; EXPORT_SYMBOL_GPL(nf_br_ops); +static struct lock_class_key bridge_netdev_addr_lock_key; + /* net device transmit always called with BH disabled */ netdev_tx_t br_dev_xmit(struct sk_buff *skb, struct net_device *dev) { @@ -87,6 +89,11 @@ out: return NETDEV_TX_OK; } +static void br_set_lockdep_class(struct net_device *dev) +{ + lockdep_set_class(&dev->addr_list_lock, &bridge_netdev_addr_lock_key); +} + static int br_dev_init(struct net_device *dev) { struct net_bridge *br = netdev_priv(dev); @@ -99,6 +106,7 @@ static int br_dev_init(struct net_device *dev) err = br_vlan_init(br); if (err) free_percpu(br->stats); + br_set_lockdep_class(dev); return err; }