netfilter: ip6t_NPT: Fix checksuming.

RFC PATCH Applied YOSHIFUJI Hideaki 4 participants 7 messages Jan 26, 2013 – Feb 7, 2013

Subsystems (2)

Networking netfilter

lore

Share & tools

lore

YOSHIFUJI Hideaki

author

2013-01-26 18:37 UTC #0 lore raw

RFC PATCH 1/4 netfilter: ip6t_NPT: Fix checksuming.

Cast __wsum from/to __sum16 is wrong. Instead, apply appropriate conversion function: csum_unfold() or csum_fold().

Signed-off-by YOSHIFUJI Hideaki <[email protected]>

---
 net/ipv6/netfilter/ip6t_NPT.c |    6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

net/ipv6/netfilter/ip6t_NPT.c +3 -3

index 7302b0b..3ff281b 100644
@@ -30,7 +30,7 @@ static int ip6t_npt_checkentry(const struct xt_tgchk_param *par)
 				(__force __wsum)npt->dst_pfx.in6.s6_addr16[i]);
 	}
 
-	npt->adjustment = (__force __sum16) csum_sub(src_sum, dst_sum);
+	npt->adjustment = csum_fold(csum_sub(src_sum, dst_sum));
 	return 0;
 }

@@ -66,8 +66,8 @@ static bool ip6t_npt_map_pfx(const struct ip6t_npt_tginfo *npt,
 			return false;
 	}
 
-	sum = (__force __sum16) csum_add((__force __wsum)addr->s6_addr16[idx],
-			 npt->adjustment);
+	sum = csum_fold(csum_add(csum_unfold((__force __sum16)addr->s6_addr16[idx]),
+				 csum_unfold(npt->adjustment)));
 	if (sum == CSUM_MANGLED_0)
 		sum = 0;
 	*(__force __sum16 *)&addr->s6_addr16[idx] = sum;

Signature · 1 line

1.7.9.5

Ulrich Weber

2013-01-29 14:42 UTC #1 lore raw

Hi Yoshofuji,

thanks for your patches! If I add a onces complement to the return value of csum_fold() it works for my setup.

Cheers
Ulrich

Quoted 33 lines · YOSHIFUJI Hideaki

Cast __wsum from/to __sum16 is wrong. Instead, apply appropriate conversion function: csum_unfold() or csum_fold().

Signed-off-by YOSHIFUJI Hideaki <[email protected]>

---
 net/ipv6/netfilter/ip6t_NPT.c |    6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

net/ipv6/netfilter/ip6t_NPT.c +3 -3

index 7302b0b..3ff281b 100644
@@ -30,7 +30,7 @@ static int ip6t_npt_checkentry(const struct xt_tgchk_param *par)
 				(__force __wsum)npt->dst_pfx.in6.s6_addr16[i]);
 	}
 
-	npt->adjustment = (__force __sum16) csum_sub(src_sum, dst_sum);
+	npt->adjustment = csum_fold(csum_sub(src_sum, dst_sum));
 	return 0;
 }

@@ -66,8 +66,8 @@ static bool ip6t_npt_map_pfx(const struct ip6t_npt_tginfo *npt,
 			return false;
 	}
 
-	sum = (__force __sum16) csum_add((__force __wsum)addr->s6_addr16[idx],
-			 npt->adjustment);
+	sum = csum_fold(csum_add(csum_unfold((__force __sum16)addr->s6_addr16[idx]),
+				 csum_unfold(npt->adjustment)));
 	if (sum == CSUM_MANGLED_0)
 		sum = 0;
 	*(__force __sum16 *)&addr->s6_addr16[idx] = sum;

Quoted 1 line

From 40e0c6d86514a8dcc80f18fbe8a2945c6ee78f6d Mon Sep 17 00:00:00 2001

From: Ulrich Weber <[email protected]>
Date: Tue, 29 Jan 2013 15:24:21 +0100
Subject: [PATCH] netfilter: ip6t_NTP: Use onces complement of csum_fold

