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  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:	Tue, 7 Jun 2011 18:25:23 -0700
From:	Jesse Gross <jesse@...ira.com>
To:	Jiri Bohac <jbohac@...e.cz>
Cc:	David Miller <davem@...emloft.net>, kaber@...sh.net,
	netdev@...r.kernel.org, pedro.netdev@...devamos.com
Subject: Re: [PATCH 1/2] vlan: only create special VLAN 0 once

On Tue, Jun 7, 2011 at 9:18 AM, Jiri Bohac <jbohac@...e.cz> wrote:
> Hi David,
>
> On Sun, Jun 05, 2011 at 02:28:23PM -0700, David Miller wrote:
>> 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.
>
> Really? In that case, we have a much bigger problem: the vlan
> code allows registering a new vlan on an interface that is down.
> And it only registers the VID with ndo_vlan_rx_add_vid() in
> register_vlan_dev() during the registration of the new vlan
> interface -- it never re-registers the VIDs on a NETDEV_UP.

No, it's not true.  All drivers store the registered vlan filters in
some way so that they can restore them when the device is reset.  This
is currently done in one of two ways: storing a bitmap or iterating
over the devices currently registered in a group.

The vlan code is moving away from directly accessing groups and no new
drivers do this.  In fact, once all drivers are converted over groups
will not even be registered on devices.  This is because otherwise
there is quite a bit of vlan code in each driver, which leads to
inconsistent behavior and bugs.

Really, all a driver needs to know is whether it should add a given
vlan to its table, not what the upper layers plan to do with it.  So
when ndo_vlan_rx_add_vid() is called it should add it to its CAM table
and store it if it is needed to restore behavior after a reset, just
as is done with all other configuration state.
--
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

Powered by blists - more mailing lists