Skip to contentSkip to conversation

Some parameter fixes for ip(6)tables.c

PATCH In review Jonas Berlin 2 participants 5 messages Mar 23, 2005 – Apr 11, 2005 netfilter

lore
Share & tools
lore

Jonas Berlin

author
#0 lore raw

PATCH 1/1 Some parameter fixes for ip(6)tables.c

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

iptables.c and ip6tables.c IMO have three parameter problems, which I attempt to fix with the attached patch. Both files have the same defects, and thus both files have logically identical fixes.

1. the optflags array contains a '3' for the OPT_LINENUMBERS entry while everywhere else '0' is used. Thus the fix changes it

2. the commands_v_options two-dimensional array lacks a column for the -c flag. I tried to deduce feasible values, please look at them before accepting :)

3. the inverse_for_options array lacks a row for the -c flag. By looking at the code I figured it does not allow the "!" inversion option, so I constructed the line accordingly.

- --
- - xkr47
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.1 (GNU/Linux)

iD8DBQFCQUTSxyF48ZTvn+4RAq5vAJ4g5FCblhR4TWJgZJoiK1nSRRa8oACcDLbz myTf8ISAGl/t4AhwNf1si8k=
=Mha+
-----END PGP SIGNATURE-----

ip6tables.c +17 -16
diff --exclude-from=/tmp/srcdiff.excludes.NOfFZH -Np -ur orig/iptables-1.3.1/ip6tables.c iptables-1.3.1-fix/ip6tables.c
@@ -103,7 +103,7 @@ static const char cmdflags[] = { 'I', 'D
 #define OPT_COUNTERS	0x00400U
 #define NUMBER_OF_OPT	11
 static const char optflags[NUMBER_OF_OPT]
-= { 'n', 's', 'd', 'p', 'j', 'v', 'x', 'i', 'o', '3', 'c'};
+= { 'n', 's', 'd', 'p', 'j', 'v', 'x', 'i', 'o', '0', 'c'};
 
 static struct option original_opts[] = {
 	{ "append", 1, 0, 'A' },

@@ -159,20 +159,20 @@ static unsigned int global_option_offset
 static char commands_v_options[NUMBER_OF_CMD][NUMBER_OF_OPT] =
 /* Well, it's better than "Re: Linux vs FreeBSD" */
 {
-	/*     -n  -s  -d  -p  -j  -v  -x  -i  -o  --line */
-/*INSERT*/    {'x',' ',' ',' ',' ',' ','x',' ',' ','x'},
-/*DELETE*/    {'x',' ',' ',' ',' ',' ','x',' ',' ','x'},
-/*DELETE_NUM*/{'x','x','x','x','x',' ','x','x','x','x'},
-/*REPLACE*/   {'x',' ',' ',' ',' ',' ','x',' ',' ','x'},
-/*APPEND*/    {'x',' ',' ',' ',' ',' ','x',' ',' ','x'},
-/*LIST*/      {' ','x','x','x','x',' ',' ','x','x',' '},
-/*FLUSH*/     {'x','x','x','x','x',' ','x','x','x','x'},
-/*ZERO*/      {'x','x','x','x','x',' ','x','x','x','x'},
-/*NEW_CHAIN*/ {'x','x','x','x','x',' ','x','x','x','x'},
-/*DEL_CHAIN*/ {'x','x','x','x','x',' ','x','x','x','x'},
-/*SET_POLICY*/{'x','x','x','x','x',' ','x','x','x','x'},
-/*CHECK*/     {'x','+','+','+','x',' ','x',' ',' ','x'},
-/*RENAME*/    {'x','x','x','x','x',' ','x','x','x','x'}
+	/*     -n  -s  -d  -p  -j  -v  -x  -i  -o  --line -c */
+/*INSERT*/    {'x',' ',' ',' ',' ',' ','x',' ',' ','x',   ' '},
+/*DELETE*/    {'x',' ',' ',' ',' ',' ','x',' ',' ','x',   'x'},
+/*DELETE_NUM*/{'x','x','x','x','x',' ','x','x','x','x',   'x'},
+/*REPLACE*/   {'x',' ',' ',' ',' ',' ','x',' ',' ','x',   'x'},
+/*APPEND*/    {'x',' ',' ',' ',' ',' ','x',' ',' ','x',   ' '},
+/*LIST*/      {' ','x','x','x','x',' ',' ','x','x',' ',   'x'},
+/*FLUSH*/     {'x','x','x','x','x',' ','x','x','x','x',   'x'},
+/*ZERO*/      {'x','x','x','x','x',' ','x','x','x','x',   'x'},
+/*NEW_CHAIN*/ {'x','x','x','x','x',' ','x','x','x','x',   'x'},
+/*DEL_CHAIN*/ {'x','x','x','x','x',' ','x','x','x','x',   'x'},
+/*SET_POLICY*/{'x','x','x','x','x',' ','x','x','x','x',   'x'},
+/*CHECK*/     {'x','+','+','+','x',' ','x',' ',' ','x',   'x'},
+/*RENAME*/    {'x','x','x','x','x',' ','x','x','x','x',   'x'}
 };
 
 static int inverse_for_options[NUMBER_OF_OPT] =

@@ -186,7 +186,8 @@ static int inverse_for_options[NUMBER_OF
 /* -x */ 0,
 /* -i */ IP6T_INV_VIA_IN,
 /* -o */ IP6T_INV_VIA_OUT,
-/*--line*/ 0
+/*--line*/ 0,
+/* -c */ 0,
 };
 
 const char *program_version;
iptables.c +17 -16
diff --exclude-from=/tmp/srcdiff.excludes.NOfFZH -Np -ur orig/iptables-1.3.1/iptables.c iptables-1.3.1-fix/iptables.c
@@ -101,7 +101,7 @@ static const char cmdflags[] = { 'I', 'D
 #define OPT_COUNTERS	0x00800U
 #define NUMBER_OF_OPT	12
 static const char optflags[NUMBER_OF_OPT]
-= { 'n', 's', 'd', 'p', 'j', 'v', 'x', 'i', 'o', 'f', '3', 'c'};
+= { 'n', 's', 'd', 'p', 'j', 'v', 'x', 'i', 'o', 'f', '0', 'c'};
 
 static struct option original_opts[] = {
 	{ "append", 1, 0, 'A' },

@@ -158,20 +158,20 @@ static unsigned int global_option_offset
 static char commands_v_options[NUMBER_OF_CMD][NUMBER_OF_OPT] =
 /* Well, it's better than "Re: Linux vs FreeBSD" */
 {
-	/*     -n  -s  -d  -p  -j  -v  -x  -i  -o  -f  --line */
-/*INSERT*/    {'x',' ',' ',' ',' ',' ','x',' ',' ',' ','x'},
-/*DELETE*/    {'x',' ',' ',' ',' ',' ','x',' ',' ',' ','x'},
-/*DELETE_NUM*/{'x','x','x','x','x',' ','x','x','x','x','x'},
-/*REPLACE*/   {'x',' ',' ',' ',' ',' ','x',' ',' ',' ','x'},
-/*APPEND*/    {'x',' ',' ',' ',' ',' ','x',' ',' ',' ','x'},
-/*LIST*/      {' ','x','x','x','x',' ',' ','x','x','x',' '},
-/*FLUSH*/     {'x','x','x','x','x',' ','x','x','x','x','x'},
-/*ZERO*/      {'x','x','x','x','x',' ','x','x','x','x','x'},
-/*NEW_CHAIN*/ {'x','x','x','x','x',' ','x','x','x','x','x'},
-/*DEL_CHAIN*/ {'x','x','x','x','x',' ','x','x','x','x','x'},
-/*SET_POLICY*/{'x','x','x','x','x',' ','x','x','x','x','x'},
-/*CHECK*/     {'x','+','+','+','x',' ','x',' ',' ',' ','x'},
-/*RENAME*/    {'x','x','x','x','x',' ','x','x','x','x','x'}
+	/*     -n  -s  -d  -p  -j  -v  -x  -i  -o  -f  --line -c */
+/*INSERT*/    {'x',' ',' ',' ',' ',' ','x',' ',' ',' ','x',   ' '},
+/*DELETE*/    {'x',' ',' ',' ',' ',' ','x',' ',' ',' ','x',   'x'},
+/*DELETE_NUM*/{'x','x','x','x','x',' ','x','x','x','x','x',   'x'},
+/*REPLACE*/   {'x',' ',' ',' ',' ',' ','x',' ',' ',' ','x',   'x'},
+/*APPEND*/    {'x',' ',' ',' ',' ',' ','x',' ',' ',' ','x',   ' '},
+/*LIST*/      {' ','x','x','x','x',' ',' ','x','x','x',' ',   'x'},
+/*FLUSH*/     {'x','x','x','x','x',' ','x','x','x','x','x',   'x'},
+/*ZERO*/      {'x','x','x','x','x',' ','x','x','x','x','x',   'x'},
+/*NEW_CHAIN*/ {'x','x','x','x','x',' ','x','x','x','x','x',   'x'},
+/*DEL_CHAIN*/ {'x','x','x','x','x',' ','x','x','x','x','x',   'x'},
+/*SET_POLICY*/{'x','x','x','x','x',' ','x','x','x','x','x',   'x'},
+/*CHECK*/     {'x','+','+','+','x',' ','x',' ',' ',' ','x',   'x'},
+/*RENAME*/    {'x','x','x','x','x',' ','x','x','x','x','x',   'x'}
 };
 
 static int inverse_for_options[NUMBER_OF_OPT] =

@@ -186,7 +186,8 @@ static int inverse_for_options[NUMBER_OF
 /* -i */ IPT_INV_VIA_IN,
 /* -o */ IPT_INV_VIA_OUT,
 /* -f */ IPT_INV_FRAG,
-/*--line*/ 0
+/*--line*/ 0,
+/* -c */ 0,
 };
 
 const char *program_version;
#1 lore raw
Quoted 2 lines · Jonas Berlin

1. the optflags array contains a '3' for the OPT_LINENUMBERS entry while everywhere else '0' is used. Thus the fix changes it

committed that change, thanks.

Quoted 7 lines

2. the commands_v_options two-dimensional array lacks a column for the -c flag. I tried to deduce feasible values, please look at them before accepting :)

3. the inverse_for_options array lacks a row for the -c flag. By looking at the code I figured it does not allow the "!" inversion option, so I constructed the line accordingly.

erm, do you find something that actually implements CMD_CHECK?

Signature · 5 lines
- Harald Welte <[email protected]>                 http://netfilter.org/
============================================================================
  "Fragmentation is like classful addressing -- an interesting early
   architectural error that shows how much experimentation was going
   on while IP was being designed."                    -- Paul Vixie
author
#2 lore raw

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

Hello, sorry for my late reply..

Quoting Harald Welte on 2005-04-01 06:35 UTC:

|> 2. the commands_v_options two-dimensional array lacks a column for the
|> -c flag. I tried to deduce feasible values, please look at them before
|> accepting :)
|>
|> 3. the inverse_for_options array lacks a row for the -c flag. By
|> looking at the code I figured it does not allow the "!" inversion
|> option, so I constructed the line accordingly.
|
|
| erm, do you find something that actually implements CMD_CHECK?

No I didn't..

What would you like me to do? Should I improve my patch to take out CMD_CHECK and adjust the bit masks accordingly? Unless I got something wrong, my fix 3 above wasn't related to CMD_CHECK in any way..?

- --
- - xkr47
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.1 (GNU/Linux)

iD8DBQFCWGbKxyF48ZTvn+4RAhlOAJ4mSLXoXP7EGUhgi12YgSYPxgrPjgCcCTiC c22FIgr6OvJYk2Jb4XpyNgo=
=XTlA
-----END PGP SIGNATURE-----

#3 lore raw
Quoted 13 lines · Jonas Berlin

Quoting Harald Welte on 2005-04-01 06:35 UTC:

|> 2. the commands_v_options two-dimensional array lacks a column for the
|> -c flag. I tried to deduce feasible values, please look at them before
|> accepting :)
|>
|> 3. the inverse_for_options array lacks a row for the -c flag. By
|> looking at the code I figured it does not allow the "!" inversion
|> option, so I constructed the line accordingly.
|
|
| erm, do you find something that actually implements CMD_CHECK?
Quoted 4 lines

No I didn't..

What would you like me to do? Should I improve my patch to take out CMD_CHECK and adjust the bit masks accordingly?

just remove CMD_CHECK and anything relating to it.

Quoted 2 lines

Unless I got something
wrong, my fix 3 above wasn't related to CMD_CHECK in any way..?

sorry, my mistake.

Signature · 5 lines
- Harald Welte <[email protected]>                 http://netfilter.org/
============================================================================
  "Fragmentation is like classful addressing -- an interesting early
   architectural error that shows how much experimentation was going
   on while IP was being designed."                    -- Paul Vixie
author
#4 lore raw

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

Quoting Harald Welte on 2005-04-10 20:43 UTC:

New patch includes fixes 2, 3 and 4 (since you already applied fix 1):

|>|> 2. the commands_v_options two-dimensional array lacks a column for the
|>|> -c flag. I tried to deduce feasible values, please look at them before
|>|> accepting :)