we need a 16bit value but not folded

Signed-off-by Ulrich Weber <[email protected]>

---
 net/ipv6/netfilter/ip6t_NPT.c |    6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

net/ipv6/netfilter/ip6t_NPT.c +3 -3

index 74e171d..61a9b95 100644
@@ -35,7 +35,7 @@ static int ip6t_npt_checkentry(const struct xt_tgchk_param *par)
 	src_sum = csum_partial(&npt->src_pfx.in6, sizeof(npt->src_pfx.in6), 0);
 	dst_sum = csum_partial(&npt->dst_pfx.in6, sizeof(npt->dst_pfx.in6), 0);
 
-	npt->adjustment = csum_fold(csum_sub(src_sum, dst_sum));
+	npt->adjustment = ~csum_fold(csum_sub(src_sum, dst_sum));
 	return 0;
 }

@@ -71,8 +71,8 @@ static bool ip6t_npt_map_pfx(const struct ip6t_npt_tginfo *npt,
 			return false;
 	}
 
-	sum = csum_fold(csum_add(csum_unfold((__force __sum16)addr->s6_addr16[idx]),
-				 csum_unfold(npt->adjustment)));
+	sum = ~csum_fold(csum_add(csum_unfold((__force __sum16)addr->s6_addr16[idx]),
+				  csum_unfold(npt->adjustment)));
 	if (sum == CSUM_MANGLED_0)
 		sum = 0;
 	*(__force __sum16 *)&addr->s6_addr16[idx] = sum;

Signature · 1 line

1.7.9.5

Pablo Neira Ayuso

maintainer

2013-01-31 10:04 UTC #2 lore raw

Hi,

Quoted 4 lines · Ulrich Weber

Hi Yoshofuji,
thanks for your patches! If I add a onces complement to the return value of csum_fold() it works for my setup.

Any consensus on the fix for this? I'd like to have some solution into 3.8-rc.

Thanks.

Quoted 74 lines · YOSHIFUJI Hideaki

Quoted 22 lines

Cast __wsum from/to __sum16 is wrong. Instead, apply appropriate conversion function: csum_unfold() or csum_fold().

Signed-off-by YOSHIFUJI Hideaki <[email protected]>

---
 net/ipv6/netfilter/ip6t_NPT.c |    6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

net/ipv6/netfilter/ip6t_NPT.c +1 -1

index 7302b0b..3ff281b 100644
@@ -30,7 +30,7 @@ static int ip6t_npt_checkentry(const struct xt_tgchk_param *par)
 				(__force __wsum)npt->dst_pfx.in6.s6_addr16[i]);
 	}
 
-	npt->adjustment = (__force __sum16) csum_sub(src_sum, dst_sum);
+	npt->adjustment = csum_fold(csum_sub(src_sum, dst_sum));
 	return 0;
 }

