Skip to contentSkip to conversation

Fix CONNMARK mask value demolition

PATCH Applied Peter Warasin 2 participants 5 messages Jan 6, 2008 – Jan 15, 2008 netfilter

lore
Share & tools
lore
#0 lore raw

PATCH 1/1 Fix CONNMARK mask value demolition

This patch fixes the problem that the CONNMARK mask value has been set to 0 whenever the CONNMARK target options has not been the last options to be processed.

---

extensions/libxt_CONNMARK.c +1 -2
Index: iptables/extensions/libxt_CONNMARK.c
===================================================================
@@ -64,13 +64,12 @@
 	struct xt_connmark_target_info *markinfo
 		= (struct xt_connmark_target_info *)(*target)->data;
 
-	markinfo->mask = 0xffffffffUL;
-
 	switch (c) {
 		char *end;
 	case '1':
 		markinfo->mode = XT_CONNMARK_SET;
 
+		markinfo->mask = 0xffffffffUL;
 		markinfo->mark = strtoul(optarg, &end, 0);
 		if (*end == '/' && end[1] != '\0')
 		    markinfo->mask = strtoul(end+1, &end, 0);
#1 lore raw

Hi

I would not like to create the impression trying to do any pressure on this topic but i recognized that my patch description maybe was a little bit poor.
In order to make sure the patch is understood correctly, here the bug report, of which that patch is the fix:

If you use the CONNMARK target, it makes a difference if you have the -j option as last option or before the matches.

iptables -t mangle -N test

This works:
iptables -v -t mangle -I test -m connmark --mark 7 -j CONNMARK --set-mark 0x7/0xf
CONNMARK all opt -- in * out * 0.0.0.0/0 -> 0.0.0.0/0 CONNMARK match 0x7 CONNMARK set 0x7/0xf

This not:
iptables -v -t mangle -I test -j CONNMARK --set-mark 0x7/0xf -m connmark --mark 7
CONNMARK all opt -- in * out * 0.0.0.0/0 -> 0.0.0.0/0 CONNMARK match 0x7 CONNMARK set 0x7

In the second call, the CONNMARK mask (0xf) will be eaten.

peter

Signature · 5 lines
:: e n d i a n
:: open source - open minds

:: peter warasin
:: http://www.endian.com   :: [email protected]

begin:vcard
fn:Peter Warasin
n:;Peter Warasin
org:Endian GmbH/Srl
adr:;;Pillhof 47;Frangart/Frangarto;BZ;I-39010;Italien/Italia email;internet:[email protected]
tel;work:+39 0471 631763
tel;fax:+39 0471 631764
x-mozilla-html:FALSE
url:http://www.endian.com
version:2.1
end:vcard

#2 lore raw
In reply to Peter Warasin
Quoted 6 lines · Peter Warasin

This patch fixes the problem that the CONNMARK mask value has been set to 0 whenever the CONNMARK target options has not been the last options to be processed.

extensions/libxt_CONNMARK.c +0 -0
@@ -64,13 +64,12 @@
 	struct xt_connmark_target_info *markinfo

extensions/libxt_CONNMARK.c line 72

Quoted 10 lines
extensions/libxt_CONNMARK.c +1 -2
 		= (struct xt_connmark_target_info *)(*target)->data;
 
-	markinfo->mask = 0xffffffffUL;
-
 	switch (c) {
 		char *end;
 	case '1':
 		markinfo->mode = XT_CONNMARK_SET;
 
+		markinfo->mask = 0xffffffffUL;

Good catch, but don't we also need set the mask for --save-mark and --restore-mark? I would suggest to move the initialzation to a ->init() function.

#3 lore raw

Hi Patrick

Quoted 3 lines · Patrick McHardy

Good catch, but don't we also need set the mask for --save-mark and --restore-mark? I would suggest to move the initialzation to a ->init() function.

Oh, yes. That's necessary.
Here's the revised patch with the init() function.

peter

Signature · 5 lines
:: e n d i a n
:: open source - open minds

:: peter warasin
:: http://www.endian.com   :: [email protected]

Fix CONNMARK mask initialisation

This patch fixes the problem that the CONNMARK mask value has been set to 0 whenever the CONNMARK target options were not the last options to be processed.
It initalizes the mask value rather than setting it for each parse.

---
 extensions/libxt_CONNMARK.c |   12 ++++++++++--
 1 file changed, 10 insertions(+), 2 deletions(-)
extensions/libxt_CONNMARK.c +10 -2
Index: iptables/extensions/libxt_CONNMARK.c
===================================================================
@@ -64,8 +64,6 @@
 	struct xt_connmark_target_info *markinfo
 		= (struct xt_connmark_target_info *)(*target)->data;
 
-	markinfo->mask = 0xffffffffUL;
-
 	switch (c) {
 		char *end;
 	case '1':
@@ -188,6 +186,14 @@
 	}
 }
 
+static void CONNMARK_init(struct xt_entry_target *t)
+{
+	struct xt_connmark_target_info *markinfo
+		= (struct xt_connmark_target_info *)t->data;
+
+	markinfo->mask = 0xffffffffUL;
+}
+
 static struct xtables_target connmark_target = {
 	.family		= AF_INET,
 	.name		= "CONNMARK",
@@ -195,6 +201,7 @@
 	.size		= XT_ALIGN(sizeof(struct xt_connmark_target_info)),
 	.userspacesize	= XT_ALIGN(sizeof(struct xt_connmark_target_info)),
 	.help		= CONNMARK_help,
+	.init           = CONNMARK_init,
 	.parse		= CONNMARK_parse,
 	.final_check	= CONNMARK_check,
 	.print		= CONNMARK_print,
@@ -209,6 +216,7 @@
 	.size		= XT_ALIGN(sizeof(struct xt_connmark_target_info)),
 	.userspacesize	= XT_ALIGN(sizeof(struct xt_connmark_target_info)),
 	.help		= CONNMARK_help,
+	.init           = CONNMARK_init,
 	.parse		= CONNMARK_parse,
 	.final_check	= CONNMARK_check,
 	.print		= CONNMARK_print,

begin:vcard
fn:Peter Warasin
n:;Peter Warasin
org:Endian GmbH/Srl
adr:;;Pillhof 47;Frangart/Frangarto;BZ;I-39010;Italien/Italia email;internet:[email protected]
tel;work:+39 0471 631763
tel;fax:+39 0471 631764
x-mozilla-html:FALSE
url:http://www.endian.com
version:2.1
end:vcard

#4 lore raw
Quoted 9 lines · Peter Warasin

Hi Patrick

Quoted 3 lines · Patrick McHardy

Good catch, but don't we also need set the mask for --save-mark and --restore-mark? I would suggest to move the initialzation to a ->init() function.

Oh, yes. That's necessary.
Here's the revised patch with the init() function.

Applied, thanks Peter.