netfilter: ebtables: remove BUGPRINT messages

They are however frequently triggered by syzkaller, so remove them.

ebtables userspace should never trigger any of these, so there is little
value in making them pr_debug (or ratelimited).

Signed-off-by: Florian Westphal <fw@strlen.de>
Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
This commit is contained in:
Florian Westphal 2019-02-19 00:37:21 +01:00 committed by Pablo Neira Ayuso
parent 4283428e49
commit d824548dae

View file

@ -31,10 +31,6 @@
/* needed for logical [in,out]-dev filtering */ /* needed for logical [in,out]-dev filtering */
#include "../br_private.h" #include "../br_private.h"
#define BUGPRINT(format, args...) printk("kernel msg: ebtables bug: please "\
"report to author: "format, ## args)
/* #define BUGPRINT(format, args...) */
/* Each cpu has its own set of counters, so there is no need for write_lock in /* Each cpu has its own set of counters, so there is no need for write_lock in
* the softirq * the softirq
* For reading or updating the counters, the user context needs to * For reading or updating the counters, the user context needs to
@ -466,8 +462,6 @@ static int ebt_verify_pointers(const struct ebt_replace *repl,
/* we make userspace set this right, /* we make userspace set this right,
* so there is no misunderstanding * so there is no misunderstanding
*/ */
BUGPRINT("EBT_ENTRY_OR_ENTRIES shouldn't be set "
"in distinguisher\n");
return -EINVAL; return -EINVAL;
} }
if (i != NF_BR_NUMHOOKS) if (i != NF_BR_NUMHOOKS)
@ -485,18 +479,14 @@ static int ebt_verify_pointers(const struct ebt_replace *repl,
offset += e->next_offset; offset += e->next_offset;
} }
} }
if (offset != limit) { if (offset != limit)
BUGPRINT("entries_size too small\n");
return -EINVAL; return -EINVAL;
}
/* check if all valid hooks have a chain */ /* check if all valid hooks have a chain */
for (i = 0; i < NF_BR_NUMHOOKS; i++) { for (i = 0; i < NF_BR_NUMHOOKS; i++) {
if (!newinfo->hook_entry[i] && if (!newinfo->hook_entry[i] &&
(valid_hooks & (1 << i))) { (valid_hooks & (1 << i)))
BUGPRINT("Valid hook without chain\n");
return -EINVAL; return -EINVAL;
}
} }
return 0; return 0;
} }
@ -523,26 +513,20 @@ ebt_check_entry_size_and_hooks(const struct ebt_entry *e,
/* this checks if the previous chain has as many entries /* this checks if the previous chain has as many entries
* as it said it has * as it said it has
*/ */
if (*n != *cnt) { if (*n != *cnt)
BUGPRINT("nentries does not equal the nr of entries "
"in the chain\n");
return -EINVAL; return -EINVAL;
}
if (((struct ebt_entries *)e)->policy != EBT_DROP && if (((struct ebt_entries *)e)->policy != EBT_DROP &&
((struct ebt_entries *)e)->policy != EBT_ACCEPT) { ((struct ebt_entries *)e)->policy != EBT_ACCEPT) {
/* only RETURN from udc */ /* only RETURN from udc */
if (i != NF_BR_NUMHOOKS || if (i != NF_BR_NUMHOOKS ||
((struct ebt_entries *)e)->policy != EBT_RETURN) { ((struct ebt_entries *)e)->policy != EBT_RETURN)
BUGPRINT("bad policy\n");
return -EINVAL; return -EINVAL;
}
} }
if (i == NF_BR_NUMHOOKS) /* it's a user defined chain */ if (i == NF_BR_NUMHOOKS) /* it's a user defined chain */
(*udc_cnt)++; (*udc_cnt)++;
if (((struct ebt_entries *)e)->counter_offset != *totalcnt) { if (((struct ebt_entries *)e)->counter_offset != *totalcnt)
BUGPRINT("counter_offset != totalcnt");
return -EINVAL; return -EINVAL;
}
*n = ((struct ebt_entries *)e)->nentries; *n = ((struct ebt_entries *)e)->nentries;
*cnt = 0; *cnt = 0;
return 0; return 0;
@ -550,15 +534,13 @@ ebt_check_entry_size_and_hooks(const struct ebt_entry *e,
/* a plain old entry, heh */ /* a plain old entry, heh */
if (sizeof(struct ebt_entry) > e->watchers_offset || if (sizeof(struct ebt_entry) > e->watchers_offset ||
e->watchers_offset > e->target_offset || e->watchers_offset > e->target_offset ||
e->target_offset >= e->next_offset) { e->target_offset >= e->next_offset)
BUGPRINT("entry offsets not in right order\n");
return -EINVAL; return -EINVAL;
}
/* this is not checked anywhere else */ /* this is not checked anywhere else */
if (e->next_offset - e->target_offset < sizeof(struct ebt_entry_target)) { if (e->next_offset - e->target_offset < sizeof(struct ebt_entry_target))
BUGPRINT("target size too small\n");
return -EINVAL; return -EINVAL;
}
(*cnt)++; (*cnt)++;
(*totalcnt)++; (*totalcnt)++;
return 0; return 0;
@ -678,18 +660,15 @@ ebt_check_entry(struct ebt_entry *e, struct net *net,
if (e->bitmask == 0) if (e->bitmask == 0)
return 0; return 0;
if (e->bitmask & ~EBT_F_MASK) { if (e->bitmask & ~EBT_F_MASK)
BUGPRINT("Unknown flag for bitmask\n");
return -EINVAL; return -EINVAL;
}
if (e->invflags & ~EBT_INV_MASK) { if (e->invflags & ~EBT_INV_MASK)
BUGPRINT("Unknown flag for inv bitmask\n");
return -EINVAL; return -EINVAL;
}
if ((e->bitmask & EBT_NOPROTO) && (e->bitmask & EBT_802_3)) { if ((e->bitmask & EBT_NOPROTO) && (e->bitmask & EBT_802_3))
BUGPRINT("NOPROTO & 802_3 not allowed\n");
return -EINVAL; return -EINVAL;
}
/* what hook do we belong to? */ /* what hook do we belong to? */
for (i = 0; i < NF_BR_NUMHOOKS; i++) { for (i = 0; i < NF_BR_NUMHOOKS; i++) {
if (!newinfo->hook_entry[i]) if (!newinfo->hook_entry[i])
@ -748,13 +727,11 @@ ebt_check_entry(struct ebt_entry *e, struct net *net,
t->u.target = target; t->u.target = target;
if (t->u.target == &ebt_standard_target) { if (t->u.target == &ebt_standard_target) {
if (gap < sizeof(struct ebt_standard_target)) { if (gap < sizeof(struct ebt_standard_target)) {
BUGPRINT("Standard target size too big\n");
ret = -EFAULT; ret = -EFAULT;
goto cleanup_watchers; goto cleanup_watchers;
} }
if (((struct ebt_standard_target *)t)->verdict < if (((struct ebt_standard_target *)t)->verdict <
-NUM_STANDARD_TARGETS) { -NUM_STANDARD_TARGETS) {
BUGPRINT("Invalid standard target\n");
ret = -EFAULT; ret = -EFAULT;
goto cleanup_watchers; goto cleanup_watchers;
} }
@ -813,10 +790,9 @@ static int check_chainloops(const struct ebt_entries *chain, struct ebt_cl_stack
if (strcmp(t->u.name, EBT_STANDARD_TARGET)) if (strcmp(t->u.name, EBT_STANDARD_TARGET))
goto letscontinue; goto letscontinue;
if (e->target_offset + sizeof(struct ebt_standard_target) > if (e->target_offset + sizeof(struct ebt_standard_target) >
e->next_offset) { e->next_offset)
BUGPRINT("Standard target size too big\n");
return -1; return -1;
}
verdict = ((struct ebt_standard_target *)t)->verdict; verdict = ((struct ebt_standard_target *)t)->verdict;
if (verdict >= 0) { /* jump to another chain */ if (verdict >= 0) { /* jump to another chain */
struct ebt_entries *hlp2 = struct ebt_entries *hlp2 =
@ -825,14 +801,12 @@ static int check_chainloops(const struct ebt_entries *chain, struct ebt_cl_stack
if (hlp2 == cl_s[i].cs.chaininfo) if (hlp2 == cl_s[i].cs.chaininfo)
break; break;
/* bad destination or loop */ /* bad destination or loop */
if (i == udc_cnt) { if (i == udc_cnt)
BUGPRINT("bad destination\n");
return -1; return -1;
}
if (cl_s[i].cs.n) { if (cl_s[i].cs.n)
BUGPRINT("loop\n");
return -1; return -1;
}
if (cl_s[i].hookmask & (1 << hooknr)) if (cl_s[i].hookmask & (1 << hooknr))
goto letscontinue; goto letscontinue;
/* this can't be 0, so the loop test is correct */ /* this can't be 0, so the loop test is correct */
@ -865,24 +839,21 @@ static int translate_table(struct net *net, const char *name,
i = 0; i = 0;
while (i < NF_BR_NUMHOOKS && !newinfo->hook_entry[i]) while (i < NF_BR_NUMHOOKS && !newinfo->hook_entry[i])
i++; i++;
if (i == NF_BR_NUMHOOKS) { if (i == NF_BR_NUMHOOKS)
BUGPRINT("No valid hooks specified\n");
return -EINVAL; return -EINVAL;
}
if (newinfo->hook_entry[i] != (struct ebt_entries *)newinfo->entries) { if (newinfo->hook_entry[i] != (struct ebt_entries *)newinfo->entries)
BUGPRINT("Chains don't start at beginning\n");
return -EINVAL; return -EINVAL;
}
/* make sure chains are ordered after each other in same order /* make sure chains are ordered after each other in same order
* as their corresponding hooks * as their corresponding hooks
*/ */
for (j = i + 1; j < NF_BR_NUMHOOKS; j++) { for (j = i + 1; j < NF_BR_NUMHOOKS; j++) {
if (!newinfo->hook_entry[j]) if (!newinfo->hook_entry[j])
continue; continue;
if (newinfo->hook_entry[j] <= newinfo->hook_entry[i]) { if (newinfo->hook_entry[j] <= newinfo->hook_entry[i])
BUGPRINT("Hook order must be followed\n");
return -EINVAL; return -EINVAL;
}
i = j; i = j;
} }
@ -900,15 +871,11 @@ static int translate_table(struct net *net, const char *name,
if (ret != 0) if (ret != 0)
return ret; return ret;
if (i != j) { if (i != j)
BUGPRINT("nentries does not equal the nr of entries in the "
"(last) chain\n");
return -EINVAL; return -EINVAL;
}
if (k != newinfo->nentries) { if (k != newinfo->nentries)
BUGPRINT("Total nentries is wrong\n");
return -EINVAL; return -EINVAL;
}
/* get the location of the udc, put them in an array /* get the location of the udc, put them in an array
* while we're at it, allocate the chainstack * while we're at it, allocate the chainstack
@ -942,7 +909,6 @@ static int translate_table(struct net *net, const char *name,
ebt_get_udc_positions, newinfo, &i, cl_s); ebt_get_udc_positions, newinfo, &i, cl_s);
/* sanity check */ /* sanity check */
if (i != udc_cnt) { if (i != udc_cnt) {
BUGPRINT("i != udc_cnt\n");
vfree(cl_s); vfree(cl_s);
return -EFAULT; return -EFAULT;
} }
@ -1042,7 +1008,6 @@ static int do_replace_finish(struct net *net, struct ebt_replace *repl,
goto free_unlock; goto free_unlock;
if (repl->num_counters && repl->num_counters != t->private->nentries) { if (repl->num_counters && repl->num_counters != t->private->nentries) {
BUGPRINT("Wrong nr. of counters requested\n");
ret = -EINVAL; ret = -EINVAL;
goto free_unlock; goto free_unlock;
} }
@ -1118,15 +1083,12 @@ static int do_replace(struct net *net, const void __user *user,
if (copy_from_user(&tmp, user, sizeof(tmp)) != 0) if (copy_from_user(&tmp, user, sizeof(tmp)) != 0)
return -EFAULT; return -EFAULT;
if (len != sizeof(tmp) + tmp.entries_size) { if (len != sizeof(tmp) + tmp.entries_size)
BUGPRINT("Wrong len argument\n");
return -EINVAL; return -EINVAL;
}
if (tmp.entries_size == 0) { if (tmp.entries_size == 0)
BUGPRINT("Entries_size never zero\n");
return -EINVAL; return -EINVAL;
}
/* overflow check */ /* overflow check */
if (tmp.nentries >= ((INT_MAX - sizeof(struct ebt_table_info)) / if (tmp.nentries >= ((INT_MAX - sizeof(struct ebt_table_info)) /
NR_CPUS - SMP_CACHE_BYTES) / sizeof(struct ebt_counter)) NR_CPUS - SMP_CACHE_BYTES) / sizeof(struct ebt_counter))
@ -1153,7 +1115,6 @@ static int do_replace(struct net *net, const void __user *user,
} }
if (copy_from_user( if (copy_from_user(
newinfo->entries, tmp.entries, tmp.entries_size) != 0) { newinfo->entries, tmp.entries, tmp.entries_size) != 0) {
BUGPRINT("Couldn't copy entries from userspace\n");
ret = -EFAULT; ret = -EFAULT;
goto free_entries; goto free_entries;
} }
@ -1194,10 +1155,8 @@ int ebt_register_table(struct net *net, const struct ebt_table *input_table,
if (input_table == NULL || (repl = input_table->table) == NULL || if (input_table == NULL || (repl = input_table->table) == NULL ||
repl->entries == NULL || repl->entries_size == 0 || repl->entries == NULL || repl->entries_size == 0 ||
repl->counters != NULL || input_table->private != NULL) { repl->counters != NULL || input_table->private != NULL)
BUGPRINT("Bad table data for ebt_register_table!!!\n");
return -EINVAL; return -EINVAL;
}
/* Don't add one table to multiple lists. */ /* Don't add one table to multiple lists. */
table = kmemdup(input_table, sizeof(struct ebt_table), GFP_KERNEL); table = kmemdup(input_table, sizeof(struct ebt_table), GFP_KERNEL);
@ -1235,13 +1194,10 @@ int ebt_register_table(struct net *net, const struct ebt_table *input_table,
((char *)repl->hook_entry[i] - repl->entries); ((char *)repl->hook_entry[i] - repl->entries);
} }
ret = translate_table(net, repl->name, newinfo); ret = translate_table(net, repl->name, newinfo);
if (ret != 0) { if (ret != 0)
BUGPRINT("Translate_table failed\n");
goto free_chainstack; goto free_chainstack;
}
if (table->check && table->check(newinfo, table->valid_hooks)) { if (table->check && table->check(newinfo, table->valid_hooks)) {
BUGPRINT("The table doesn't like its own initial data, lol\n");
ret = -EINVAL; ret = -EINVAL;
goto free_chainstack; goto free_chainstack;
} }
@ -1252,7 +1208,6 @@ int ebt_register_table(struct net *net, const struct ebt_table *input_table,
list_for_each_entry(t, &net->xt.tables[NFPROTO_BRIDGE], list) { list_for_each_entry(t, &net->xt.tables[NFPROTO_BRIDGE], list) {
if (strcmp(t->name, table->name) == 0) { if (strcmp(t->name, table->name) == 0) {
ret = -EEXIST; ret = -EEXIST;
BUGPRINT("Table name already exists\n");
goto free_unlock; goto free_unlock;
} }
} }
@ -1320,7 +1275,6 @@ static int do_update_counters(struct net *net, const char *name,
goto free_tmp; goto free_tmp;
if (num_counters != t->private->nentries) { if (num_counters != t->private->nentries) {
BUGPRINT("Wrong nr of counters\n");
ret = -EINVAL; ret = -EINVAL;
goto unlock_mutex; goto unlock_mutex;
} }
@ -1447,10 +1401,8 @@ static int copy_counters_to_user(struct ebt_table *t,
if (num_counters == 0) if (num_counters == 0)
return 0; return 0;
if (num_counters != nentries) { if (num_counters != nentries)
BUGPRINT("Num_counters wrong\n");
return -EINVAL; return -EINVAL;
}
counterstmp = vmalloc(array_size(nentries, sizeof(*counterstmp))); counterstmp = vmalloc(array_size(nentries, sizeof(*counterstmp)));
if (!counterstmp) if (!counterstmp)
@ -1496,15 +1448,11 @@ static int copy_everything_to_user(struct ebt_table *t, void __user *user,
(tmp.num_counters ? nentries * sizeof(struct ebt_counter) : 0)) (tmp.num_counters ? nentries * sizeof(struct ebt_counter) : 0))
return -EINVAL; return -EINVAL;
if (tmp.nentries != nentries) { if (tmp.nentries != nentries)
BUGPRINT("Nentries wrong\n");
return -EINVAL; return -EINVAL;
}
if (tmp.entries_size != entries_size) { if (tmp.entries_size != entries_size)
BUGPRINT("Wrong size\n");
return -EINVAL; return -EINVAL;
}
ret = copy_counters_to_user(t, oldcounters, tmp.counters, ret = copy_counters_to_user(t, oldcounters, tmp.counters,
tmp.num_counters, nentries); tmp.num_counters, nentries);
@ -1576,7 +1524,6 @@ static int do_ebt_get_ctl(struct sock *sk, int cmd, void __user *user, int *len)
} }
mutex_unlock(&ebt_mutex); mutex_unlock(&ebt_mutex);
if (copy_to_user(user, &tmp, *len) != 0) { if (copy_to_user(user, &tmp, *len) != 0) {
BUGPRINT("c2u Didn't work\n");
ret = -EFAULT; ret = -EFAULT;
break; break;
} }