netdev - Re: [PATCH 0/3] bonding: 3 fixes for 2.6.24

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] [thread-next>] [day] [month] [year] [list]

Message-ID: <18609.1200160598@death>
Date:	Sat, 12 Jan 2008 09:56:38 -0800
From:	Jay Vosburgh <fubar@...ibm.com>
To:	Krzysztof Oledzki <olel@....pl>
cc:	Andy Gospodarek <andy@...yhouse.net>, netdev@...r.kernel.org,
	Jeff Garzik <jgarzik@...ox.com>,
	David Miller <davem@...emloft.net>,
	Herbert Xu <herbert@...dor.apana.org.au>
Subject: Re: [PATCH 0/3] bonding: 3 fixes for 2.6.24 

Krzysztof Oledzki <olel@....pl> wrote:
[...]
>Exactly. All I need to do is to reboot my server, I have 100% probability
>to get the warning.

	I wish it were that easy for me; I'm not sure what magic thing
you've got on your server or network that I don't, but I haven't been
able to make this lockdep warning happen at all.

>Right. So, what is the final patch? I would like to test it if that's
>possible. ;)

	Can you test the following and let me know if it triggers the
warning?  I believe this is the minimum locking needed, and based on
input from Herbert, we shouldn't need to hold the lock at _bh.  If this
one works, and nobody sees any other issues with it, then it's the final
patch for this lockdep problem.  I'll add some deep, meaningful comments
to explain the locking a bit (i.e., we're called with rtnl for the
allmulti and promisc cases, so we're ok there without additional locks,
but the later code could be called from anywhere, so it needs locks to
prevent the slave list from changing, but the mc_lists themselves are
covered by the netif_tx_lock that all callers will hold), but this would
be the actual code change.

diff --git a/drivers/net/bonding/bond_main.c b/drivers/net/bonding/bond_main.c
index 77d004d..6906dbc 100644
--- a/drivers/net/bonding/bond_main.c
+++ b/drivers/net/bonding/bond_main.c
@@ -3937,8 +3937,6 @@ static void bond_set_multicast_list(struct net_device *bond_dev)
 	struct bonding *bond = bond_dev->priv;
 	struct dev_mc_list *dmi;
 
-	write_lock_bh(&bond->lock);
-
 	/*
 	 * Do promisc before checking multicast_mode
 	 */
@@ -3959,6 +3957,8 @@ static void bond_set_multicast_list(struct net_device *bond_dev)
 		bond_set_allmulti(bond, -1);
 	}
 
+	read_lock(&bond->lock);
+
 	bond->flags = bond_dev->flags;
 
 	/* looking for addresses to add to slaves' mc list */
@@ -3979,7 +3979,7 @@ static void bond_set_multicast_list(struct net_device *bond_dev)
 	bond_mc_list_destroy(bond);
 	bond_mc_list_copy(bond_dev->mc_list, bond, GFP_ATOMIC);
 
-	write_unlock_bh(&bond->lock);
+	read_unlock(&bond->lock);
 }
 
 /*


	-J

---
	-Jay Vosburgh, IBM Linux Technology Center, fubar@...ibm.com
--
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