for the record; the values were deduced from the commandline help texts.

|>|> 3. the inverse_for_options array lacks a row for the -c flag. By
|>|> looking at the code I figured it does not allow the "!" inversion
|>|> option, so I constructed the line accordingly.

| just remove CMD_CHECK and anything relating to it.

4. Removed CMD_CHECK; updated bitmasks and commands_v_options accordingly

- --
- - xkr47
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.1 (GNU/Linux)

iD8DBQFCWlHOxyF48ZTvn+4RAo1XAKC3O1J8qQ6jEenNy/OGrgrEo4CJ3gCdGhsM qhya1K1gwxoc8Lw1+p6zTqs=
=ctxE
-----END PGP SIGNATURE-----

ip6tables.c +19 -20
diff --exclude-from=/tmp/srcdiff.excludes.AIs1cV -Np -ur orig-iptables-1.3.1/ip6tables.c iptables-1.3.1/ip6tables.c
@@ -81,11 +81,10 @@
 #define CMD_NEW_CHAIN		0x0100U
 #define CMD_DELETE_CHAIN	0x0200U
 #define CMD_SET_POLICY		0x0400U
-#define CMD_CHECK		0x0800U
-#define CMD_RENAME_CHAIN	0x1000U
-#define NUMBER_OF_CMD	13
-static const char cmdflags[] = { 'I', 'D', 'D', 'R', 'A', 'L', 'F', 'Z',
-				 'N', 'X', 'P', 'E' };
+#define CMD_RENAME_CHAIN	0x0800U
+#define NUMBER_OF_CMD	12
+static const char cmdflags[NUMBER_OF_CMD]
+= { 'I', 'D', 'D', 'R', 'A', 'L', 'F', 'Z', 'N', 'X', 'P', 'E' };
 
 #define OPTION_OFFSET 256
 

