Re: [PATCH ipsec-next] Clear secpath on loopback_xmit

2018-10-08 Thread Steffen Klassert
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 

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 
>  #include 
>  #include 
> +#include 
>  #include   /* For the statistics structure. */
>  #include /* For ARPHRD_ETHER */
>  #include 
> @@ -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


[PATCH ipsec-next] Clear secpath on loopback_xmit

2018-10-05 Thread Benedict Wong
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.

Tests: xfrm tunnel mode tests added for loopback:
https://android-review.googlesource.com/c/kernel/tests/+/777328
Signed-off-by: Benedict Wong 
---
 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 
 #include 
 #include 
+#include 
 #include /* For the statistics structure. */
 #include   /* For ARPHRD_ETHER */
 #include 
@@ -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.
+   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