net: ipv6: Fixed up ipsec packet be re-routing issue

From: Hui Zhang <huizhang@marvell.com>

    Bug report on https://bugzilla.kernel.org/show_bug.cgi?id=75781

    When a local output ipsec packet match the mangle table rule,
    and be set mark value, the packet will be route again in
    route_me_harder -> _session_decoder6

    In this case, the nhoff in CB of skb was still the default
    value 0. So the protocal match can't success and the packet can't match
    correct SA rule,and then the packet be send out in plaintext.

    To fixed up the issue. The CB->nhoff must be set.

Signed-off-by: huizhang <huizhang@marvell.com>
---
 net/ipv6/xfrm6_policy.c |    2 ++
 1 file changed, 2 insertions(+)

Hello.

On 22-05-2014 11:05, huizhang wrote:

> From: Hui Zhang <huizhang@marvell.com>

>      Bug report on https://bugzilla.kernel.org/show_bug.cgi?id=75781
>
>      When a local output ipsec packet match the mangle table rule,
>      and be set mark value, the packet will be route again in
>      route_me_harder -> _session_decoder6

>      In this case, the nhoff in CB of skb was still the default
>      value 0. So the protocal match can't success and the packet can't match
>      correct SA rule,and then the packet be send out in plaintext.

>      To fixed up the issue. The CB->nhoff must be set.

> Signed-off-by: huizhang <huizhang@marvell.com>

    Why not Hui Zhang, like in the From field?

> ---
>   net/ipv6/xfrm6_policy.c |    2 ++
>   1 file changed, 2 insertions(+)

> diff --git a/net/ipv6/xfrm6_policy.c b/net/ipv6/xfrm6_policy.c
> index 5f8e128..869b68b 100644
> --- a/net/ipv6/xfrm6_policy.c
> +++ b/net/ipv6/xfrm6_policy.c
> @@ -134,6 +134,8 @@ _decode_session6(struct sk_buff *skb, struct flowi *fl, int reverse)
>   	const struct ipv6hdr *hdr = ipv6_hdr(skb);
>   	struct ipv6_opt_hdr *exthdr;
>   	const unsigned char *nh = skb_network_header(skb);
> +	if(IP6CB(skb)->nhoff==0)

    Please, surround == with a space on each side.

WBR, Sergei

--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

From: huizhang <huizhang@marvell.com>
Date: Thu, 22 May 2014 15:05:46 +0800

> diff --git a/net/ipv6/xfrm6_policy.c b/net/ipv6/xfrm6_policy.c
> index 5f8e128..869b68b 100644
> --- a/net/ipv6/xfrm6_policy.c
> +++ b/net/ipv6/xfrm6_policy.c
> @@ -134,6 +134,8 @@ _decode_session6(struct sk_buff *skb, struct flowi *fl, int reverse)
>  	const struct ipv6hdr *hdr = ipv6_hdr(skb);
>  	struct ipv6_opt_hdr *exthdr;
>  	const unsigned char *nh = skb_network_header(skb);
> +	if(IP6CB(skb)->nhoff==0)
> +		IP6CB(skb)->nhoff = offsetof(struct ipv6hdr, nexthdr);
>  	u8 nexthdr = nh[IP6CB(skb)->nhoff];
>  	int oif = 0;

Never put actual statements in the middle of a series of variable
declarations.

Also, it would probably be better to do this assignment in
__ip_local_out().

That's the bug, we only set nhoff in the input paths, we need
to set it in the output paths too if reaching _decode_session6
is possible for output packets.
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

2014-05-23 0:12 GMT+08:00 David Miller <davem@davemloft.net>:
> From: huizhang <huizhang@marvell.com>
> Date: Thu, 22 May 2014 15:05:46 +0800
>
>> diff --git a/net/ipv6/xfrm6_policy.c b/net/ipv6/xfrm6_policy.c
>> index 5f8e128..869b68b 100644
>> --- a/net/ipv6/xfrm6_policy.c
>> +++ b/net/ipv6/xfrm6_policy.c
>> @@ -134,6 +134,8 @@ _decode_session6(struct sk_buff *skb, struct flowi *fl, int reverse)
>>       const struct ipv6hdr *hdr = ipv6_hdr(skb);
>>       struct ipv6_opt_hdr *exthdr;
>>       const unsigned char *nh = skb_network_header(skb);
>> +     if(IP6CB(skb)->nhoff==0)
>> +             IP6CB(skb)->nhoff = offsetof(struct ipv6hdr, nexthdr);
>>       u8 nexthdr = nh[IP6CB(skb)->nhoff];
>>       int oif = 0;
>
> Never put actual statements in the middle of a series of variable
> declarations.
>
> Also, it would probably be better to do this assignment in
> __ip_local_out().
>
> That's the bug, we only set nhoff in the input paths, we need
> to set it in the output paths too if reaching _decode_session6
> is possible for output packets.

You are right,  __ip_local_out maybe a better place.I will submit another patch.
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html