net/ipv6/netfilter/ip6t_NPT.c static bool ip6t_npt_map_pfx(const struct ip6t_npt_tginfo *npt, · line 73

Quoted 10 lines

net/ipv6/netfilter/ip6t_NPT.c +2 -2

@@ -66,8 +66,8 @@ static bool ip6t_npt_map_pfx(const struct ip6t_npt_tginfo *npt,
 			return false;
 	}
 
-	sum = (__force __sum16) csum_add((__force __wsum)addr->s6_addr16[idx],
-			 npt->adjustment);
+	sum = csum_fold(csum_add(csum_unfold((__force __sum16)addr->s6_addr16[idx]),
+				 csum_unfold(npt->adjustment)));
 	if (sum == CSUM_MANGLED_0)
 		sum = 0;
 	*(__force __sum16 *)&addr->s6_addr16[idx] = sum;

From 40e0c6d86514a8dcc80f18fbe8a2945c6ee78f6d Mon Sep 17 00:00:00 2001
From: Ulrich Weber <[email protected]>
Date: Tue, 29 Jan 2013 15:24:21 +0100
Subject: [PATCH] netfilter: ip6t_NTP: Use onces complement of csum_fold

we need a 16bit value but not folded

Signed-off-by Ulrich Weber <[email protected]>

---
 net/ipv6/netfilter/ip6t_NPT.c |    6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

net/ipv6/netfilter/ip6t_NPT.c +3 -3

index 74e171d..61a9b95 100644
@@ -35,7 +35,7 @@ static int ip6t_npt_checkentry(const struct xt_tgchk_param *par)
 	src_sum = csum_partial(&npt->src_pfx.in6, sizeof(npt->src_pfx.in6), 0);
 	dst_sum = csum_partial(&npt->dst_pfx.in6, sizeof(npt->dst_pfx.in6), 0);
 
-	npt->adjustment = csum_fold(csum_sub(src_sum, dst_sum));
+	npt->adjustment = ~csum_fold(csum_sub(src_sum, dst_sum));
 	return 0;
 }

@@ -71,8 +71,8 @@ static bool ip6t_npt_map_pfx(const struct ip6t_npt_tginfo *npt,
 			return false;
 	}
 
-	sum = csum_fold(csum_add(csum_unfold((__force __sum16)addr->s6_addr16[idx]),
-				 csum_unfold(npt->adjustment)));
+	sum = ~csum_fold(csum_add(csum_unfold((__force __sum16)addr->s6_addr16[idx]),
+				  csum_unfold(npt->adjustment)));
 	if (sum == CSUM_MANGLED_0)
 		sum = 0;
 	*(__force __sum16 *)&addr->s6_addr16[idx] = sum;

Signature · 1 line

1.7.9.5

Ulrich Weber

2013-01-31 15:59 UTC #3 lore raw

From my side, apply yoshofuji patches and my
onces complement patch, that worked for me ;)

I can do some more testing tomorrow with different addresses and ranges if nobody else finds time...

Cheers
Ulrich

Quoted 84 lines · Pablo Neira Ayuso

Hi,
Quoted 4 lines · Ulrich Weber
Hi Yoshofuji,
thanks for your patches! If I add a onces complement to the return value of csum_fold() it works for my setup.
Any consensus on the fix for this? I'd like to have some solution into 3.8-rc.
Thanks.
Quoted 72 lines · YOSHIFUJI Hideaki
Quoted 22 lines
Cast __wsum from/to __sum16 is wrong. Instead, apply appropriate conversion function: csum_unfold() or csum_fold().
Signed-off-by YOSHIFUJI Hideaki <[email protected]>
---
  net/ipv6/netfilter/ip6t_NPT.c |    6 +++---
  1 file changed, 3 insertions(+), 3 deletions(-)
net/ipv6/netfilter/ip6t_NPT.c +1 -1
index 7302b0b..3ff281b 100644
@@ -30,7 +30,7 @@ static int ip6t_npt_checkentry(const struct xt_tgchk_param *par)
  				(__force __wsum)npt->dst_pfx.in6.s6_addr16[i]);
  	}
 
-	npt->adjustment = (__force __sum16) csum_sub(src_sum, dst_sum);
+	npt->adjustment = csum_fold(csum_sub(src_sum, dst_sum));
  	return 0;
  }
 
