[PATCH]extensions/tos_values.c mask value not accurate in certain condition

[PATCH]extensions/tos_values.c mask value not accurate in certain condition

[DuanZhenzhong]

scene:
# iptables -V
iptables v1.4.10
# iptables -v -t mangle -A MANGLE_OUTPUT -p tcp --dport 20 -j TOS
--set-tos 8
TOS tcp opt -- in * out * 0.0.0.0/0 -> 0.0.0.0/0 tcp dpt:20 TOS set
0x08/0xff
# iptables -v -t mangle -A MANGLE_OUTPUT -p tcp --dport 20 -j TOS
--set-tos Maximize-Throughput
TOS tcp opt -- in * out * 0.0.0.0/0 -> 0.0.0.0/0 tcp dpt:20 TOS set
0x08/0x3f

mask value is different for the same tos value. This is because below
code piece:
static bool tos_parse_numeric(const char *str, struct tos_value_mask *tvm,
unsigned int bits)
{
const unsigned int max = (1 << bits) - 1;
......
tvm->mask = max;
......
static bool tos_parse_symbolic(const char *str, struct tos_value_mask *tvm,
unsigned int def_mask)
{
const unsigned int max = UINT8_MAX;
const struct tos_symbol_info *symbol;
char *tmp;

if (xtables_strtoui(str, &tmp, NULL, 0, max))
return tos_parse_numeric(str, tvm, max);

/* Do not consider ECN bits */
tvm->mask = def_mask;
.......
For tos value 8, bits shift lead to a overflow and trim, so the mask is
0xff no matter what the def_mask is.
For tos symbol Maximize-Throughput, tvm->mask got def_mask 0x3f.

PATCH:
diff -up iptables-1.4.10/extensions/tos_values.c.org
iptables-1.4.10/extensions/tos_values.c
--- iptables-1.4.10/extensions/tos_values.c.org 2010-11-02
13:08:32.000000000 +0800
+++ iptables-1.4.10/extensions/tos_values.c 2010-11-02
13:09:00.000000000 +0800
@@ -34,7 +34,7 @@ static const struct tos_symbol_info {
static bool tos_parse_numeric(const char *str, struct tos_value_mask *tvm,
unsigned int bits)
{
- const unsigned int max = (1 << bits) - 1;
+ const unsigned int max = bits;
unsigned int value;
char *end;

@@ -59,7 +59,7 @@ static bool tos_parse_numeric(const char
static bool tos_parse_symbolic(const char *str, struct tos_value_mask *tvm,
unsigned int def_mask)
{
- const unsigned int max = UINT8_MAX;
+ const unsigned int max = def_mask;
const struct tos_symbol_info *symbol;
char *tmp;

--------------------------------------------------------------------------

Re: [PATCH]extensions/tos_values.c mask value not accurate in certain condition

[Jan Engelhardt]

On Tuesday 2010-11-02 06:26, DuanZhenzhong wrote:

>scene:
># iptables -V
>iptables v1.4.10
># iptables -v -t mangle -A MANGLE_OUTPUT -p tcp --dport 20 -j TOS
>--set-tos 8
>TOS tcp opt -- in * out * 0.0.0.0/0 -> 0.0.0.0/0 tcp dpt:20 TOS set
>0x08/0xff
># iptables -v -t mangle -A MANGLE_OUTPUT -p tcp --dport 20 -j TOS
>--set-tos Maximize-Throughput
>TOS tcp opt -- in * out * 0.0.0.0/0 -> 0.0.0.0/0 tcp dpt:20 TOS set
>0x08/0x3f
>
>mask value is different for the same tos value. This is because below
>code piece:
>static bool tos_parse_numeric(const char *str, struct tos_value_mask *tvm,
>unsigned int bits)
>{
>const unsigned int max = (1 << bits) - 1;
>......
>tvm->mask = max;
>......
>static bool tos_parse_symbolic(const char *str, struct tos_value_mask *tvm,
>unsigned int def_mask)
>{
>const unsigned int max = UINT8_MAX;
>const struct tos_symbol_info *symbol;
>char *tmp;
>
>if (xtables_strtoui(str, &tmp, NULL, 0, max))
>return tos_parse_numeric(str, tvm, max);
>
>/* Do not consider ECN bits */
>tvm->mask = def_mask;
>.......
>For tos value 8, bits shift lead to a overflow and trim, so the mask is
>0xff no matter what the def_mask is.

>For tos symbol Maximize-Throughput, tvm->mask got def_mask 0x3f.

This at least is intended.



Patrick, please grab from
	git://dev.medozas.de/iptables master

parent 600f38db82548a683775fd89b6e136673e924097 (v1.4.9-24-g600f38d)
commit 648fd1ad68ae2ec675ac07efee80783912535404
Author: Jan Engelhardt <jengelh@medozas.de>
Date:   Tue Nov 2 09:10:34 2010 +0100

libxt_TOS: avoid an undesired overflowing computation

The @bits parameter was wrongly labeled and should have been @max
already. This makes the - overflowing - 1<<bits redundant of course.

Signed-off-by: Jan Engelhardt <jengelh@medozas.de>
---
 extensions/tos_values.c |   19 +++++++++++--------
 1 files changed, 11 insertions(+), 8 deletions(-)

diff --git a/extensions/tos_values.c b/extensions/tos_values.c
index 10add19..a65ef25 100644
--- a/extensions/tos_values.c
+++ b/extensions/tos_values.c
@@ -26,15 +26,13 @@ static const struct tos_symbol_info {
 /*
  * tos_parse_numeric - parse sth. like "15/255"
  *
- * @s:		input string
- * @info:	accompanying structure
- * @bits:	number of bits that are allowed
- *		(8 for IPv4 TOS field, 4 for IPv6 Priority Field)
+ * @str:	input string
+ * @tvm:	(value/mask) tuple
+ * @max:	maximum allowed value (must be pow(2,some_int)-1)
  */
 static bool tos_parse_numeric(const char *str, struct tos_value_mask *tvm,
-                              unsigned int bits)
+                              unsigned int max)
 {
-	const unsigned int max = (1 << bits) - 1;
 	unsigned int value;
 	char *end;
 
@@ -56,17 +54,22 @@ static bool tos_parse_numeric(const char *str, struct tos_value_mask *tvm,
 	return true;
 }
 
+/**
+ * @str:	input string
+ * @tvm:	(value/mask) tuple
+ * @def_mask:	mask to force when a symbolic name is used
+ */
 static bool tos_parse_symbolic(const char *str, struct tos_value_mask *tvm,
     unsigned int def_mask)
 {
-	const unsigned int max = UINT8_MAX;
+	static const unsigned int max = UINT8_MAX;
 	const struct tos_symbol_info *symbol;
 	char *tmp;
 
 	if (xtables_strtoui(str, &tmp, NULL, 0, max))
 		return tos_parse_numeric(str, tvm, max);
 
-	/* Do not consider ECN bits */
+	/* Do not consider ECN bits when using preset names */
 	tvm->mask = def_mask;
 	for (symbol = tos_symbol_names; symbol->name != NULL; ++symbol)
 		if (strcasecmp(str, symbol->name) == 0) {
-- 
# Created with git-export-patch

Re: [PATCH]extensions/tos_values.c mask value not accurate in certain condition

[Patrick McHardy]

Am 02.11.2010 09:20, schrieb Jan Engelhardt:
> Patrick, please grab from
> 	git://dev.medozas.de/iptables master

It seems this was based on the iptables-next branch. However I should
have applied the option precedence patch to the main branch anyways.

Pulled, thanks.