From 049b35560b2cc7facc246f78134bbd68caa37e34 Mon Sep 17 00:00:00 2001 From: Fabio Olive Leite <fleite@redhat.com> Date: Thu, 3 Jul 2008 17:11:39 -0400 Subject: [PATCH] [net] Fixing bonding rtnl_lock screwups Message-id: <20080605221454.GB5901@sleipnir.ol> O-Subject: [5.3 NET BONDING BZ450219] Fixing bonding rtnl_lock screwups Bugzilla: 450219 Hi all, Between 5.1 and 5.2 (especially after -62.el5) some changes to the bonding driver left unbalanced rtnl_lock and rtnl_unlock calls in the sysfs interface code. Specifically, it is possible for bonding_store_bonds() to return without unlocking rtnl and for bonding_store_slaves() to do an unlock without going through the codepath that locks. The patch below is being tested at a customer right now. They were seeing some bonding errors followed by several RTNL assertion errors. The fix is partly based on upstream commit c4ebc66a1a8e3576322a9f47f0d06ec3c96a08d7. I believe we should do a 5.2.Z as well, as it was broken on 5.2. Please let me know if the fix below is suboptimal. Acked-by: Neil Horman <nhorman@redhat.com> Acked-by: Rik van Riel <riel@redhat.com> Acked-by: Arnaldo Carvalho de Melo <acme@redhat.com> Acked-by: Andy Gospodarek <gospo@redhat.com> Acked-by: "David S. Miller" <davem@redhat.com> --- drivers/net/bonding/bond_sysfs.c | 20 ++++++++++---------- 1 files changed, 10 insertions(+), 10 deletions(-) diff --git a/drivers/net/bonding/bond_sysfs.c b/drivers/net/bonding/bond_sysfs.c index dbd9b96..b1f861f 100644 --- a/drivers/net/bonding/bond_sysfs.c +++ b/drivers/net/bonding/bond_sysfs.c @@ -147,29 +147,29 @@ static ssize_t bonding_store_bonds(struct class *cls, const char *buffer, size_t ": Unable remove bond %s due to open references.\n", ifname); res = -EPERM; - goto out; + goto out_unlock; } printk(KERN_INFO DRV_NAME ": %s is being deleted...\n", bond->dev->name); bond_destroy(bond); - up_write(&bonding_rwsem); - rtnl_unlock(); - goto out; + goto out_unlock; } printk(KERN_ERR DRV_NAME ": unable to delete non-existent bond %s\n", ifname); res = -ENODEV; - up_write(&bonding_rwsem); - rtnl_unlock(); - goto out; + goto out_unlock; } err_no_cmd: printk(KERN_ERR DRV_NAME ": no command found in bonding_masters. Use +ifname or -ifname.\n"); - res = -EPERM; + return -EPERM; + +out_unlock: + up_write(&bonding_rwsem); + rtnl_unlock(); /* Always return either count or an error. If you return 0, you'll * get called forever, which is bad. @@ -254,8 +254,8 @@ static ssize_t bonding_store_slaves(struct class_device *cd, const char *buffer, printk(KERN_ERR DRV_NAME ": %s: Unable to update slaves because interface is down.\n", bond->dev->name); - ret = -EPERM; - goto out; + /* early return before rtnl_lock() */ + return -EPERM; } /* Note: We can't hold bond->lock here, as bond_create grabs it. */ -- 1.5.5.1