net/ipv6/netfilter/ip6t_NPT.c static bool ip6t_npt_map_pfx(const struct ip6t_npt_tginfo *npt, · line 73
Quoted 10 lines
net/ipv6/netfilter/ip6t_NPT.c +2 -2
@@ -66,8 +66,8 @@ static bool ip6t_npt_map_pfx(const struct ip6t_npt_tginfo *npt,
  			return false;
  	}
 
-	sum = (__force __sum16) csum_add((__force __wsum)addr->s6_addr16[idx],
-			 npt->adjustment);
+	sum = csum_fold(csum_add(csum_unfold((__force __sum16)addr->s6_addr16[idx]),
+				 csum_unfold(npt->adjustment)));
  	if (sum == CSUM_MANGLED_0)
  		sum = 0;
  	*(__force __sum16 *)&addr->s6_addr16[idx] = sum;
From 40e0c6d86514a8dcc80f18fbe8a2945c6ee78f6d Mon Sep 17 00:00:00 2001
From: Ulrich Weber <[email protected]>
Date: Tue, 29 Jan 2013 15:24:21 +0100
Subject: [PATCH] netfilter: ip6t_NTP: Use onces complement of csum_fold
we need a 16bit value but not folded
Signed-off-by Ulrich Weber <[email protected]>
---
  net/ipv6/netfilter/ip6t_NPT.c |    6 +++---
  1 file changed, 3 insertions(+), 3 deletions(-)
net/ipv6/netfilter/ip6t_NPT.c +3 -3
index 74e171d..61a9b95 100644
@@ -35,7 +35,7 @@ static int ip6t_npt_checkentry(const struct xt_tgchk_param *par)
  	src_sum = csum_partial(&npt->src_pfx.in6, sizeof(npt->src_pfx.in6), 0);
  	dst_sum = csum_partial(&npt->dst_pfx.in6, sizeof(npt->dst_pfx.in6), 0);
 
-	npt->adjustment = csum_fold(csum_sub(src_sum, dst_sum));
+	npt->adjustment = ~csum_fold(csum_sub(src_sum, dst_sum));
  	return 0;
  }
 
@@ -71,8 +71,8 @@ static bool ip6t_npt_map_pfx(const struct ip6t_npt_tginfo *npt,
  			return false;
  	}
 
-	sum = csum_fold(csum_add(csum_unfold((__force __sum16)addr->s6_addr16[idx]),
-				 csum_unfold(npt->adjustment)));
+	sum = ~csum_fold(csum_add(csum_unfold((__force __sum16)addr->s6_addr16[idx]),
+				  csum_unfold(npt->adjustment)));
  	if (sum == CSUM_MANGLED_0)
  		sum = 0;
  	*(__force __sum16 *)&addr->s6_addr16[idx] = sum;
Signature · 1 line
1.7.9.5

Jean-Michel DILLY

2013-01-31 18:45 UTC #4 lore raw

I'll make some tests this week-end. I have had no time theses days.

Regards,
Jean-Michel

Quoted 90 lines · Ulrich Weber

From my side, apply yoshofuji patches and my
onces complement patch, that worked for me ;)
I can do some more testing tomorrow with different addresses and ranges if nobody else finds time...
Cheers
Ulrich
Quoted 78 lines · Pablo Neira Ayuso
Hi,
Quoted 4 lines · Ulrich Weber
Hi Yoshofuji,
thanks for your patches! If I add a onces complement to the return value of csum_fold() it works for my setup.
Any consensus on the fix for this? I'd like to have some solution into 3.8-rc.
Thanks.
Quoted 66 lines · YOSHIFUJI Hideaki
Quoted 19 lines
Cast __wsum from/to __sum16 is wrong. Instead, apply appropriate conversion function: csum_unfold() or csum_fold().
Signed-off-by YOSHIFUJI Hideaki <[email protected]>
---
 net/ipv6/netfilter/ip6t_NPT.c |    6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)
net/ipv6/netfilter/ip6t_NPT.c +1 -0
index 7302b0b..3ff281b 100644
@@ -30,7 +30,7 @@ static int ip6t_npt_checkentry(const struct xt_tgchk_param *par)
 				(__force __wsum)npt->dst_pfx.in6.s6_addr16[i]);
 	}
 -	npt->adjustment = (__force __sum16) csum_sub(src_sum, dst_sum);
+	npt->adjustment = csum_fold(csum_sub(src_sum, dst_sum));
 	return 0;
 }
