Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1754160AbbGPBKD (ORCPT ); Wed, 15 Jul 2015 21:10:03 -0400 Received: from youngberry.canonical.com ([91.189.89.112]:55120 "EHLO youngberry.canonical.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752214AbbGPBJt (ORCPT ); Wed, 15 Jul 2015 21:09:49 -0400 From: Kamal Mostafa To: linux-kernel@vger.kernel.org, stable@vger.kernel.org, kernel-team@lists.ubuntu.com Cc: Nikolay Aleksandrov , "David S. Miller" , Kamal Mostafa Subject: [PATCH 3.19.y-ckt 003/251] bridge: fix br_stp_set_bridge_priority race conditions Date: Wed, 15 Jul 2015 18:05:24 -0700 Message-Id: <1437008972-9140-4-git-send-email-kamal@canonical.com> X-Mailer: git-send-email 1.9.1 In-Reply-To: <1437008972-9140-1-git-send-email-kamal@canonical.com> References: <1437008972-9140-1-git-send-email-kamal@canonical.com> X-Extended-Stable: 3.19 Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 2814 Lines: 79 3.19.8-ckt4 -stable review patch. If anyone has any objections, please let me know. ------------------ From: Nikolay Aleksandrov [ Upstream commit 2dab80a8b486f02222a69daca6859519e05781d9 ] After the ->set() spinlocks were removed br_stp_set_bridge_priority was left running without any protection when used via sysfs. It can race with port add/del and could result in use-after-free cases and corrupted lists. Tested by running port add/del in a loop with stp enabled while setting priority in a loop, crashes are easily reproducible. The spinlocks around sysfs ->set() were removed in commit: 14f98f258f19 ("bridge: range check STP parameters") There's also a race condition in the netlink priority support that is fixed by this change, but it was introduced recently and the fixes tag covers it, just in case it's needed the commit is: af615762e972 ("bridge: add ageing_time, stp_state, priority over netlink") Signed-off-by: Nikolay Aleksandrov Fixes: 14f98f258f19 ("bridge: range check STP parameters") Signed-off-by: David S. Miller Signed-off-by: Kamal Mostafa --- net/bridge/br_ioctl.c | 2 -- net/bridge/br_stp_if.c | 4 +++- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/net/bridge/br_ioctl.c b/net/bridge/br_ioctl.c index a9a4a1b..8d423bc 100644 --- a/net/bridge/br_ioctl.c +++ b/net/bridge/br_ioctl.c @@ -247,9 +247,7 @@ static int old_dev_ioctl(struct net_device *dev, struct ifreq *rq, int cmd) if (!ns_capable(dev_net(dev)->user_ns, CAP_NET_ADMIN)) return -EPERM; - spin_lock_bh(&br->lock); br_stp_set_bridge_priority(br, args[1]); - spin_unlock_bh(&br->lock); return 0; case BRCTL_SET_PORT_PRIORITY: diff --git a/net/bridge/br_stp_if.c b/net/bridge/br_stp_if.c index 4114687..7832d07 100644 --- a/net/bridge/br_stp_if.c +++ b/net/bridge/br_stp_if.c @@ -243,12 +243,13 @@ bool br_stp_recalculate_bridge_id(struct net_bridge *br) return true; } -/* called under bridge lock */ +/* Acquires and releases bridge lock */ void br_stp_set_bridge_priority(struct net_bridge *br, u16 newprio) { struct net_bridge_port *p; int wasroot; + spin_lock_bh(&br->lock); wasroot = br_is_root_bridge(br); list_for_each_entry(p, &br->port_list, list) { @@ -266,6 +267,7 @@ void br_stp_set_bridge_priority(struct net_bridge *br, u16 newprio) br_port_state_selection(br); if (br_is_root_bridge(br) && !wasroot) br_become_root_bridge(br); + spin_unlock_bh(&br->lock); } /* called under bridge lock */ -- 1.9.1 -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/