netdev - Re: [PATCH net-next] [v2] bonding: remove entries for master_ip and vlan

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: <20120321.223411.1471023630379810739.davem@davemloft.net>
Date:	Wed, 21 Mar 2012 22:34:11 -0400 (EDT)
From:	David Miller <davem@...emloft.net>
To:	andy@...yhouse.net
Cc:	netdev@...r.kernel.org, ralf.zeidler@....com
Subject: Re: [PATCH net-next] [v2] bonding: remove entries for master_ip
 and vlan_ip and query devices instead

From: Andy Gospodarek <andy@...yhouse.net>
Date: Wed, 21 Mar 2012 17:36:42 -0400

> As the Subject indicates this patch drops the master_ip and vlan_ip
> elements from the 'bonding' and 'vlan_entry' structs, respectively.
> This can be done because a device's address-list is now traversed to
> determine the optimal source IP address for ARP requests and for checks
> to see if the bonding device has a particular IP address.  This code
> could have all be contained inside the bonding driver, but it made more
> sense to me to EXPORT and call inet_confirm_addr since it did exactly
> what was needed.

I like this patch a lot but you have one little bug that needs to be
fixed:

> +	rcu_read_lock();
> +	in_dev = __in_dev_get_rcu(dev);
> +	rcu_read_unlock();
> +
> +	if (in_dev)
> +		addr = inet_confirm_addr(in_dev, dst, local, RT_SCOPE_HOST);

If you're going to do an RCU ref-less lookup of in_dev and then use
it, you have to include the "use" inside of the RCU protected section
as well.

Otherwise as soon as you rcu_read_unlock() the in_dev could be freed
up on you.

The only exception would be if you know that all callers of
bond_confirm_addr() ran in an RCU protected section, but I do not
think that is universally the case here.

If you think it might be the case that we are RCU protected in all of
these code paths already, you can remove the RCU locking altogether
from bond_confirm_addr() and run with lockdep enabled while exercising
all of the relevant code paths.
--
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