Fix CONNMARK mask value demolition
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
@@ -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 );
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
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.
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
@@ -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
Patrick McHardy
AppliedQuoted 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 theinit()function.
Applied, thanks Peter.