@@ -159,20 +158,19 @@ static unsigned int global_option_offset
 static char commands_v_options[NUMBER_OF_CMD][NUMBER_OF_OPT] =
 /* Well, it's better than "Re: Linux vs FreeBSD" */
 {
-	/*     -n  -s  -d  -p  -j  -v  -x  -i  -o  --line */
-/*INSERT*/    {'x',' ',' ',' ',' ',' ','x',' ',' ','x'},
-/*DELETE*/    {'x',' ',' ',' ',' ',' ','x',' ',' ','x'},
-/*DELETE_NUM*/{'x','x','x','x','x',' ','x','x','x','x'},
-/*REPLACE*/   {'x',' ',' ',' ',' ',' ','x',' ',' ','x'},
-/*APPEND*/    {'x',' ',' ',' ',' ',' ','x',' ',' ','x'},
-/*LIST*/      {' ','x','x','x','x',' ',' ','x','x',' '},
-/*FLUSH*/     {'x','x','x','x','x',' ','x','x','x','x'},
-/*ZERO*/      {'x','x','x','x','x',' ','x','x','x','x'},
-/*NEW_CHAIN*/ {'x','x','x','x','x',' ','x','x','x','x'},
-/*DEL_CHAIN*/ {'x','x','x','x','x',' ','x','x','x','x'},
-/*SET_POLICY*/{'x','x','x','x','x',' ','x','x','x','x'},
-/*CHECK*/     {'x','+','+','+','x',' ','x',' ',' ','x'},
-/*RENAME*/    {'x','x','x','x','x',' ','x','x','x','x'}
+	/*     -n  -s  -d  -p  -j  -v  -x  -i  -o  --line -c */
+/*INSERT*/    {'x',' ',' ',' ',' ',' ','x',' ',' ','x',   ' '},
+/*DELETE*/    {'x',' ',' ',' ',' ',' ','x',' ',' ','x',   'x'},
+/*DELETE_NUM*/{'x','x','x','x','x',' ','x','x','x','x',   'x'},
+/*REPLACE*/   {'x',' ',' ',' ',' ',' ','x',' ',' ','x',   'x'},
+/*APPEND*/    {'x',' ',' ',' ',' ',' ','x',' ',' ','x',   ' '},
+/*LIST*/      {' ','x','x','x','x',' ',' ','x','x',' ',   'x'},
+/*FLUSH*/     {'x','x','x','x','x',' ','x','x','x','x',   'x'},
+/*ZERO*/      {'x','x','x','x','x',' ','x','x','x','x',   'x'},
+/*NEW_CHAIN*/ {'x','x','x','x','x',' ','x','x','x','x',   'x'},
+/*DEL_CHAIN*/ {'x','x','x','x','x',' ','x','x','x','x',   'x'},
+/*SET_POLICY*/{'x','x','x','x','x',' ','x','x','x','x',   'x'},
+/*RENAME*/    {'x','x','x','x','x',' ','x','x','x','x',   'x'}
 };
 
 static int inverse_for_options[NUMBER_OF_OPT] =

