Commit 0d7df906 authored by Florian Westphal's avatar Florian Westphal Committed by Pablo Neira Ayuso

netfilter: x_tables: ensure last rule in base chain matches underflow/policy

Harmless from kernel point of view, but again iptables assumes that
this is true when decoding ruleset coming from kernel.

If a (syzkaller generated) ruleset doesn't have the underflow/policy
stored as the last rule in the base chain, then iptables will abort()
because it doesn't find the chain policy.

libiptc assumes that the policy is the last rule in the basechain, which
is only true for iptables-generated rulesets.

Unfortunately this needs code duplication -- the functions need the
struct layout of the rule head, but that is different for
ip/ip6/arptables.

NB: pr_warn could be pr_debug but in case this break rulesets somehow its
useful to know why blob was rejected.
Signed-off-by: default avatarFlorian Westphal <fw@strlen.de>
Signed-off-by: default avatarPablo Neira Ayuso <pablo@netfilter.org>
parent 89370860
...@@ -309,10 +309,13 @@ static int mark_source_chains(const struct xt_table_info *newinfo, ...@@ -309,10 +309,13 @@ static int mark_source_chains(const struct xt_table_info *newinfo,
for (hook = 0; hook < NF_ARP_NUMHOOKS; hook++) { for (hook = 0; hook < NF_ARP_NUMHOOKS; hook++) {
unsigned int pos = newinfo->hook_entry[hook]; unsigned int pos = newinfo->hook_entry[hook];
struct arpt_entry *e = entry0 + pos; struct arpt_entry *e = entry0 + pos;
unsigned int last_pos, depth;
if (!(valid_hooks & (1 << hook))) if (!(valid_hooks & (1 << hook)))
continue; continue;
depth = 0;
last_pos = pos;
/* Set initial back pointer. */ /* Set initial back pointer. */
e->counters.pcnt = pos; e->counters.pcnt = pos;
...@@ -343,6 +346,8 @@ static int mark_source_chains(const struct xt_table_info *newinfo, ...@@ -343,6 +346,8 @@ static int mark_source_chains(const struct xt_table_info *newinfo,
pos = e->counters.pcnt; pos = e->counters.pcnt;
e->counters.pcnt = 0; e->counters.pcnt = 0;
if (depth)
--depth;
/* We're at the start. */ /* We're at the start. */
if (pos == oldpos) if (pos == oldpos)
goto next; goto next;
...@@ -367,6 +372,9 @@ static int mark_source_chains(const struct xt_table_info *newinfo, ...@@ -367,6 +372,9 @@ static int mark_source_chains(const struct xt_table_info *newinfo,
if (!xt_find_jump_offset(offsets, newpos, if (!xt_find_jump_offset(offsets, newpos,
newinfo->number)) newinfo->number))
return 0; return 0;
if (entry0 + newpos != arpt_next_entry(e))
++depth;
} else { } else {
/* ... this is a fallthru */ /* ... this is a fallthru */
newpos = pos + e->next_offset; newpos = pos + e->next_offset;
...@@ -377,8 +385,15 @@ static int mark_source_chains(const struct xt_table_info *newinfo, ...@@ -377,8 +385,15 @@ static int mark_source_chains(const struct xt_table_info *newinfo,
e->counters.pcnt = pos; e->counters.pcnt = pos;
pos = newpos; 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; return 1;
} }
......
...@@ -378,10 +378,13 @@ mark_source_chains(const struct xt_table_info *newinfo, ...@@ -378,10 +378,13 @@ mark_source_chains(const struct xt_table_info *newinfo,
for (hook = 0; hook < NF_INET_NUMHOOKS; hook++) { for (hook = 0; hook < NF_INET_NUMHOOKS; hook++) {
unsigned int pos = newinfo->hook_entry[hook]; unsigned int pos = newinfo->hook_entry[hook];
struct ipt_entry *e = entry0 + pos; struct ipt_entry *e = entry0 + pos;
unsigned int last_pos, depth;
if (!(valid_hooks & (1 << hook))) if (!(valid_hooks & (1 << hook)))
continue; continue;
depth = 0;
last_pos = pos;
/* Set initial back pointer. */ /* Set initial back pointer. */
e->counters.pcnt = pos; e->counters.pcnt = pos;
...@@ -410,6 +413,8 @@ mark_source_chains(const struct xt_table_info *newinfo, ...@@ -410,6 +413,8 @@ mark_source_chains(const struct xt_table_info *newinfo,
pos = e->counters.pcnt; pos = e->counters.pcnt;
e->counters.pcnt = 0; e->counters.pcnt = 0;
if (depth)
--depth;
/* We're at the start. */ /* We're at the start. */
if (pos == oldpos) if (pos == oldpos)
goto next; goto next;
...@@ -434,6 +439,9 @@ mark_source_chains(const struct xt_table_info *newinfo, ...@@ -434,6 +439,9 @@ mark_source_chains(const struct xt_table_info *newinfo,
if (!xt_find_jump_offset(offsets, newpos, if (!xt_find_jump_offset(offsets, newpos,
newinfo->number)) newinfo->number))
return 0; return 0;
if (entry0 + newpos != ipt_next_entry(e))
++depth;
} else { } else {
/* ... this is a fallthru */ /* ... this is a fallthru */
newpos = pos + e->next_offset; newpos = pos + e->next_offset;
...@@ -444,8 +452,15 @@ mark_source_chains(const struct xt_table_info *newinfo, ...@@ -444,8 +452,15 @@ mark_source_chains(const struct xt_table_info *newinfo,
e->counters.pcnt = pos; e->counters.pcnt = pos;
pos = newpos; 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; return 1;
} }
......
...@@ -396,10 +396,13 @@ mark_source_chains(const struct xt_table_info *newinfo, ...@@ -396,10 +396,13 @@ mark_source_chains(const struct xt_table_info *newinfo,
for (hook = 0; hook < NF_INET_NUMHOOKS; hook++) { for (hook = 0; hook < NF_INET_NUMHOOKS; hook++) {
unsigned int pos = newinfo->hook_entry[hook]; unsigned int pos = newinfo->hook_entry[hook];
struct ip6t_entry *e = entry0 + pos; struct ip6t_entry *e = entry0 + pos;
unsigned int last_pos, depth;
if (!(valid_hooks & (1 << hook))) if (!(valid_hooks & (1 << hook)))
continue; continue;
depth = 0;
last_pos = pos;
/* Set initial back pointer. */ /* Set initial back pointer. */
e->counters.pcnt = pos; e->counters.pcnt = pos;
...@@ -428,6 +431,8 @@ mark_source_chains(const struct xt_table_info *newinfo, ...@@ -428,6 +431,8 @@ mark_source_chains(const struct xt_table_info *newinfo,
pos = e->counters.pcnt; pos = e->counters.pcnt;
e->counters.pcnt = 0; e->counters.pcnt = 0;
if (depth)
--depth;
/* We're at the start. */ /* We're at the start. */
if (pos == oldpos) if (pos == oldpos)
goto next; goto next;
...@@ -452,6 +457,9 @@ mark_source_chains(const struct xt_table_info *newinfo, ...@@ -452,6 +457,9 @@ mark_source_chains(const struct xt_table_info *newinfo,
if (!xt_find_jump_offset(offsets, newpos, if (!xt_find_jump_offset(offsets, newpos,
newinfo->number)) newinfo->number))
return 0; return 0;
if (entry0 + newpos != ip6t_next_entry(e))
++depth;
} else { } else {
/* ... this is a fallthru */ /* ... this is a fallthru */
newpos = pos + e->next_offset; newpos = pos + e->next_offset;
...@@ -462,8 +470,15 @@ mark_source_chains(const struct xt_table_info *newinfo, ...@@ -462,8 +470,15 @@ mark_source_chains(const struct xt_table_info *newinfo,
e->counters.pcnt = pos; e->counters.pcnt = pos;
pos = newpos; 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; return 1;
} }
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment