From mboxrd@z Thu Jan  1 00:00:00 1970
From: Steffen Klassert <steffen.klassert@secunet.com>
Subject: Re: [PATCH ipsec-next] Clear secpath on loopback_xmit
Date: Mon, 8 Oct 2018 08:40:39 +0200
Message-ID: <20181008064039.GJ3823@gauss3.secunet.de>
References: <20181005182327.14819-1-benedictwong@google.com>
Mime-Version: 1.0
Content-Type: text/plain; charset="us-ascii"
Cc: <netdev@vger.kernel.org>, <nharold@google.com>,
        <lorenzo@google.com>
To: Benedict Wong <benedictwong@google.com>
Return-path: <netdev-owner@vger.kernel.org>
Received: from a.mx.secunet.com ([62.96.220.36]:44792 "EHLO a.mx.secunet.com"
        rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP
        id S1725857AbeJHNuu (ORCPT <rfc822;netdev@vger.kernel.org>);
        Mon, 8 Oct 2018 09:50:50 -0400
Content-Disposition: inline
In-Reply-To: <20181005182327.14819-1-benedictwong@google.com>
Sender: netdev-owner@vger.kernel.org
List-ID: <netdev.vger.kernel.org>

On Fri, Oct 05, 2018 at 11:23:28AM -0700, Benedict Wong wrote:
> This patch clears the skb->sp when transmitted over loopback. This
> ensures that the loopback-ed packet does not have any secpath
> information from the outbound transforms.
> 
> At present, this causes XFRM tunnel mode packets to be dropped with
> XFRMINNOPOLS, due to the outbound state being in the secpath, without
> a matching inbound policy. Clearing the secpath ensures that all states
> added to the secpath are exclusively from the inbound processing.

This looks like a fix, so the target should be the ipsec tree,
not ipsec-next.

> 
> Tests: xfrm tunnel mode tests added for loopback:
>     https://android-review.googlesource.com/c/kernel/tests/+/777328
> Signed-off-by: Benedict Wong <benedictwong@google.com>

Please add a 'Fixes:' tag.

> ---
>  drivers/net/loopback.c | 4 ++++
>  1 file changed, 4 insertions(+)
> 
> diff --git a/drivers/net/loopback.c b/drivers/net/loopback.c
> index 30612497643c..a6bf54df94bd 100644
> --- a/drivers/net/loopback.c
> +++ b/drivers/net/loopback.c
> @@ -50,6 +50,7 @@
>  #include <linux/ethtool.h>
>  #include <net/sock.h>
>  #include <net/checksum.h>
> +#include <net/xfrm.h>
>  #include <linux/if_ether.h>	/* For the statistics structure. */
>  #include <linux/if_arp.h>	/* For ARPHRD_ETHER */
>  #include <linux/ip.h>
> @@ -82,6 +83,9 @@ static netdev_tx_t loopback_xmit(struct sk_buff *skb,
>  	 */
>  	skb_dst_force(skb);
>  
> +	// Clear secpath to ensure xfrm policy check not tainted by outbound SAs.

Comments should use /* ... */, like it is done below.

> +	secpath_reset(skb);
> +
>  	skb->protocol = eth_type_trans(skb, dev);
>  
>  	/* it's OK to use per_cpu_ptr() because BHs are off */
> -- 
> 2.19.0.605.g01d371f741-goog