net/ipv6/netfilter/ip6t_NPT.c static int ip6t_npt_checkentry(const struct xt_tgchk_param *par)
Quoted 10 lines
net/ipv6/netfilter/ip6t_NPT.c +2 -1
 @@ -66,8 +66,8 @@ static bool ip6t_npt_map_pfx(const struct ip6t_npt_tginfo *npt,
 			return false;
 	}
 -	sum = (__force __sum16) csum_add((__force __wsum)addr->s6_addr16[idx],
-			 npt->adjustment);
+	sum = csum_fold(csum_add(csum_unfold((__force __sum16)addr->s6_addr16[idx]),
+				 csum_unfold(npt->adjustment)));
 	if (sum == CSUM_MANGLED_0)
 		sum = 0;
 	*(__force __sum16 *)&addr->s6_addr16[idx] = sum;
From 40e0c6d86514a8dcc80f18fbe8a2945c6ee78f6d Mon Sep 17 00:00:00 2001
From: Ulrich Weber <[email protected]>
Date: Tue, 29 Jan 2013 15:24:21 +0100
Subject: [PATCH] netfilter: ip6t_NTP: Use onces complement of csum_fold
we need a 16bit value but not folded
Signed-off-by Ulrich Weber <[email protected]>
---
 net/ipv6/netfilter/ip6t_NPT.c |    6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)
net/ipv6/netfilter/ip6t_NPT.c +3 -1
index 74e171d..61a9b95 100644
@@ -35,7 +35,7 @@ static int ip6t_npt_checkentry(const struct xt_tgchk_param *par)
 	src_sum = csum_partial(&npt->src_pfx.in6, sizeof(npt->src_pfx.in6), 0);
 	dst_sum = csum_partial(&npt->dst_pfx.in6, sizeof(npt->dst_pfx.in6), 0);
 -	npt->adjustment = csum_fold(csum_sub(src_sum, dst_sum));
+	npt->adjustment = ~csum_fold(csum_sub(src_sum, dst_sum));
 	return 0;
 }
 @@ -71,8 +71,8 @@ static bool ip6t_npt_map_pfx(const struct ip6t_npt_tginfo *npt,
 			return false;
 	}
 -	sum = csum_fold(csum_add(csum_unfold((__force __sum16)addr->s6_addr16[idx]),
-				 csum_unfold(npt->adjustment)));
+	sum = ~csum_fold(csum_add(csum_unfold((__force __sum16)addr->s6_addr16[idx]),
+				  csum_unfold(npt->adjustment)));
 	if (sum == CSUM_MANGLED_0)
 		sum = 0;
 	*(__force __sum16 *)&addr->s6_addr16[idx] = sum;
Signature · 1 line
1.7.9.5

Jean-Michel DILLY

2013-02-03 22:31 UTC #5 lore raw

In reply to Ulrich Weber

Looks good from here. :)

Regards,
JMichel DILLY

Quoted 90 lines · Ulrich Weber

From my side, apply yoshofuji patches and my
onces complement patch, that worked for me ;)
I can do some more testing tomorrow with different addresses and ranges if nobody else finds time...
Cheers
Ulrich
Quoted 78 lines · Pablo Neira Ayuso
Hi,
Quoted 4 lines · Ulrich Weber
Hi Yoshofuji,
thanks for your patches! If I add a onces complement to the return value of csum_fold() it works for my setup.
Any consensus on the fix for this? I'd like to have some solution into 3.8-rc.
Thanks.
Quoted 66 lines · YOSHIFUJI Hideaki
Quoted 19 lines
Cast __wsum from/to __sum16 is wrong. Instead, apply appropriate conversion function: csum_unfold() or csum_fold().
Signed-off-by YOSHIFUJI Hideaki <[email protected]>
---
 net/ipv6/netfilter/ip6t_NPT.c |    6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)
