netdev - Re: [PATCH next 3/3] net: Use l3_dev instead of skb->dev for L3 processing

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 for Android: free password hash cracker in your pocket

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:	Fri, 4 Mar 2016 14:12:31 -0800
From:	Mahesh Bandewar <maheshb@...gle.com>
To:	Cong Wang <xiyou.wangcong@...il.com>
Cc:	Mahesh Bandewar <mahesh@...dewar.net>,
	David Miller <davem@...emloft.net>,
	Eric Dumazet <edumazet@...gle.com>,
	netdev <netdev@...r.kernel.org>, Tim Hockin <thockin@...gle.com>,
	Alex Pollitt <alex.pollitt@...aswitch.com>,
	Matthew Dupre <matthew.dupre@...aswitch.com>
Subject: Re: [PATCH next 3/3] net: Use l3_dev instead of skb->dev for L3 processing

On Fri, Mar 4, 2016 at 9:30 AM, Cong Wang <xiyou.wangcong@...il.com> wrote:
> On Thu, Mar 3, 2016 at 5:42 PM, Mahesh Bandewar <maheshb@...gle.com> wrote:
>>>>> As you mentioned logically we should be able to pass the skb in master's
>>>>> ns
>>>>> until L3 processing is completed. This patch series attempts to do that by
>>>>> disassociating this logic from skb->dev and adding it to l3_dev. This
>>>>> should
>>>>> include not just IPT but all that is done in L3 phase (IPT, routing etc.)
>>>>> Also since dev->l3_dev is same as dev, this should not break any existing
>>>>> logic.
>>>>>
>>>> Well, looking at the code I realized that I missed few places (especially
>>>> routing
>>>> logic) which continues using skb->dev in ingress path and should be
>>>> corrected to
>>>> use l3_dev. I'll correct those places and send the next version.
>>>
>>>
>>> Look, even you yourself are missing something here. ;) This is exactly why
>>> I suggest to consider another approach. Please don't introduce any code
>>> that is hard to debug even for yourself. The struct net pointer is passed
>>> around in kernel network subsystem almost everywhere, it is not easy to make
>>> it bug-free by just switching skb->dev.
>>>
>> I disagree. Conceptually this is very easy to understand as we are taking L3
>> processing off of skb->dev and loading it onto dev->l3_dev. So
>> everything that is
>> associated with l3_dev is for L3. This should neither make debugging harder
>> nor add complicated code.
>
> Nope, conceptually it is not right, that breaks _isolation_ in
> concept, we need to
> make each skb really traverse in the original stack, not just
> switching skb->dev,
> that is cheating. It is just too easy to hide bugs in your way, we
> never use network
> namespace in this way before.

Unfortunately we don't have a way to switch to ns after L3 processing.
So I would
argue it the other-way around. The way it is now; breaks the _isolation_ model.
If the default-ns is responsible for whole L3 (in this situation) and
it does pretty well
on egress but there is no way to do that in ingress path. IPtables is
not the only thing,
how about routing, how about IPsec? None of this will function well.
So we need to
have a generic solution to solve all these problems.