Skip to content

Commit b763bd2

Browse files
Florian Westphalgregkh
authored andcommitted
netfilter: ebtables: remove BUGPRINT messages
commit d824548 upstream. 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 <[email protected]> Signed-off-by: Pablo Neira Ayuso <[email protected]> Signed-off-by: Greg Kroah-Hartman <[email protected]>
1 parent 4f160d8 commit b763bd2

File tree

1 file changed

+39
-92
lines changed

1 file changed

+39
-92
lines changed

net/bridge/netfilter/ebtables.c

Lines changed: 39 additions & 92 deletions
Original file line numberDiff line numberDiff line change
@@ -31,10 +31,6 @@
3131
/* needed for logical [in,out]-dev filtering */
3232
#include "../br_private.h"
3333

34-
#define BUGPRINT(format, args...) printk("kernel msg: ebtables bug: please "\
35-
"report to author: "format, ## args)
36-
/* #define BUGPRINT(format, args...) */
37-
3834
/* Each cpu has its own set of counters, so there is no need for write_lock in
3935
* the softirq
4036
* For reading or updating the counters, the user context needs to
@@ -453,8 +449,6 @@ static int ebt_verify_pointers(const struct ebt_replace *repl,
453449
/* we make userspace set this right,
454450
* so there is no misunderstanding
455451
*/
456-
BUGPRINT("EBT_ENTRY_OR_ENTRIES shouldn't be set "
457-
"in distinguisher\n");
458452
return -EINVAL;
459453
}
460454
if (i != NF_BR_NUMHOOKS)
@@ -472,18 +466,14 @@ static int ebt_verify_pointers(const struct ebt_replace *repl,
472466
offset += e->next_offset;
473467
}
474468
}
475-
if (offset != limit) {
476-
BUGPRINT("entries_size too small\n");
469+
if (offset != limit)
477470
return -EINVAL;
478-
}
479471

