Skip to content

Commit

Permalink
Revert "netfilter: x_tables: ensure last rule in base chain matches u…
Browse files Browse the repository at this point in the history 
…nderflow/policy"

This reverts commit 0d7df90.

Valdis Kletnieks reported that xtables is broken in linux-next since
0d7df90  ("netfilter: x_tables: ensure last rule in base chain
matches underflow/policy"), as kernel rejects the (well-formed) ruleset:

[   64.402790] ip6_tables: last base chain position 1136 doesn't match underflow 1344 (hook 1)

mark_source_chains is not the correct place for such a check, as it
terminates evaluation of a chain once it sees an unconditional verdict
(following rules are known to be unreachable). It seems preferrable to
fix libiptc instead, so remove this check again.

Fixes: 0d7df90 ("netfilter: x_tables: ensure last rule in base chain matches underflow/policy")
Reported-by: Valdis Kletnieks <[email protected]>
Signed-off-by: Florian Westphal <[email protected]>
Signed-off-by: Pablo Neira Ayuso <[email protected]>
  • Loading branch information
@ummakynes
Florian Westphal authored and ummakynes committed Mar 30, 2018
1 parent 9ba5c40 commit e3b5e1e
Show file tree
Hide file tree
Showing 3 changed files with 3 additions and 48 deletions.
17 changes: 1 addition & 16 deletions net/ipv4/netfilter/arp_tables.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -309,13 +309,10 @@ static int mark_source_chains(const struct xt_table_info *newinfo,
for (hook = 0; hook < NF_ARP_NUMHOOKS; hook++) {
unsigned int pos = newinfo->hook_entry[hook];
struct arpt_entry *e = entry0 + pos;
unsigned int last_pos, depth;

if (!(valid_hooks & (1 << hook)))
continue;

depth = 0;
last_pos = pos;
/* Set initial back pointer. */
e->counters.pcnt = pos;

Expand Down Expand Up @@ -346,8 +343,6 @@ static int mark_source_chains(const struct xt_table_info *newinfo,
pos = e->counters.pcnt;
e->counters.pcnt = 0;

if (depth)
--depth;
/* We're at the start. */
if (pos == oldpos)
goto next;
Expand All @@ -372,9 +367,6 @@ static int mark_source_chains(const struct xt_table_info *newinfo,
if (!xt_find_jump_offset(offsets, newpos,
newinfo->number))
return 0;

if (entry0 + newpos != arpt_next_entry(e))
++depth;
} else {
/* ... this is a fallthru */
newpos = pos + e->next_offset;
Expand All @@ -385,15 +377,8 @@ static int mark_source_chains(const struct xt_table_info *newinfo,
e->counters.pcnt = pos;
pos = newpos;
}
if (depth == 0)
last_pos = pos;
}
next:
if (last_pos != newinfo->underflow[hook]) {
pr_err_ratelimited("last base chain position %u doesn't match underflow %u (hook %u)\n",
last_pos, newinfo->underflow[hook], hook);
return 0;
}
next: ;
}
return 1;
}
Expand Down
17 changes: 1 addition & 16 deletions net/ipv4/netfilter/ip_tables.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -378,13 +378,10 @@ mark_source_chains(const struct xt_table_info *newinfo,
for (hook = 0; hook < NF_INET_NUMHOOKS; hook++) {
unsigned int pos = newinfo->hook_entry[hook];
struct ipt_entry *e = entry0 + pos;
unsigned int last_pos, depth;

if (!(valid_hooks & (1 << hook)))
continue;

depth = 0;
last_pos = pos;
/* Set initial back pointer. */
e->counters.pcnt = pos;

Expand Down Expand Up @@ -413,8 +410,6 @@ mark_source_chains(const struct xt_table_info *newinfo,
pos = e->counters.pcnt;
e->counters.pcnt = 0;

if (depth)
--depth;
/* We're at the start. */
if (pos == oldpos)
goto next;
Expand All @@ -439,9 +434,6 @@ mark_source_chains(const struct xt_table_info *newinfo,
if (!xt_find_jump_offset(offsets, newpos,
newinfo->number))
return 0;

if (entry0 + newpos != ipt_next_entry(e))
++depth;
} else {
/* ... this is a fallthru */
newpos = pos + e->next_offset;
Expand All @@ -452,15 +444,8 @@ mark_source_chains(const struct xt_table_info *newinfo,
e->counters.pcnt = pos;
pos = newpos;
}
if (depth == 0)
last_pos = pos;
}
next:
if (last_pos != newinfo->underflow[hook]) {
pr_err_ratelimited("last base chain position %u doesn't match underflow %u (hook %u)\n",
last_pos, newinfo->underflow[hook], hook);
return 0;
}
next: ;
}
return 1;
}
Expand Down
17 changes: 1 addition & 16 deletions net/ipv6/netfilter/ip6_tables.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -396,13 +396,10 @@ mark_source_chains(const struct xt_table_info *newinfo,
for (hook = 0; hook < NF_INET_NUMHOOKS; hook++) {
unsigned int pos = newinfo->hook_entry[hook];
struct ip6t_entry *e = entry0 + pos;
unsigned int last_pos, depth;

if (!(valid_hooks & (1 << hook)))
continue;

depth = 0;
last_pos = pos;
/* Set initial back pointer. */
e->counters.pcnt = pos;

Expand Down Expand Up @@ -431,8 +428,6 @@ mark_source_chains(const struct xt_table_info *newinfo,
pos = e->counters.pcnt;
e->counters.pcnt = 0;

if (depth)
--depth;
/* We're at the start. */
if (pos == oldpos)
goto next;
Expand All @@ -457,9 +452,6 @@ mark_source_chains(const struct xt_table_info *newinfo,
if (!xt_find_jump_offset(offsets, newpos,
newinfo->number))
return 0;

if (entry0 + newpos != ip6t_next_entry(e))
++depth;
} else {
/* ... this is a fallthru */
newpos = pos + e->next_offset;
Expand All @@ -470,15 +462,8 @@ mark_source_chains(const struct xt_table_info *newinfo,
e->counters.pcnt = pos;
pos = newpos;
}
if (depth == 0)
last_pos = pos;
}
next:
if (last_pos != newinfo->underflow[hook]) {
pr_err_ratelimited("last base chain position %u doesn't match underflow %u (hook %u)\n",
last_pos, newinfo->underflow[hook], hook);
return 0;
}
next: ;
}
return 1;
}
Expand Down

0 comments on commit e3b5e1e

Please sign in to comment.