netdev - Re: [PATCH net-next] 8021q: validate SAN MAC address

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: <20121113.165300.1149843648625252626.davem@davemloft.net>
Date:	Tue, 13 Nov 2012 16:53:00 -0500 (EST)
From:	David Miller <davem@...emloft.net>
To:	mchan@...adcom.com
Cc:	netdev@...r.kernel.org
Subject: Re: [PATCH net-next] 8021q: validate SAN MAC address

From: "Michael Chan" <mchan@...adcom.com>
Date: Fri, 9 Nov 2012 14:54:04 -0800

> Some bnx2x multi-function devices are configured for FCoE only in
> a PCI function and only has a SAN MAC address.  The current code
> will fail ->ndo_validate_addr() when a VLAN device is brought up
> over such a device.
> 
> Check the real device's dev_addrs[] for a valid SAN MAC address
> when validating the MAC address if the regular MAC address is not
> valid.
> 
> Signed-off-by: Michael Chan <mchan@...adcom.com>

I've read this thread and I still don't think this is a nice
change at all.

It's beyond hackish to allow configuring a VLAN with a completely
invalid MAC address just because the underlying real device just so
happens to have a storage MAC address.

I can only assume that, in this case, this code block is triggered
in vlan_dev_open():

	if (!ether_addr_equal(dev->dev_addr, real_dev->dev_addr)) {
		err = dev_uc_add(real_dev, dev->dev_addr);
		if (err < 0)
			goto out;
	}

And if so we're adding an invalid ethernet address to the underlying
device's UC list.  Yet another terrible aspect of this situation.

The more I look into this situation the more it looks like piling crap
on top of crap on top of crap.  It's a long depency chain of special
cases just to get this weird setup working.

I'm sorry, I'm not making the situation even worse by applying this
patch.  If you want to have real bypasses to allow VLAN VIDs to
be configured on SAN devices, create a real interface for it, rather
than hack to shreds the stuff we have already.

Thanks.
--
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