lkml.org 
[lkml]   [2022]   [Sep]   [2]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    Patch in this message
    /
    From
    Subject[PATCH 4.14 09/42] netfilter: ebtables: reject blobs that dont provide all entry points
    Date
    From: Florian Westphal <fw@strlen.de>

    [ Upstream commit 7997eff82828304b780dc0a39707e1946d6f1ebf ]

    Harshit Mogalapalli says:
    In ebt_do_table() function dereferencing 'private->hook_entry[hook]'
    can lead to NULL pointer dereference. [..] Kernel panic:

    general protection fault, probably for non-canonical address 0xdffffc0000000005: 0000 [#1] PREEMPT SMP KASAN
    KASAN: null-ptr-deref in range [0x0000000000000028-0x000000000000002f]
    [..]
    RIP: 0010:ebt_do_table+0x1dc/0x1ce0
    Code: 89 fa 48 c1 ea 03 80 3c 02 00 0f 85 5c 16 00 00 48 b8 00 00 00 00 00 fc ff df 49 8b 6c df 08 48 8d 7d 2c 48 89 fa 48 c1 ea 03 <0f> b6 14 02 48 89 f8 83 e0 07 83 c0 03 38 d0 7c 08 84 d2 0f 85 88
    [..]
    Call Trace:
    nf_hook_slow+0xb1/0x170
    __br_forward+0x289/0x730
    maybe_deliver+0x24b/0x380
    br_flood+0xc6/0x390
    br_dev_xmit+0xa2e/0x12c0

    For some reason ebtables rejects blobs that provide entry points that are
    not supported by the table, but what it should instead reject is the
    opposite: blobs that DO NOT provide an entry point supported by the table.

    t->valid_hooks is the bitmask of hooks (input, forward ...) that will see
    packets. Providing an entry point that is not support is harmless
    (never called/used), but the inverse isn't: it results in a crash
    because the ebtables traverser doesn't expect a NULL blob for a location
    its receiving packets for.

    Instead of fixing all the individual checks, do what iptables is doing and
    reject all blobs that differ from the expected hooks.

    Fixes: 1da177e4c3f4 ("Linux-2.6.12-rc2")
    Reported-by: Harshit Mogalapalli <harshit.m.mogalapalli@oracle.com>
    Reported-by: syzkaller <syzkaller@googlegroups.com>
    Signed-off-by: Florian Westphal <fw@strlen.de>
    Signed-off-by: Sasha Levin <sashal@kernel.org>
    ---
    include/linux/netfilter_bridge/ebtables.h | 4 ----
    net/bridge/netfilter/ebtable_broute.c | 8 --------
    net/bridge/netfilter/ebtable_filter.c | 8 --------
    net/bridge/netfilter/ebtable_nat.c | 8 --------
    net/bridge/netfilter/ebtables.c | 8 +-------
    5 files changed, 1 insertion(+), 35 deletions(-)

    diff --git a/include/linux/netfilter_bridge/ebtables.h b/include/linux/netfilter_bridge/ebtables.h
    index 0773b5a032f1f..f014aee2f7184 100644
    --- a/include/linux/netfilter_bridge/ebtables.h
    +++ b/include/linux/netfilter_bridge/ebtables.h
    @@ -98,10 +98,6 @@ struct ebt_table {
    struct ebt_replace_kernel *table;
    unsigned int valid_hooks;
    rwlock_t lock;
    - /* e.g. could be the table explicitly only allows certain
    - * matches, targets, ... 0 == let it in */
    - int (*check)(const struct ebt_table_info *info,
    - unsigned int valid_hooks);
    /* the data used by the kernel */
    struct ebt_table_info *private;
    struct module *me;
    diff --git a/net/bridge/netfilter/ebtable_broute.c b/net/bridge/netfilter/ebtable_broute.c
    index 276b60262981c..b21c8a317be73 100644
    --- a/net/bridge/netfilter/ebtable_broute.c
    +++ b/net/bridge/netfilter/ebtable_broute.c
    @@ -33,18 +33,10 @@ static struct ebt_replace_kernel initial_table = {
    .entries = (char *)&initial_chain,
    };

    -static int check(const struct ebt_table_info *info, unsigned int valid_hooks)
    -{
    - if (valid_hooks & ~(1 << NF_BR_BROUTING))
    - return -EINVAL;
    - return 0;
    -}
    -
    static const struct ebt_table broute_table = {
    .name = "broute",
    .table = &initial_table,
    .valid_hooks = 1 << NF_BR_BROUTING,
    - .check = check,
    .me = THIS_MODULE,
    };

    diff --git a/net/bridge/netfilter/ebtable_filter.c b/net/bridge/netfilter/ebtable_filter.c
    index c41da5fac84f4..c59021989af32 100644
    --- a/net/bridge/netfilter/ebtable_filter.c
    +++ b/net/bridge/netfilter/ebtable_filter.c
    @@ -41,18 +41,10 @@ static struct ebt_replace_kernel initial_table = {
    .entries = (char *)initial_chains,
    };

    -static int check(const struct ebt_table_info *info, unsigned int valid_hooks)
    -{
    - if (valid_hooks & ~FILTER_VALID_HOOKS)
    - return -EINVAL;
    - return 0;
    -}
    -
    static const struct ebt_table frame_filter = {
    .name = "filter",
    .table = &initial_table,
    .valid_hooks = FILTER_VALID_HOOKS,
    - .check = check,
    .me = THIS_MODULE,
    };

    diff --git a/net/bridge/netfilter/ebtable_nat.c b/net/bridge/netfilter/ebtable_nat.c
    index 08df7406ecb38..1bb12157ce09d 100644
    --- a/net/bridge/netfilter/ebtable_nat.c
    +++ b/net/bridge/netfilter/ebtable_nat.c
    @@ -41,18 +41,10 @@ static struct ebt_replace_kernel initial_table = {
    .entries = (char *)initial_chains,
    };

    -static int check(const struct ebt_table_info *info, unsigned int valid_hooks)
    -{
    - if (valid_hooks & ~NAT_VALID_HOOKS)
    - return -EINVAL;
    - return 0;
    -}
    -
    static const struct ebt_table frame_nat = {
    .name = "nat",
    .table = &initial_table,
    .valid_hooks = NAT_VALID_HOOKS,
    - .check = check,
    .me = THIS_MODULE,
    };

    diff --git a/net/bridge/netfilter/ebtables.c b/net/bridge/netfilter/ebtables.c
    index a1834ad7422ce..a54149f10f7ef 100644
    --- a/net/bridge/netfilter/ebtables.c
    +++ b/net/bridge/netfilter/ebtables.c
    @@ -991,8 +991,7 @@ static int do_replace_finish(struct net *net, struct ebt_replace *repl,
    goto free_iterate;
    }

    - /* the table doesn't like it */
    - if (t->check && (ret = t->check(newinfo, repl->valid_hooks)))
    + if (repl->valid_hooks != t->valid_hooks)
    goto free_unlock;

    if (repl->num_counters && repl->num_counters != t->private->nentries) {
    @@ -1200,11 +1199,6 @@ int ebt_register_table(struct net *net, const struct ebt_table *input_table,
    if (ret != 0)
    goto free_chainstack;

    - if (table->check && table->check(newinfo, table->valid_hooks)) {
    - ret = -EINVAL;
    - goto free_chainstack;
    - }
    -
    table->private = newinfo;
    rwlock_init(&table->lock);
    mutex_lock(&ebt_mutex);
    --
    2.35.1


    \
     
     \ /
      Last update: 2022-09-02 14:28    [W:2.918 / U:0.020 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site