diff mbox

[RFC-PATCH] netfilter: payload_len is be16, add size of struct rather than size of pointer

Message ID 1226078023.11596.17.camel@brick
State Accepted, archived
Delegated to: David Miller
Headers show

Commit Message

Harvey Harrison Nov. 7, 2008, 5:13 p.m. UTC
payload_len is a be16 value, not cpu_endian, also the size of a ponter
to a struct ipv6hdr was being added, not the size of the struct itself.

Signed-off-by: Harvey Harrison <harvey.harrison@gmail.com>
---
I'm quite supicious of the following code in net/netfilter/ipvs/ip_vs_xmit.c

Line 714:
	iph->payload_len	=	old_iph->payload_len + sizeof(old_iph);

I believe that the payload_len is a big-endian value and this is treating it as
cpu-ordered.  In addition, it is adding the size of a pointer to a struct ipv6hdr
and not the size of the struct itself.

If I'm correct, I'd suggest the following is what _may_ have been intended.

 net/netfilter/ipvs/ip_vs_xmit.c |    3 ++-
 1 files changed, 2 insertions(+), 1 deletions(-)

Comments

Julius Volz Nov. 8, 2008, 12:51 a.m. UTC | #1
On Fri, Nov 7, 2008 at 6:13 PM, Harvey Harrison
<harvey.harrison@gmail.com> wrote:
> payload_len is a be16 value, not cpu_endian, also the size of a ponter
> to a struct ipv6hdr was being added, not the size of the struct itself.
>
> Signed-off-by: Harvey Harrison <harvey.harrison@gmail.com>
> ---
> I'm quite supicious of the following code in net/netfilter/ipvs/ip_vs_xmit.c
>
> Line 714:
>        iph->payload_len        =       old_iph->payload_len + sizeof(old_iph);
>
> I believe that the payload_len is a big-endian value and this is treating it as
> cpu-ordered.  In addition, it is adding the size of a pointer to a struct ipv6hdr
> and not the size of the struct itself.
>
> If I'm correct, I'd suggest the following is what _may_ have been intended.
>
>  net/netfilter/ipvs/ip_vs_xmit.c |    3 ++-
>  1 files changed, 2 insertions(+), 1 deletions(-)
>
> diff --git a/net/netfilter/ipvs/ip_vs_xmit.c b/net/netfilter/ipvs/ip_vs_xmit.c
> index 2f36721..425ab14 100644
> --- a/net/netfilter/ipvs/ip_vs_xmit.c
> +++ b/net/netfilter/ipvs/ip_vs_xmit.c
> @@ -711,7 +711,8 @@ ip_vs_tunnel_xmit_v6(struct sk_buff *skb, struct ip_vs_conn *cp,
>        iph                     =       ipv6_hdr(skb);
>        iph->version            =       6;
>        iph->nexthdr            =       IPPROTO_IPV6;
> -       iph->payload_len        =       old_iph->payload_len + sizeof(old_iph);
> +       iph->payload_len        =       old_iph->payload_len;
> +       be16_add_cpu(&iph->payload_len, sizeof(*old_iph));
>        iph->priority           =       old_iph->priority;
>        memset(&iph->flow_lbl, 0, sizeof(iph->flow_lbl));
>        iph->daddr              =       rt->rt6i_dst.addr;
> --
> 1.6.0.3.756.gb776d

Yes, both of that makes sense, thank you!

This function was never tested because IIRC there was/is no
decapsulation support for IPv6 in IPv6 tunneling on the receiving side
(please correct me if I'm wrong). I sent some packets through it which
somehow arrived at the real server, but that was about it. So the
whole code in that function is pretty only an idea of how the v6
tunneling should work ;)

Julius
--
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
David Miller Nov. 11, 2008, 12:46 a.m. UTC | #2
From: Harvey Harrison <harvey.harrison@gmail.com>
Date: Fri, 07 Nov 2008 09:13:43 -0800

> payload_len is a be16 value, not cpu_endian, also the size of a ponter
> to a struct ipv6hdr was being added, not the size of the struct itself.
> 
> Signed-off-by: Harvey Harrison <harvey.harrison@gmail.com>

Applied, thanks for fixing this Harvey.
--
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
diff mbox

Patch

diff --git a/net/netfilter/ipvs/ip_vs_xmit.c b/net/netfilter/ipvs/ip_vs_xmit.c
index 2f36721..425ab14 100644
--- a/net/netfilter/ipvs/ip_vs_xmit.c
+++ b/net/netfilter/ipvs/ip_vs_xmit.c
@@ -711,7 +711,8 @@  ip_vs_tunnel_xmit_v6(struct sk_buff *skb, struct ip_vs_conn *cp,
 	iph			=	ipv6_hdr(skb);
 	iph->version		=	6;
 	iph->nexthdr		=	IPPROTO_IPV6;
-	iph->payload_len	=	old_iph->payload_len + sizeof(old_iph);
+	iph->payload_len	=	old_iph->payload_len;
+	be16_add_cpu(&iph->payload_len, sizeof(*old_iph));
 	iph->priority		=	old_iph->priority;
 	memset(&iph->flow_lbl, 0, sizeof(iph->flow_lbl));
 	iph->daddr		=	rt->rt6i_dst.addr;