netdev - Re: [net PATCH] ip_vti/ip6_vti: Clear skb->mark when resetting skb->dev in receive path

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:	Fri, 15 May 2015 12:14:43 -0700
From:	Alexander Duyck <alexander.duyck@...il.com>
To:	David Miller <davem@...hat.com>, herbert@...dor.apana.org.au
CC:	alexander.h.duyck@...hat.com, netdev@...r.kernel.org,
	steffen.klassert@...unet.com, tgraf@...g.ch
Subject: Re: [net PATCH] ip_vti/ip6_vti: Clear skb->mark when resetting skb->dev
 in receive path

On 05/15/2015 09:37 AM, David Miller wrote:
> From: Herbert Xu <herbert@...dor.apana.org.au>
> Date: Thu, 14 May 2015 14:26:14 +0800
>
>> On Wed, May 13, 2015 at 11:14:39PM -0700, Alexander Duyck wrote:
>>> The fact is I am not all that familiar with the vti code and just
>>> started crawling through it a few days ago, but it seems like it is
>>> overwriting the skb->mark value with the i_key to determine which
>>> policy to use.  The code prior to commit df3893c176e9 ("vti: Update
>>> the ipv4 side to use it's own receive hook.") was saving the old
>>> skb->mark, overwriting it, and then restoring it after a call to
>>> xfrm4_policy_check.  After that commit it was letting
>>> skb_scrub_packet in vti_rcv_cb clear the mark and it was just
>>> dropped.
>> Steffen, why is vti touching skb->mark at all? This is supposed
>> to be a field used by user-space to control a packet as it moves
>> inside the kernel.  Seconding it for other purposes looks very
>> wrong.
> If anything, the skb_scrub_packet() call right above the skb->mark
> clears should be taking care of this.

That only applies if you are crossing namespaces which we are not in 
this case.

> The only case where mark should be cleared is if we are changing
> namespaces, and that's exactly the policy implemented by
> skb_scrub_packet() currently.

Right.  The problem is it looks like vti and vti6 are using the mark to 
signal to the policy that is meant to be used for either end of the 
tunnel.  From what I can tell at some point there was a pre-routing hook 
that was used but later it was replaced with the i_key for input, and 
o_key for output.

> Yeah, this mark handling via tunnel->parms.o_key looks not so good.

So is there any recommendations for an alternative to make it so that 
the ipsec endpoint is identified as needing to be encrypted or 
decrypted?  If needed I could probably take a day or two to try and 
address it as I still have a few other minor things I want to try and 
fix such as the MTU configuration for vti/vti6.

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