480472
/* check if all valid hooks have a chain */
481473
for (i = 0; i < NF_BR_NUMHOOKS; i++) {
482474
if (!newinfo->hook_entry[i] &&
483-
(valid_hooks & (1 << i))) {
484-
BUGPRINT("Valid hook without chain\n");
475+
(valid_hooks & (1 << i)))
485476
return -EINVAL;
486-
}
487477
}
488478
return 0;
489479
}
@@ -510,42 +500,34 @@ ebt_check_entry_size_and_hooks(const struct ebt_entry *e,
510500
/* this checks if the previous chain has as many entries
511501
* as it said it has
512502
*/
513-
if (*n != *cnt) {
514-
BUGPRINT("nentries does not equal the nr of entries "
515-
"in the chain\n");
503+
if (*n != *cnt)
516504
return -EINVAL;
517-
}
505+
518506
if (((struct ebt_entries *)e)->policy != EBT_DROP &&
519507
((struct ebt_entries *)e)->policy != EBT_ACCEPT) {
520508
/* only RETURN from udc */
521509
if (i != NF_BR_NUMHOOKS ||
522-
((struct ebt_entries *)e)->policy != EBT_RETURN) {
523-
BUGPRINT("bad policy\n");
510+
((struct ebt_entries *)e)->policy != EBT_RETURN)
524511
return -EINVAL;
525-
}
526512
}
527513
if (i == NF_BR_NUMHOOKS) /* it's a user defined chain */
528514
(*udc_cnt)++;
529-
if (((struct ebt_entries *)e)->counter_offset != *totalcnt) {
530-
BUGPRINT("counter_offset != totalcnt");
515+
if (((struct ebt_entries *)e)->counter_offset != *totalcnt)
531516
return -EINVAL;
532-
}
533517
*n = ((struct ebt_entries *)e)->nentries;
534518
*cnt = 0;
535519
return 0;
536520
}
537521
/* a plain old entry, heh */
538522
if (sizeof(struct ebt_entry) > e->watchers_offset ||
539523
e->watchers_offset > e->target_offset ||
540-
e->target_offset >= e->next_offset) {
541-
BUGPRINT("entry offsets not in right order\n");
524+
e->target_offset >= e->next_offset)
542525
return -EINVAL;
543-
}
526+
544527
/* this is not checked anywhere else */
545-
if (e->next_offset - e->target_offset < sizeof(struct ebt_entry_target)) {
546-
BUGPRINT("target size too small\n");
528+
if (e->next_offset - e->target_offset < sizeof(struct ebt_entry_target))
547529
return -EINVAL;
548-
}
530+
549531
(*cnt)++;
550532
(*totalcnt)++;
551533
return 0;
@@ -665,18 +647,15 @@ ebt_check_entry(struct ebt_entry *e, struct net *net,
665647
if (e->bitmask == 0)
666648
return 0;
667649

668-
if (e->bitmask & ~EBT_F_MASK) {
669-
BUGPRINT("Unknown flag for bitmask\n");
650+
if (e->bitmask & ~EBT_F_MASK)
670651
return -EINVAL;
671-
}
672-
if (e->invflags & ~EBT_INV_MASK) {
673-
BUGPRINT("Unknown flag for inv bitmask\n");
652+
653+
if (e->invflags & ~EBT_INV_MASK)
674654
return -EINVAL;
675-
}
676-
if ((e->bitmask & EBT_NOPROTO) && (e->bitmask & EBT_802_3)) {
677-
BUGPRINT("NOPROTO & 802_3 not allowed\n");
655+
656+
if ((e->bitmask & EBT_NOPROTO) && (e->bitmask & EBT_802_3))
678657
return -EINVAL;
679-
}
658+
680659
/* what hook do we belong to? */
681660
for (i = 0; i < NF_BR_NUMHOOKS; i++) {
682661
if (!newinfo->hook_entry[i])
@@ -735,13 +714,11 @@ ebt_check_entry(struct ebt_entry *e, struct net *net,
735714
t->u.target = target;
736715
if (t->u.target == &ebt_standard_target) {
737716
if (gap < sizeof(struct ebt_standard_target)) {
738-
BUGPRINT("Standard target size too big\n");
739717
ret = -EFAULT;
740718
goto cleanup_watchers;
741719
}
742720
if (((struct ebt_standard_target *)t)->verdict <
743721
-NUM_STANDARD_TARGETS) {
744-
BUGPRINT("Invalid standard target\n");
745722
ret = -EFAULT;
746723
goto cleanup_watchers;
747724
}
@@ -801,10 +778,9 @@ static int check_chainloops(const struct ebt_entries *chain, struct ebt_cl_stack
801778
if (strcmp(t->u.name, EBT_STANDARD_TARGET))
802779
goto letscontinue;
803780
if (e->target_offset + sizeof(struct ebt_standard_target) >
804-
e->next_offset) {
805-
BUGPRINT("Standard target size too big\n");
781+
e->next_offset)
806782
return -1;
807-
}
783+
808784
verdict = ((struct ebt_standard_target *)t)->verdict;
809785
if (verdict >= 0) { /* jump to another chain */
810786
struct ebt_entries *hlp2 =
@@ -813,14 +789,12 @@ static int check_chainloops(const struct ebt_entries *chain, struct ebt_cl_stack
813789
if (hlp2 == cl_s[i].cs.chaininfo)
814790
break;
815791
/* bad destination or loop */
816-
if (i == udc_cnt) {
817-
BUGPRINT("bad destination\n");
792+
if (i == udc_cnt)
818793
return -1;
819-
}
820-
if (cl_s[i].cs.n) {
821-
BUGPRINT("loop\n");
794+
795+
if (cl_s[i].cs.n)
822796
return -1;
823-
}
797+
824798
if (cl_s[i].hookmask & (1 << hooknr))
825799
goto letscontinue;
826800
/* this can't be 0, so the loop test is correct */
@@ -853,24 +827,21 @@ static int translate_table(struct net *net, const char *name,
853827
i = 0;
854828
while (i < NF_BR_NUMHOOKS && !newinfo->hook_entry[i])
855829
i++;
856-
if (i == NF_BR_NUMHOOKS) {
857-
BUGPRINT("No valid hooks specified\n");
830+
if (i == NF_BR_NUMHOOKS)
858831
return -EINVAL;
859-
}
860-
if (newinfo->hook_entry[i] != (struct ebt_entries *)newinfo->entries) {
861-
BUGPRINT("Chains don't start at beginning\n");
832+
833+
if (newinfo->hook_entry[i] != (struct ebt_entries *)newinfo->entries)
862834
return -EINVAL;
863-
}
835+
864836
/* make sure chains are ordered after each other in same order
865837
* as their corresponding hooks
866838
*/
867839
for (j = i + 1; j < NF_BR_NUMHOOKS; j++) {
868840
if (!newinfo->hook_entry[j])
869841
continue;
870-
if (newinfo->hook_entry[j] <= newinfo->hook_entry[i]) {
871-
BUGPRINT("Hook order must be followed\n");
842+
if (newinfo->hook_entry[j] <= newinfo->hook_entry[i])
872843
return -EINVAL;
873-
}
844+
874845
i = j;
875846
}
876847

@@ -888,15 +859,11 @@ static int translate_table(struct net *net, const char *name,
888859
if (ret != 0)
889860
return ret;
890861

891-
if (i != j) {
892-
BUGPRINT("nentries does not equal the nr of entries in the "
893-
"(last) chain\n");
862+
if (i != j)
894863
return -EINVAL;
895-
}
896-
if (k != newinfo->nentries) {
897-
BUGPRINT("Total nentries is wrong\n");
864+
865+
if (k != newinfo->nentries)
898866
return -EINVAL;
899-
}
900867

901868
/* get the location of the udc, put them in an array
902869
* while we're at it, allocate the chainstack
@@ -929,7 +896,6 @@ static int translate_table(struct net *net, const char *name,
929896
ebt_get_udc_positions, newinfo, &i, cl_s);
930897
/* sanity check */
931898
if (i != udc_cnt) {
932-
BUGPRINT("i != udc_cnt\n");
933899
vfree(cl_s);
934900
return -EFAULT;
935901
}
@@ -1030,7 +996,6 @@ static int do_replace_finish(struct net *net, struct ebt_replace *repl,
1030996
goto free_unlock;
1031997

1032998
if (repl->num_counters && repl->num_counters != t->private->nentries) {
1033-
BUGPRINT("Wrong nr. of counters requested\n");
1034999
ret = -EINVAL;
10351000
goto free_unlock;
10361001
}
@@ -1115,15 +1080,12 @@ static int do_replace(struct net *net, const void __user *user,
11151080
if (copy_from_user(&tmp, user, sizeof(tmp)) != 0)
11161081
return -EFAULT;
11171082

1118-
if (len != sizeof(tmp) + tmp.entries_size) {
1119-
BUGPRINT("Wrong len argument\n");
1083+
if (len != sizeof(tmp) + tmp.entries_size)
11201084
return -EINVAL;
1121-
}
11221085

1123-
if (tmp.entries_size == 0) {
1124-
BUGPRINT("Entries_size never zero\n");
1086+
if (tmp.entries_size == 0)
11251087
return -EINVAL;
1126-
}
1088+
11271089
/* overflow check */
11281090
if (tmp.nentries >= ((INT_MAX - sizeof(struct ebt_table_info)) /
11291091
NR_CPUS - SMP_CACHE_BYTES) / sizeof(struct ebt_counter))
@@ -1150,7 +1112,6 @@ static int do_replace(struct net *net, const void __user *user,
11501112
}
11511113
if (copy_from_user(
11521114
newinfo->entries, tmp.entries, tmp.entries_size) != 0) {
1153-
BUGPRINT("Couldn't copy entries from userspace\n");
11541115
ret = -EFAULT;
11551116
goto free_entries;
11561117
}
@@ -1197,10 +1158,8 @@ int ebt_register_table(struct net *net, const struct ebt_table *input_table,
11971158

11981159
if (input_table == NULL || (repl = input_table->table) == NULL ||
11991160
repl->entries == NULL || repl->entries_size == 0 ||
1200-
repl->counters != NULL || input_table->private != NULL) {
1201-
BUGPRINT("Bad table data for ebt_register_table!!!\n");
1161+
repl->counters != NULL || input_table->private != NULL)
12021162
return -EINVAL;
1203-
}
12041163

12051164
/* Don't add one table to multiple lists. */
12061165
table = kmemdup(input_table, sizeof(struct ebt_table), GFP_KERNEL);
@@ -1238,13 +1197,10 @@ int ebt_register_table(struct net *net, const struct ebt_table *input_table,
12381197
((char *)repl->hook_entry[i] - repl->entries);
12391198
}
12401199
ret = translate_table(net, repl->name, newinfo);
1241-
if (ret != 0) {
1242-
BUGPRINT("Translate_table failed\n");
1200+
if (ret != 0)
12431201
goto free_chainstack;
1244-
}
12451202

12461203
if (table->check && table->check(newinfo, table->valid_hooks)) {
1247-
BUGPRINT("The table doesn't like its own initial data, lol\n");
12481204
ret = -EINVAL;
12491205
goto free_chainstack;
12501206
}
@@ -1255,7 +1211,6 @@ int ebt_register_table(struct net *net, const struct ebt_table *input_table,
12551211
list_for_each_entry(t, &net->xt.tables[NFPROTO_BRIDGE], list) {
12561212
if (strcmp(t->name, table->name) == 0) {
12571213
ret = -EEXIST;
1258-
BUGPRINT("Table name already exists\n");
12591214
goto free_unlock;
12601215
}
12611216
}
@@ -1327,7 +1282,6 @@ static int do_update_counters(struct net *net, const char *name,
13271282
goto free_tmp;
13281283

13291284
if (num_counters != t->private->nentries) {
1330-
BUGPRINT("Wrong nr of counters\n");
13311285
ret = -EINVAL;
13321286
goto unlock_mutex;
13331287
}
@@ -1452,10 +1406,8 @@ static int copy_counters_to_user(struct ebt_table *t,
14521406
if (num_counters == 0)
14531407
return 0;
14541408

1455-
if (num_counters != nentries) {
1456-
BUGPRINT("Num_counters wrong\n");
1409+
if (num_counters != nentries)
14571410
return -EINVAL;
1458-
}
14591411

14601412
counterstmp = vmalloc(nentries * sizeof(*counterstmp));
14611413
if (!counterstmp)
@@ -1501,15 +1453,11 @@ static int copy_everything_to_user(struct ebt_table *t, void __user *user,
15011453
(tmp.num_counters ? nentries * sizeof(struct ebt_counter) : 0))
15021454
return -EINVAL;
15031455

1504-
if (tmp.nentries != nentries) {
1505-
BUGPRINT("Nentries wrong\n");
1456+
if (tmp.nentries != nentries)
15061457
return -EINVAL;
1507-
}
15081458

1509-
if (tmp.entries_size != entries_size) {
1510-
BUGPRINT("Wrong size\n");
1459+
if (tmp.entries_size != entries_size)
15111460
return -EINVAL;
1512-
}
15131461

15141462
ret = copy_counters_to_user(t, oldcounters, tmp.counters,
15151463
tmp.num_counters, nentries);
@@ -1581,7 +1529,6 @@ static int do_ebt_get_ctl(struct sock *sk, int cmd, void __user *user, int *len)
15811529
}
15821530
mutex_unlock(&ebt_mutex);
15831531
if (copy_to_user(user, &tmp, *len) != 0) {
1584-
BUGPRINT("c2u Didn't work\n");
15851532
ret = -EFAULT;
15861533
break;
15871534
}

0 commit comments

Comments
 (0)