@@ -186,7 +184,8 @@ static int inverse_for_options[NUMBER_OF
 /* -x */ 0,
 /* -i */ IP6T_INV_VIA_IN,
 /* -o */ IP6T_INV_VIA_OUT,
-/*--line*/ 0
+/*--line*/ 0,
+/* -c */ 0,
 };
 
 const char *program_version;
iptables.c +19 -20
diff --exclude-from=/tmp/srcdiff.excludes.AIs1cV -Np -ur orig-iptables-1.3.1/iptables.c iptables-1.3.1/iptables.c
@@ -78,11 +78,10 @@
 #define CMD_NEW_CHAIN		0x0100U
 #define CMD_DELETE_CHAIN	0x0200U
 #define CMD_SET_POLICY		0x0400U
-#define CMD_CHECK		0x0800U
-#define CMD_RENAME_CHAIN	0x1000U
-#define NUMBER_OF_CMD	13
-static const char cmdflags[] = { 'I', 'D', 'D', 'R', 'A', 'L', 'F', 'Z',
-				 'N', 'X', 'P', 'E' };
+#define CMD_RENAME_CHAIN	0x0800U
+#define NUMBER_OF_CMD	12
+static const char cmdflags[NUMBER_OF_CMD]
+= { 'I', 'D', 'D', 'R', 'A', 'L', 'F', 'Z', 'N', 'X', 'P', 'E' };
 
 #define OPTION_OFFSET 256
 

