netdev - Re: [GIT] Networking

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: <87zjwda9bl.fsf@nemi.mork.no>
Date:	Thu, 02 May 2013 13:51:42 +0200
From:	Bjørn Mork <bjorn@...k.no>
To:	Ben Hutchings <bhutchings@...arflare.com>
Cc:	David Miller <davem@...emloft.net>, <kaber@...sh.net>,
	<torvalds@...ux-foundation.org>, <hayeswang@...ltek.com>,
	<akpm@...ux-foundation.org>, <netdev@...r.kernel.org>,
	<linux-kernel@...r.kernel.org>
Subject: Re: [GIT] Networking

Ben Hutchings <bhutchings@...arflare.com> writes:
> On Thu, 2013-05-02 at 05:17 -0400, David Miller wrote:
>> From: Bjørn Mork <bjorn@...k.no>
>> Date: Thu, 02 May 2013 11:06:42 +0200
>> 
>> > Adding the new netdev features will make it go from 1 to 2:
>> 
>> We already had more than 31 feature bits before Patrick's
>> changes, and I'm pretty sure this was the case when we added
>> that ethtool API.
>
> It wasn't, but this should be OK.  Userland is supposed to query the
> number of features using ETHTOOL_GSSET_INFO and then work out the number
> of words/blocks using FEATURE_BITS_TO_BLOCKS().  


Looking at
http://cgit.freedesktop.org/NetworkManager/NetworkManager/tree/src/platform/nm-linux-platform.c#n1025
there seems to be a couple of bugs in this area. This is certainly
abusing the exported API, but it does mean that NM breaks if you ever
move NETIF_F_VLAN_CHALLENGED (like the 802.1ad patch did):

----
#define NETIF_F_VLAN_CHALLENGED (1 << 10)

static gboolean
link_supports_vlans (NMPlatform *platform, int ifindex)
{
	auto_nl_object struct rtnl_link *rtnllink = link_get (platform, ifindex);
	const char *name = nm_platform_link_get_name (ifindex);
	struct {
		struct ethtool_gfeatures features;
		struct ethtool_get_features_block features_block;
	} edata = { .features = { .cmd = ETHTOOL_GFEATURES, .size = 1 } };

	/* Only ARPHRD_ETHER links can possibly support VLANs. */
	if (!rtnllink || rtnl_link_get_arptype (rtnllink) != ARPHRD_ETHER)
		return FALSE;

	if (!name || !ethtool_get (name, &edata))
		return FALSE;

	return !(edata.features.features[0].active & NETIF_F_VLAN_CHALLENGED);
}
----


Not that I see how this particular bug matters unless you need VLAN
support in NM.  But there could be similar issues around.  I guess
avoiding unnecessary renumbering of the NETIF_F bits can save us some
trouble.  Although you can certainly argue that those bits never were
intended to be part of the API, and that using them like this is a user
application bug.



Bjørn
--
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