netdev - Re: [PATCH net-next v10 2/4] net: Introduce generic failover module

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 for Android: free password hash cracker in your pocket

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:   Mon, 7 May 2018 17:24:27 -0700
From:   "Samudrala, Sridhar" <sridhar.samudrala@...el.com>
To:     Stephen Hemminger <stephen@...workplumber.org>
Cc:     mst@...hat.com, davem@...emloft.net, netdev@...r.kernel.org,
        virtualization@...ts.linux-foundation.org,
        virtio-dev@...ts.oasis-open.org, jesse.brandeburg@...el.com,
        alexander.h.duyck@...el.com, kubakici@...pl, jasowang@...hat.com,
        loseweigh@...il.com, jiri@...nulli.us, aaron.f.brown@...el.com
Subject: Re: [PATCH net-next v10 2/4] net: Introduce generic failover module



On 5/7/2018 4:53 PM, Stephen Hemminger wrote:
> On Mon,  7 May 2018 15:10:44 -0700
> Sridhar Samudrala <sridhar.samudrala@...el.com> wrote:
>
>> +static struct net_device *net_failover_get_bymac(u8 *mac,
>> +						 struct net_failover_ops **ops)
>> +{
>> +	struct net_device *failover_dev;
>> +	struct net_failover *failover;
>> +
>> +	spin_lock(&net_failover_lock);
>> +	list_for_each_entry(failover, &net_failover_list, list) {
>> +		failover_dev = rtnl_dereference(failover->failover_dev);
>> +		if (ether_addr_equal(failover_dev->perm_addr, mac)) {
>> +			*ops = rtnl_dereference(failover->ops);
>> +			spin_unlock(&net_failover_lock);
>> +			return failover_dev;
>> +		}
>> +	}
>> +	spin_unlock(&net_failover_lock);
>> +	return NULL;
>> +}
> This is broken if non-ethernet devices such as Infiniband are present.

There is check to make sure that a slave and failover devices are of the same type in
net_failover_slave_register()

	failover_dev = net_failover_get_bymac(slave_dev->perm_addr, &nfo_ops);
         if (!failover_dev)
                 goto done;

         if (failover_dev->type != slave_dev->type)
                 goto done;

Do you think this is not good enough? I had an explicit check for ARPHRD_ETHER in
earlier patchsets, but removed it based on Jiri's comment.