@@ -158,20 +157,19 @@ static unsigned int global_option_offset
 static char commands_v_options[NUMBER_OF_CMD][NUMBER_OF_OPT] =
 /* Well, it's better than "Re: Linux vs FreeBSD" */
 {
-	/*     -n  -s  -d  -p  -j  -v  -x  -i  -o  -f  --line */
-/*INSERT*/    {'x',' ',' ',' ',' ',' ','x',' ',' ',' ','x'},
-/*DELETE*/    {'x',' ',' ',' ',' ',' ','x',' ',' ',' ','x'},
-/*DELETE_NUM*/{'x','x','x','x','x',' ','x','x','x','x','x'},
-/*REPLACE*/   {'x',' ',' ',' ',' ',' ','x',' ',' ',' ','x'},
-/*APPEND*/    {'x',' ',' ',' ',' ',' ','x',' ',' ',' ','x'},
-/*LIST*/      {' ','x','x','x','x',' ',' ','x','x','x',' '},
-/*FLUSH*/     {'x','x','x','x','x',' ','x','x','x','x','x'},
-/*ZERO*/      {'x','x','x','x','x',' ','x','x','x','x','x'},
-/*NEW_CHAIN*/ {'x','x','x','x','x',' ','x','x','x','x','x'},
-/*DEL_CHAIN*/ {'x','x','x','x','x',' ','x','x','x','x','x'},
-/*SET_POLICY*/{'x','x','x','x','x',' ','x','x','x','x','x'},
-/*CHECK*/     {'x','+','+','+','x',' ','x',' ',' ',' ','x'},
-/*RENAME*/    {'x','x','x','x','x',' ','x','x','x','x','x'}
+	/*     -n  -s  -d  -p  -j  -v  -x  -i  -o  -f  --line -c */
+/*INSERT*/    {'x',' ',' ',' ',' ',' ','x',' ',' ',' ','x',   ' '},
+/*DELETE*/    {'x',' ',' ',' ',' ',' ','x',' ',' ',' ','x',   'x'},
+/*DELETE_NUM*/{'x','x','x','x','x',' ','x','x','x','x','x',   'x'},
+/*REPLACE*/   {'x',' ',' ',' ',' ',' ','x',' ',' ',' ','x',   'x'},
+/*APPEND*/    {'x',' ',' ',' ',' ',' ','x',' ',' ',' ','x',   ' '},
+/*LIST*/      {' ','x','x','x','x',' ',' ','x','x','x',' ',   'x'},
+/*FLUSH*/     {'x','x','x','x','x',' ','x','x','x','x','x',   'x'},
+/*ZERO*/      {'x','x','x','x','x',' ','x','x','x','x','x',   'x'},
+/*NEW_CHAIN*/ {'x','x','x','x','x',' ','x','x','x','x','x',   'x'},
+/*DEL_CHAIN*/ {'x','x','x','x','x',' ','x','x','x','x','x',   'x'},
+/*SET_POLICY*/{'x','x','x','x','x',' ','x','x','x','x','x',   'x'},
+/*RENAME*/    {'x','x','x','x','x',' ','x','x','x','x','x',   'x'}
 };
 
 static int inverse_for_options[NUMBER_OF_OPT] =

@@ -186,7 +184,8 @@ static int inverse_for_options[NUMBER_OF
 /* -i */ IPT_INV_VIA_IN,
 /* -o */ IPT_INV_VIA_OUT,
 /* -f */ IPT_INV_FRAG,
-/*--line*/ 0
+/*--line*/ 0,
+/* -c */ 0,
 };
 
 const char *program_version;