netdev - Re: [PATCH] bonding: 802.3ad: make aggregator

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: <21312.1392412579@death.nxdomain>
Date:	Fri, 14 Feb 2014 13:16:19 -0800
From:	Jay Vosburgh <fubar@...ibm.com>
To:	Jiri Bohac <jbohac@...e.cz>
cc:	Flavio Leitner <fbl@...hat.com>,
	Veaceslav Falico <vfalico@...hat.com>,
	Andy Gospodarek <andy@...yhouse.net>, netdev@...r.kernel.org
Subject: Re: [PATCH] bonding: 802.3ad: make aggregator_identifier bond-private

Jiri Bohac <jbohac@...e.cz> wrote:

>On Fri, Feb 14, 2014 at 05:12:43PM -0200, Flavio Leitner wrote:
>> On Fri, Feb 14, 2014 at 06:13:50PM +0100, Jiri Bohac wrote:
>> > Fix this by making aggregator_identifier private to the bond.
>> 
>> I don't see how you fix the duplicate agg id with this patch because
>> you initialize for each bond to 0, then use the same algo further on.
>> So, what is changing?
>
>My understanding is that the aggregator identifier is used
>internally by the bond and never appears anywhere in the LACP
>traffic.
>
>So having duplicate aggregator ids between two bonds on the same
>machine does not matter. But it is a problem if two aggregators
>in the same bond share the same id.
>
>Is my understanding wrong?

	Your understanding is correct.

>> Actually, aggregator_identifier is a global variable to make sure the
>> counter is always increasing for new bonds.  So, the fix would be to
>> not reset it to zero, isn't it?
>
>I was considering this fix, but my concern was that the variable
>(u16) would overflow sooner than it does now. It would take 2^16
>enslavings on the machine, while with my patch you need 2^16
>enslavings on a single bond.
>
>Hypothetically, a rogue NET_ADMIN in one net namespace may cause
>this overflow to break a bond in another nemespace.
>
>Maybe I'm being paranoid? ;)

	Personally, for ease of reading debug messages, I would prefer
the globally unique ID (or a patch to update the pr_debugs to add the
bond name).  From a technical point of view either way will function
correctly.  I'm not too worried about the overflow of the ID.

	-J

---
	-Jay Vosburgh, IBM Linux Technology Center, fubar@...ibm.com

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