On Thu, 17 Jan 2008 16:25:02 -0800
Jay Vosburgh <[EMAIL PROTECTED]> wrote:
> Fix the handling of rtnl and the bonding_rwsem to always be acquired
> in a consistent order (rtnl, then bonding_rwsem).
>
> The existing code sometimes acquired them in this order, and sometimes
> in the opposite order, which opens a window for deadlock between ifenslave
> and sysfs.
>
> ...
>
> int bond_create(char *name, struct bond_params *params, struct bonding
> **newbond)
> {
> struct net_device *bond_dev;
> + struct bonding *bond, *nxt;
> int res;
>
> rtnl_lock();
> + down_write(&bonding_rwsem);
> +
> + /* Check to see if the bond already exists. */
> + list_for_each_entry_safe(bond, nxt, &bond_dev_list, bond_list)
this could (should) use list_for_each_entry().
> + if (strnicmp(bond->dev->name, name, IFNAMSIZ) == 0) {
> + printk(KERN_ERR DRV_NAME
> + ": cannot add bond %s; it already exists\n",
> + name);
> + res = -EPERM;
> + goto out_rtnl;
> + }
> +
> bond_dev = alloc_netdev(sizeof(struct bonding), name ? name : "",
> ether_setup);
> if (!bond_dev) {
> @@ -4915,10 +4928,12 @@ int bond_create(char *name, struct bond_params
> *params, struct bonding **newbond
>
> netif_carrier_off(bond_dev);
>
> + up_write(&bonding_rwsem);
> rtnl_unlock(); /* allows sysfs registration of net device */
> res = bond_create_sysfs_entry(bond_dev->priv);
> if (res < 0) {
> rtnl_lock();
> + down_write(&bonding_rwsem);
> goto out_bond;
> }
>
> @@ -4929,6 +4944,7 @@ out_bond:
> out_netdev:
> free_netdev(bond_dev);
> out_rtnl:
> + up_write(&bonding_rwsem);
> rtnl_unlock();
> return res;
> }
> @@ -4949,6 +4965,9 @@ static int __init bonding_init(void)
> #ifdef CONFIG_PROC_FS
> bond_create_proc_dir();
> #endif
> +
> + init_rwsem(&bonding_rwsem);
It would be better to initialise this at compile time with DECLARE_RWSEM().
But neither of those things need to be done for 2.6.24.
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
