netdev - Re: [PATCH 1/2] vlan: only create special VLAN 0 once

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]

Date:	Sun, 05 Jun 2011 14:28:23 -0700 (PDT)
From:	David Miller <davem@...emloft.net>
To:	jbohac@...e.cz
Cc:	kaber@...sh.net, netdev@...r.kernel.org,
	pedro.netdev@...devamos.com
Subject: Re: [PATCH 1/2] vlan: only create special VLAN 0 once

From: Jiri Bohac <jbohac@...e.cz>
Date: Fri, 3 Jun 2011 22:07:38 +0200

> Commit ad1afb00 registers a VLAN with vid == 0 for every device to handle
> 802.1p frames.  This is currently done on every NETDEV_UP event and the special
> vlan is never unregistered.  This may have strange effects on drivers
> implementning ndo_vlan_rx_add_vid(). E.g. bonding will allocate a linked-list
> element each time, causing a memory leak.
> 
> Only register the special VLAN once on NETDEV_REGISTER.
> 
> Signed-off-by: Jiri Bohac <jbohac@...e.cz>

I recognize the problem, but this solution isn't all that good.

I am pretty sure that the hardware device driver methods that
implement ndo_vlan_rx_add_vid() assume that the device is up.
Because most drivers completely reset the chip when the
interface is brought up and this will likely clear out the
VLAN ID tables in the chip.

Second, now even devices which don't ever get brought up will
have the VLAN ID 0 thing allocated.

Probably the thing to do is to remove the VLAN ID 0 entry on
NETDEV_DOWN.

Something like:

diff --git a/net/8021q/vlan.c b/net/8021q/vlan.c
index c7a581a..135019d 100644
--- a/net/8021q/vlan.c
+++ b/net/8021q/vlan.c
@@ -379,6 +379,14 @@ static int vlan_device_event(struct notifier_block *unused, unsigned long event,
 		dev->netdev_ops->ndo_vlan_rx_add_vid(dev, 0);
 	}
 
+	if ((event == NETDEV_DOWN) &&
+	    (dev->features & NETIF_F_HW_VLAN_FILTER) &&
+	    dev->netdev_ops->ndo_vlan_rx_kill_vid) {
+		pr_info("8021q: removing VLAN 0 from HW filter on device %s\n",
+			dev->name);
+		dev->netdev_ops->ndo_vlan_rx_kill_vid(dev, 0);
+	}
+
 	grp = rtnl_dereference(dev->vlgrp);
 	if (!grp)
 		goto out;
--
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