lkml.org 
[lkml]   [2019]   [Mar]   [26]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    Patch in this message
    /
    From
    Subject[PATCH 4.19 37/45] netfilter: ebtables: remove BUGPRINT messages
    Date
    4.19-stable review patch.  If anyone has any objections, please let me know.

    ------------------

    From: Florian Westphal <fw@strlen.de>

    commit d824548dae220820bdf69b2d1561b7c4b072783f 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 <fw@strlen.de>
    Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
    Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>

    ---
    net/bridge/netfilter/ebtables.c | 131 +++++++++++-----------------------------
    1 file changed, 39 insertions(+), 92 deletions(-)

    --- a/net/bridge/netfilter/ebtables.c
    +++ b/net/bridge/netfilter/ebtables.c
    @@ -31,10 +31,6 @@
    /* needed for logical [in,out]-dev filtering */
    #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
    * the softirq
    * For reading or updating the counters, the user context needs to
    @@ -466,8 +462,6 @@ static int ebt_verify_pointers(const str
    /* we make userspace set this right,
    * so there is no misunderstanding
    */
    - BUGPRINT("EBT_ENTRY_OR_ENTRIES shouldn't be set "
    - "in distinguisher\n");
    return -EINVAL;
    }
    if (i != NF_BR_NUMHOOKS)
    @@ -485,18 +479,14 @@ static int ebt_verify_pointers(const str
    offset += e->next_offset;
    }
    }
    - if (offset != limit) {
    - BUGPRINT("entries_size too small\n");
    + if (offset != limit)
    return -EINVAL;
    - }

    /* check if all valid hooks have a chain */
    for (i = 0; i < NF_BR_NUMHOOKS; i++) {
    if (!newinfo->hook_entry[i] &&
    - (valid_hooks & (1 << i))) {
    - BUGPRINT("Valid hook without chain\n");
    + (valid_hooks & (1 << i)))
    return -EINVAL;
    - }
    }
    return 0;
    }
    @@ -523,26 +513,20 @@ ebt_check_entry_size_and_hooks(const str
    /* this checks if the previous chain has as many entries
    * as it said it has
    */
    - if (*n != *cnt) {
    - BUGPRINT("nentries does not equal the nr of entries "
    - "in the chain\n");
    + if (*n != *cnt)
    return -EINVAL;
    - }
    +
    if (((struct ebt_entries *)e)->policy != EBT_DROP &&
    ((struct ebt_entries *)e)->policy != EBT_ACCEPT) {
    /* only RETURN from udc */
    if (i != NF_BR_NUMHOOKS ||
    - ((struct ebt_entries *)e)->policy != EBT_RETURN) {
    - BUGPRINT("bad policy\n");
    + ((struct ebt_entries *)e)->policy != EBT_RETURN)
    return -EINVAL;
    - }
    }
    if (i == NF_BR_NUMHOOKS) /* it's a user defined chain */
    (*udc_cnt)++;
    - if (((struct ebt_entries *)e)->counter_offset != *totalcnt) {
    - BUGPRINT("counter_offset != totalcnt");
    + if (((struct ebt_entries *)e)->counter_offset != *totalcnt)
    return -EINVAL;
    - }
    *n = ((struct ebt_entries *)e)->nentries;
    *cnt = 0;
    return 0;
    @@ -550,15 +534,13 @@ ebt_check_entry_size_and_hooks(const str
    /* a plain old entry, heh */
    if (sizeof(struct ebt_entry) > e->watchers_offset ||
    e->watchers_offset > e->target_offset ||
    - e->target_offset >= e->next_offset) {
    - BUGPRINT("entry offsets not in right order\n");
    + e->target_offset >= e->next_offset)
    return -EINVAL;
    - }
    +
    /* this is not checked anywhere else */
    - if (e->next_offset - e->target_offset < sizeof(struct ebt_entry_target)) {
    - BUGPRINT("target size too small\n");
    + if (e->next_offset - e->target_offset < sizeof(struct ebt_entry_target))
    return -EINVAL;
    - }
    +
    (*cnt)++;
    (*totalcnt)++;
    return 0;
    @@ -678,18 +660,15 @@ ebt_check_entry(struct ebt_entry *e, str
    if (e->bitmask == 0)
    return 0;

    - if (e->bitmask & ~EBT_F_MASK) {
    - BUGPRINT("Unknown flag for bitmask\n");
    + if (e->bitmask & ~EBT_F_MASK)
    return -EINVAL;
    - }
    - if (e->invflags & ~EBT_INV_MASK) {
    - BUGPRINT("Unknown flag for inv bitmask\n");
    +
    + if (e->invflags & ~EBT_INV_MASK)
    return -EINVAL;
    - }
    - if ((e->bitmask & EBT_NOPROTO) && (e->bitmask & EBT_802_3)) {
    - BUGPRINT("NOPROTO & 802_3 not allowed\n");
    +
    + if ((e->bitmask & EBT_NOPROTO) && (e->bitmask & EBT_802_3))
    return -EINVAL;
    - }
    +
    /* what hook do we belong to? */
    for (i = 0; i < NF_BR_NUMHOOKS; i++) {
    if (!newinfo->hook_entry[i])
    @@ -748,13 +727,11 @@ ebt_check_entry(struct ebt_entry *e, str
    t->u.target = target;
    if (t->u.target == &ebt_standard_target) {
    if (gap < sizeof(struct ebt_standard_target)) {
    - BUGPRINT("Standard target size too big\n");
    ret = -EFAULT;
    goto cleanup_watchers;
    }
    if (((struct ebt_standard_target *)t)->verdict <
    -NUM_STANDARD_TARGETS) {
    - BUGPRINT("Invalid standard target\n");
    ret = -EFAULT;
    goto cleanup_watchers;
    }
    @@ -813,10 +790,9 @@ static int check_chainloops(const struct
    if (strcmp(t->u.name, EBT_STANDARD_TARGET))
    goto letscontinue;
    if (e->target_offset + sizeof(struct ebt_standard_target) >
    - e->next_offset) {
    - BUGPRINT("Standard target size too big\n");
    + e->next_offset)
    return -1;
    - }
    +
    verdict = ((struct ebt_standard_target *)t)->verdict;
    if (verdict >= 0) { /* jump to another chain */
    struct ebt_entries *hlp2 =
    @@ -825,14 +801,12 @@ static int check_chainloops(const struct
    if (hlp2 == cl_s[i].cs.chaininfo)
    break;
    /* bad destination or loop */
    - if (i == udc_cnt) {
    - BUGPRINT("bad destination\n");
    + if (i == udc_cnt)
    return -1;
    - }
    - if (cl_s[i].cs.n) {
    - BUGPRINT("loop\n");
    +
    + if (cl_s[i].cs.n)
    return -1;
    - }
    +
    if (cl_s[i].hookmask & (1 << hooknr))
    goto letscontinue;
    /* this can't be 0, so the loop test is correct */
    @@ -865,24 +839,21 @@ static int translate_table(struct net *n
    i = 0;
    while (i < NF_BR_NUMHOOKS && !newinfo->hook_entry[i])
    i++;
    - if (i == NF_BR_NUMHOOKS) {
    - BUGPRINT("No valid hooks specified\n");
    + if (i == NF_BR_NUMHOOKS)
    return -EINVAL;
    - }
    - if (newinfo->hook_entry[i] != (struct ebt_entries *)newinfo->entries) {
    - BUGPRINT("Chains don't start at beginning\n");
    +
    + if (newinfo->hook_entry[i] != (struct ebt_entries *)newinfo->entries)
    return -EINVAL;
    - }
    +
    /* make sure chains are ordered after each other in same order
    * as their corresponding hooks
    */
    for (j = i + 1; j < NF_BR_NUMHOOKS; j++) {
    if (!newinfo->hook_entry[j])
    continue;
    - if (newinfo->hook_entry[j] <= newinfo->hook_entry[i]) {
    - BUGPRINT("Hook order must be followed\n");
    + if (newinfo->hook_entry[j] <= newinfo->hook_entry[i])
    return -EINVAL;
    - }
    +
    i = j;
    }

    @@ -900,15 +871,11 @@ static int translate_table(struct net *n
    if (ret != 0)
    return ret;

    - if (i != j) {
    - BUGPRINT("nentries does not equal the nr of entries in the "
    - "(last) chain\n");
    + if (i != j)
    return -EINVAL;
    - }
    - if (k != newinfo->nentries) {
    - BUGPRINT("Total nentries is wrong\n");
    +
    + if (k != newinfo->nentries)
    return -EINVAL;
    - }

    /* get the location of the udc, put them in an array
    * while we're at it, allocate the chainstack
    @@ -942,7 +909,6 @@ static int translate_table(struct net *n
    ebt_get_udc_positions, newinfo, &i, cl_s);
    /* sanity check */
    if (i != udc_cnt) {
    - BUGPRINT("i != udc_cnt\n");
    vfree(cl_s);
    return -EFAULT;
    }
    @@ -1042,7 +1008,6 @@ static int do_replace_finish(struct net
    goto free_unlock;

    if (repl->num_counters && repl->num_counters != t->private->nentries) {
    - BUGPRINT("Wrong nr. of counters requested\n");
    ret = -EINVAL;
    goto free_unlock;
    }
    @@ -1118,15 +1083,12 @@ static int do_replace(struct net *net, c
    if (copy_from_user(&tmp, user, sizeof(tmp)) != 0)
    return -EFAULT;

    - if (len != sizeof(tmp) + tmp.entries_size) {
    - BUGPRINT("Wrong len argument\n");
    + if (len != sizeof(tmp) + tmp.entries_size)
    return -EINVAL;
    - }

    - if (tmp.entries_size == 0) {
    - BUGPRINT("Entries_size never zero\n");
    + if (tmp.entries_size == 0)
    return -EINVAL;
    - }
    +
    /* overflow check */
    if (tmp.nentries >= ((INT_MAX - sizeof(struct ebt_table_info)) /
    NR_CPUS - SMP_CACHE_BYTES) / sizeof(struct ebt_counter))
    @@ -1153,7 +1115,6 @@ static int do_replace(struct net *net, c
    }
    if (copy_from_user(
    newinfo->entries, tmp.entries, tmp.entries_size) != 0) {
    - BUGPRINT("Couldn't copy entries from userspace\n");
    ret = -EFAULT;
    goto free_entries;
    }
    @@ -1194,10 +1155,8 @@ int ebt_register_table(struct net *net,

    if (input_table == NULL || (repl = input_table->table) == NULL ||
    repl->entries == NULL || repl->entries_size == 0 ||
    - repl->counters != NULL || input_table->private != NULL) {
    - BUGPRINT("Bad table data for ebt_register_table!!!\n");
    + repl->counters != NULL || input_table->private != NULL)
    return -EINVAL;
    - }

    /* Don't add one table to multiple lists. */
    table = kmemdup(input_table, sizeof(struct ebt_table), GFP_KERNEL);
    @@ -1235,13 +1194,10 @@ int ebt_register_table(struct net *net,
    ((char *)repl->hook_entry[i] - repl->entries);
    }
    ret = translate_table(net, repl->name, newinfo);
    - if (ret != 0) {
    - BUGPRINT("Translate_table failed\n");
    + if (ret != 0)
    goto free_chainstack;
    - }

    if (table->check && table->check(newinfo, table->valid_hooks)) {
    - BUGPRINT("The table doesn't like its own initial data, lol\n");
    ret = -EINVAL;
    goto free_chainstack;
    }
    @@ -1252,7 +1208,6 @@ int ebt_register_table(struct net *net,
    list_for_each_entry(t, &net->xt.tables[NFPROTO_BRIDGE], list) {
    if (strcmp(t->name, table->name) == 0) {
    ret = -EEXIST;
    - BUGPRINT("Table name already exists\n");
    goto free_unlock;
    }
    }
    @@ -1320,7 +1275,6 @@ static int do_update_counters(struct net
    goto free_tmp;

    if (num_counters != t->private->nentries) {
    - BUGPRINT("Wrong nr of counters\n");
    ret = -EINVAL;
    goto unlock_mutex;
    }
    @@ -1447,10 +1401,8 @@ static int copy_counters_to_user(struct
    if (num_counters == 0)
    return 0;

    - if (num_counters != nentries) {
    - BUGPRINT("Num_counters wrong\n");
    + if (num_counters != nentries)
    return -EINVAL;
    - }

    counterstmp = vmalloc(array_size(nentries, sizeof(*counterstmp)));
    if (!counterstmp)
    @@ -1496,15 +1448,11 @@ static int copy_everything_to_user(struc
    (tmp.num_counters ? nentries * sizeof(struct ebt_counter) : 0))
    return -EINVAL;

    - if (tmp.nentries != nentries) {
    - BUGPRINT("Nentries wrong\n");
    + if (tmp.nentries != nentries)
    return -EINVAL;
    - }

    - if (tmp.entries_size != entries_size) {
    - BUGPRINT("Wrong size\n");
    + if (tmp.entries_size != entries_size)
    return -EINVAL;
    - }

    ret = copy_counters_to_user(t, oldcounters, tmp.counters,
    tmp.num_counters, nentries);
    @@ -1576,7 +1524,6 @@ static int do_ebt_get_ctl(struct sock *s
    }
    mutex_unlock(&ebt_mutex);
    if (copy_to_user(user, &tmp, *len) != 0) {
    - BUGPRINT("c2u Didn't work\n");
    ret = -EFAULT;
    break;
    }

    \
     
     \ /
      Last update: 2019-03-26 07:44    [W:4.536 / U:0.184 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site