net/ipv6/netfilter/ip6t_NPT.c +1 -0
index 7302b0b..3ff281b 100644
@@ -30,7 +30,7 @@ static int ip6t_npt_checkentry(const struct xt_tgchk_param *par)
 				(__force __wsum)npt->dst_pfx.in6.s6_addr16[i]);
 	}
 -	npt->adjustment = (__force __sum16) csum_sub(src_sum, dst_sum);
+	npt->adjustment = csum_fold(csum_sub(src_sum, dst_sum));
 	return 0;
 }
net/ipv6/netfilter/ip6t_NPT.c static int ip6t_npt_checkentry(const struct xt_tgchk_param *par)
Quoted 10 lines
net/ipv6/netfilter/ip6t_NPT.c +2 -1
 @@ -66,8 +66,8 @@ static bool ip6t_npt_map_pfx(const struct ip6t_npt_tginfo *npt,
 			return false;
 	}
 -	sum = (__force __sum16) csum_add((__force __wsum)addr->s6_addr16[idx],
-			 npt->adjustment);
+	sum = csum_fold(csum_add(csum_unfold((__force __sum16)addr->s6_addr16[idx]),
+				 csum_unfold(npt->adjustment)));
 	if (sum == CSUM_MANGLED_0)
 		sum = 0;
 	*(__force __sum16 *)&addr->s6_addr16[idx] = sum;
From 40e0c6d86514a8dcc80f18fbe8a2945c6ee78f6d Mon Sep 17 00:00:00 2001
From: Ulrich Weber <[email protected]>
Date: Tue, 29 Jan 2013 15:24:21 +0100
Subject: [PATCH] netfilter: ip6t_NTP: Use onces complement of csum_fold
we need a 16bit value but not folded
Signed-off-by Ulrich Weber <[email protected]>
---
 net/ipv6/netfilter/ip6t_NPT.c |    6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)
net/ipv6/netfilter/ip6t_NPT.c +3 -1
index 74e171d..61a9b95 100644
@@ -35,7 +35,7 @@ static int ip6t_npt_checkentry(const struct xt_tgchk_param *par)
 	src_sum = csum_partial(&npt->src_pfx.in6, sizeof(npt->src_pfx.in6), 0);
 	dst_sum = csum_partial(&npt->dst_pfx.in6, sizeof(npt->dst_pfx.in6), 0);
 -	npt->adjustment = csum_fold(csum_sub(src_sum, dst_sum));
+	npt->adjustment = ~csum_fold(csum_sub(src_sum, dst_sum));
 	return 0;
 }
 @@ -71,8 +71,8 @@ static bool ip6t_npt_map_pfx(const struct ip6t_npt_tginfo *npt,
 			return false;
 	}
 -	sum = csum_fold(csum_add(csum_unfold((__force __sum16)addr->s6_addr16[idx]),
-				 csum_unfold(npt->adjustment)));
+	sum = ~csum_fold(csum_add(csum_unfold((__force __sum16)addr->s6_addr16[idx]),
+				  csum_unfold(npt->adjustment)));
 	if (sum == CSUM_MANGLED_0)
 		sum = 0;
 	*(__force __sum16 *)&addr->s6_addr16[idx] = sum;
Signature · 1 line
1.7.9.5

Pablo Neira Ayuso

maintainer Applied

2013-02-07 18:14 UTC #6 lore raw

In reply to YOSHIFUJI Hideaki

Quoted 2 lines · YOSHIFUJI Hideaki

Cast __wsum from/to __sum16 is wrong. Instead, apply appropriate conversion function: csum_unfold() or csum_fold().

Applied.

I have mangled this patch by collapsing Ulrich's:

http://patchwork.ozlabs.org/patch/216563/

And I have added this to the description:

[ The original patch has been modified to undo the final ~ that
csum_fold returns. We only need to fold the 32-bit word that results from the checksum calculation into a 16-bit to ensure that the original subnet is restored appropriately. Spotted by Ulrich Weber. ]

Thanks.