diff options
author | Florian Westphal <fw@strlen.de> | 2018-03-30 05:39:12 -0400 |
---|---|---|
committer | Pablo Neira Ayuso <pablo@netfilter.org> | 2018-03-30 06:20:32 -0400 |
commit | e3b5e1ec75234fb6b27708a316cdf69f9fb176a8 (patch) | |
tree | 3a09869c582d612e08c626d94a0c67fd6742e11d | |
parent | 9ba5c404bf1d6284f0269411b33394362b7ff405 (diff) |
Revert "netfilter: x_tables: ensure last rule in base chain matches underflow/policy"
This reverts commit 0d7df906a0e78079a02108b06d32c3ef2238ad25.
Valdis Kletnieks reported that xtables is broken in linux-next since
0d7df906a0e78 ("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: 0d7df906a0e78 ("netfilter: x_tables: ensure last rule in base chain matches underflow/policy")
Reported-by: Valdis Kletnieks <valdis.kletnieks@vt.edu>
Signed-off-by: Florian Westphal <fw@strlen.de>
Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
-rw-r--r-- | net/ipv4/netfilter/arp_tables.c | 17 | ||||
-rw-r--r-- | net/ipv4/netfilter/ip_tables.c | 17 | ||||
-rw-r--r-- | net/ipv6/netfilter/ip6_tables.c | 17 |
3 files changed, 3 insertions, 48 deletions
diff --git a/net/ipv4/netfilter/arp_tables.c b/net/ipv4/netfilter/arp_tables.c index f366ff1cfc19..aaafdbd15ad3 100644 --- a/net/ipv4/netfilter/arp_tables.c +++ b/net/ipv4/netfilter/arp_tables.c | |||
@@ -309,13 +309,10 @@ static int mark_source_chains(const struct xt_table_info *newinfo, | |||
309 | for (hook = 0; hook < NF_ARP_NUMHOOKS; hook++) { | 309 | for (hook = 0; hook < NF_ARP_NUMHOOKS; hook++) { |
310 | unsigned int pos = newinfo->hook_entry[hook]; | 310 | unsigned int pos = newinfo->hook_entry[hook]; |
311 | struct arpt_entry *e = entry0 + pos; | 311 | struct arpt_entry *e = entry0 + pos; |
312 | unsigned int last_pos, depth; | ||
313 | 312 | ||
314 | if (!(valid_hooks & (1 << hook))) | 313 | if (!(valid_hooks & (1 << hook))) |
315 | continue; | 314 | continue; |
316 | 315 | ||
317 | depth = 0; | ||
318 | last_pos = pos; | ||
319 | /* Set initial back pointer. */ | 316 | /* Set initial back pointer. */ |
320 | e->counters.pcnt = pos; | 317 | e->counters.pcnt = pos; |
321 | 318 | ||
@@ -346,8 +343,6 @@ static int mark_source_chains(const struct xt_table_info *newinfo, | |||
346 | pos = e->counters.pcnt; | 343 | pos = e->counters.pcnt; |
347 | e->counters.pcnt = 0; | 344 | e->counters.pcnt = 0; |
348 | 345 | ||
349 | if (depth) | ||
350 | --depth; | ||
351 | /* We're at the start. */ | 346 | /* We're at the start. */ |
352 | if (pos == oldpos) | 347 | if (pos == oldpos) |
353 | goto next; | 348 | goto next; |
@@ -372,9 +367,6 @@ static int mark_source_chains(const struct xt_table_info *newinfo, | |||
372 | if (!xt_find_jump_offset(offsets, newpos, | 367 | if (!xt_find_jump_offset(offsets, newpos, |
373 | newinfo->number)) | 368 | newinfo->number)) |
374 | return 0; | 369 | return 0; |
375 | |||
376 | if (entry0 + newpos != arpt_next_entry(e)) | ||
377 | ++depth; | ||
378 | } else { | 370 | } else { |
379 | /* ... this is a fallthru */ | 371 | /* ... this is a fallthru */ |
380 | newpos = pos + e->next_offset; | 372 | newpos = pos + e->next_offset; |
@@ -385,15 +377,8 @@ static int mark_source_chains(const struct xt_table_info *newinfo, | |||
385 | e->counters.pcnt = pos; | 377 | e->counters.pcnt = pos; |
386 | pos = newpos; | 378 | pos = newpos; |
387 | } | 379 | } |
388 | if (depth == 0) | ||
389 | last_pos = pos; | ||
390 | } | ||
391 | next: | ||
392 | if (last_pos != newinfo->underflow[hook]) { | ||
393 | pr_err_ratelimited("last base chain position %u doesn't match underflow %u (hook %u)\n", | ||
394 | last_pos, newinfo->underflow[hook], hook); | ||
395 | return 0; | ||
396 | } | 380 | } |
381 | next: ; | ||
397 | } | 382 | } |
398 | return 1; | 383 | return 1; |
399 | } | 384 | } |
diff --git a/net/ipv4/netfilter/ip_tables.c b/net/ipv4/netfilter/ip_tables.c index 2362ca2c9e0c..f9063513f9d1 100644 --- a/net/ipv4/netfilter/ip_tables.c +++ b/net/ipv4/netfilter/ip_tables.c | |||
@@ -378,13 +378,10 @@ mark_source_chains(const struct xt_table_info *newinfo, | |||
378 | for (hook = 0; hook < NF_INET_NUMHOOKS; hook++) { | 378 | for (hook = 0; hook < NF_INET_NUMHOOKS; hook++) { |
379 | unsigned int pos = newinfo->hook_entry[hook]; | 379 | unsigned int pos = newinfo->hook_entry[hook]; |
380 | struct ipt_entry *e = entry0 + pos; | 380 | struct ipt_entry *e = entry0 + pos; |
381 | unsigned int last_pos, depth; | ||
382 | 381 | ||
383 | if (!(valid_hooks & (1 << hook))) | 382 | if (!(valid_hooks & (1 << hook))) |
384 | continue; | 383 | continue; |
385 | 384 | ||
386 | depth = 0; | ||
387 | last_pos = pos; | ||
388 | /* Set initial back pointer. */ | 385 | /* Set initial back pointer. */ |
389 | e->counters.pcnt = pos; | 386 | e->counters.pcnt = pos; |
390 | 387 | ||
@@ -413,8 +410,6 @@ mark_source_chains(const struct xt_table_info *newinfo, | |||
413 | pos = e->counters.pcnt; | 410 | pos = e->counters.pcnt; |
414 | e->counters.pcnt = 0; | 411 | e->counters.pcnt = 0; |
415 | 412 | ||
416 | if (depth) | ||
417 | --depth; | ||
418 | /* We're at the start. */ | 413 | /* We're at the start. */ |
419 | if (pos == oldpos) | 414 | if (pos == oldpos) |
420 | goto next; | 415 | goto next; |
@@ -439,9 +434,6 @@ mark_source_chains(const struct xt_table_info *newinfo, | |||
439 | if (!xt_find_jump_offset(offsets, newpos, | 434 | if (!xt_find_jump_offset(offsets, newpos, |
440 | newinfo->number)) | 435 | newinfo->number)) |
441 | return 0; | 436 | return 0; |
442 | |||
443 | if (entry0 + newpos != ipt_next_entry(e)) | ||
444 | ++depth; | ||
445 | } else { | 437 | } else { |
446 | /* ... this is a fallthru */ | 438 | /* ... this is a fallthru */ |
447 | newpos = pos + e->next_offset; | 439 | newpos = pos + e->next_offset; |
@@ -452,15 +444,8 @@ mark_source_chains(const struct xt_table_info *newinfo, | |||
452 | e->counters.pcnt = pos; | 444 | e->counters.pcnt = pos; |
453 | pos = newpos; | 445 | pos = newpos; |
454 | } | 446 | } |
455 | if (depth == 0) | ||
456 | last_pos = pos; | ||
457 | } | ||
458 | next: | ||
459 | if (last_pos != newinfo->underflow[hook]) { | ||
460 | pr_err_ratelimited("last base chain position %u doesn't match underflow %u (hook %u)\n", | ||
461 | last_pos, newinfo->underflow[hook], hook); | ||
462 | return 0; | ||
463 | } | 447 | } |
448 | next: ; | ||
464 | } | 449 | } |
465 | return 1; | 450 | return 1; |
466 | } | 451 | } |
diff --git a/net/ipv6/netfilter/ip6_tables.c b/net/ipv6/netfilter/ip6_tables.c index 004508753abc..3c36a4c77f29 100644 --- a/net/ipv6/netfilter/ip6_tables.c +++ b/net/ipv6/netfilter/ip6_tables.c | |||
@@ -396,13 +396,10 @@ mark_source_chains(const struct xt_table_info *newinfo, | |||
396 | for (hook = 0; hook < NF_INET_NUMHOOKS; hook++) { | 396 | for (hook = 0; hook < NF_INET_NUMHOOKS; hook++) { |
397 | unsigned int pos = newinfo->hook_entry[hook]; | 397 | unsigned int pos = newinfo->hook_entry[hook]; |
398 | struct ip6t_entry *e = entry0 + pos; | 398 | struct ip6t_entry *e = entry0 + pos; |
399 | unsigned int last_pos, depth; | ||
400 | 399 | ||
401 | if (!(valid_hooks & (1 << hook))) | 400 | if (!(valid_hooks & (1 << hook))) |
402 | continue; | 401 | continue; |
403 | 402 | ||
404 | depth = 0; | ||
405 | last_pos = pos; | ||
406 | /* Set initial back pointer. */ | 403 | /* Set initial back pointer. */ |
407 | e->counters.pcnt = pos; | 404 | e->counters.pcnt = pos; |
408 | 405 | ||
@@ -431,8 +428,6 @@ mark_source_chains(const struct xt_table_info *newinfo, | |||
431 | pos = e->counters.pcnt; | 428 | pos = e->counters.pcnt; |
432 | e->counters.pcnt = 0; | 429 | e->counters.pcnt = 0; |
433 | 430 | ||
434 | if (depth) | ||
435 | --depth; | ||
436 | /* We're at the start. */ | 431 | /* We're at the start. */ |
437 | if (pos == oldpos) | 432 | if (pos == oldpos) |
438 | goto next; | 433 | goto next; |
@@ -457,9 +452,6 @@ mark_source_chains(const struct xt_table_info *newinfo, | |||
457 | if (!xt_find_jump_offset(offsets, newpos, | 452 | if (!xt_find_jump_offset(offsets, newpos, |
458 | newinfo->number)) | 453 | newinfo->number)) |
459 | return 0; | 454 | return 0; |
460 | |||
461 | if (entry0 + newpos != ip6t_next_entry(e)) | ||
462 | ++depth; | ||
463 | } else { | 455 | } else { |
464 | /* ... this is a fallthru */ | 456 | /* ... this is a fallthru */ |
465 | newpos = pos + e->next_offset; | 457 | newpos = pos + e->next_offset; |
@@ -470,15 +462,8 @@ mark_source_chains(const struct xt_table_info *newinfo, | |||
470 | e->counters.pcnt = pos; | 462 | e->counters.pcnt = pos; |
471 | pos = newpos; | 463 | pos = newpos; |
472 | } | 464 | } |
473 | if (depth == 0) | ||
474 | last_pos = pos; | ||
475 | } | ||
476 | next: | ||
477 | if (last_pos != newinfo->underflow[hook]) { | ||
478 | pr_err_ratelimited("last base chain position %u doesn't match underflow %u (hook %u)\n", | ||
479 | last_pos, newinfo->underflow[hook], hook); | ||
480 | return 0; | ||
481 | } | 465 | } |
466 | next: ; | ||
482 | } | 467 | } |
483 | return 1; | 468 | return 1; |
484 | } | 469 | } |