lkml.org 
[lkml]   [2022]   [Jun]   [3]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    Patch in this message
    /
    From
    Subject[PATCH 5.17 24/75] netfilter: nf_tables: double hook unregistration in netns path
    Date
    From: Pablo Neira Ayuso <pablo@netfilter.org>

    commit f9a43007d3f7ba76d5e7f9421094f00f2ef202f8 upstream.

    __nft_release_hooks() is called from pre_netns exit path which
    unregisters the hooks, then the NETDEV_UNREGISTER event is triggered
    which unregisters the hooks again.

    [ 565.221461] WARNING: CPU: 18 PID: 193 at net/netfilter/core.c:495 __nf_unregister_net_hook+0x247/0x270
    [...]
    [ 565.246890] CPU: 18 PID: 193 Comm: kworker/u64:1 Tainted: G E 5.18.0-rc7+ #27
    [ 565.253682] Workqueue: netns cleanup_net
    [ 565.257059] RIP: 0010:__nf_unregister_net_hook+0x247/0x270
    [...]
    [ 565.297120] Call Trace:
    [ 565.300900] <TASK>
    [ 565.304683] nf_tables_flowtable_event+0x16a/0x220 [nf_tables]
    [ 565.308518] raw_notifier_call_chain+0x63/0x80
    [ 565.312386] unregister_netdevice_many+0x54f/0xb50

    Unregister and destroy netdev hook from netns pre_exit via kfree_rcu
    so the NETDEV_UNREGISTER path see unregistered hooks.

    Fixes: 767d1216bff8 ("netfilter: nftables: fix possible UAF over chains from packet path in netns")
    Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
    Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
    ---
    net/netfilter/nf_tables_api.c | 54 +++++++++++++++++++++++++++++++-----------
    1 file changed, 41 insertions(+), 13 deletions(-)

    --- a/net/netfilter/nf_tables_api.c
    +++ b/net/netfilter/nf_tables_api.c
    @@ -222,12 +222,18 @@ err_register:
    }

    static void nft_netdev_unregister_hooks(struct net *net,
    - struct list_head *hook_list)
    + struct list_head *hook_list,
    + bool release_netdev)
    {
    - struct nft_hook *hook;
    + struct nft_hook *hook, *next;

    - list_for_each_entry(hook, hook_list, list)
    + list_for_each_entry_safe(hook, next, hook_list, list) {
    nf_unregister_net_hook(net, &hook->ops);
    + if (release_netdev) {
    + list_del(&hook->list);
    + kfree_rcu(hook, rcu);
    + }
    + }
    }

    static int nf_tables_register_hook(struct net *net,
    @@ -253,9 +259,10 @@ static int nf_tables_register_hook(struc
    return nf_register_net_hook(net, &basechain->ops);
    }

    -static void nf_tables_unregister_hook(struct net *net,
    - const struct nft_table *table,
    - struct nft_chain *chain)
    +static void __nf_tables_unregister_hook(struct net *net,
    + const struct nft_table *table,
    + struct nft_chain *chain,
    + bool release_netdev)
    {
    struct nft_base_chain *basechain;
    const struct nf_hook_ops *ops;
    @@ -270,11 +277,19 @@ static void nf_tables_unregister_hook(st
    return basechain->type->ops_unregister(net, ops);

    if (nft_base_chain_netdev(table->family, basechain->ops.hooknum))
    - nft_netdev_unregister_hooks(net, &basechain->hook_list);
    + nft_netdev_unregister_hooks(net, &basechain->hook_list,
    + release_netdev);
    else
    nf_unregister_net_hook(net, &basechain->ops);
    }

    +static void nf_tables_unregister_hook(struct net *net,
    + const struct nft_table *table,
    + struct nft_chain *chain)
    +{
    + return __nf_tables_unregister_hook(net, table, chain, false);
    +}
    +
    static void nft_trans_commit_list_add_tail(struct net *net, struct nft_trans *trans)
    {
    struct nftables_pernet *nft_net = nft_pernet(net);
    @@ -7222,13 +7237,25 @@ static void nft_unregister_flowtable_hoo
    FLOW_BLOCK_UNBIND);
    }

    -static void nft_unregister_flowtable_net_hooks(struct net *net,
    - struct list_head *hook_list)
    +static void __nft_unregister_flowtable_net_hooks(struct net *net,
    + struct list_head *hook_list,
    + bool release_netdev)
    {
    - struct nft_hook *hook;
    + struct nft_hook *hook, *next;

    - list_for_each_entry(hook, hook_list, list)
    + list_for_each_entry_safe(hook, next, hook_list, list) {
    nf_unregister_net_hook(net, &hook->ops);
    + if (release_netdev) {
    + list_del(&hook->list);
    + kfree_rcu(hook);
    + }
    + }
    +}
    +
    +static void nft_unregister_flowtable_net_hooks(struct net *net,
    + struct list_head *hook_list)
    +{
    + __nft_unregister_flowtable_net_hooks(net, hook_list, false);
    }

    static int nft_register_flowtable_net_hooks(struct net *net,
    @@ -9672,9 +9699,10 @@ static void __nft_release_hook(struct ne
    struct nft_chain *chain;

    list_for_each_entry(chain, &table->chains, list)
    - nf_tables_unregister_hook(net, table, chain);
    + __nf_tables_unregister_hook(net, table, chain, true);
    list_for_each_entry(flowtable, &table->flowtables, list)
    - nft_unregister_flowtable_net_hooks(net, &flowtable->hook_list);
    + __nft_unregister_flowtable_net_hooks(net, &flowtable->hook_list,
    + true);
    }

    static void __nft_release_hooks(struct net *net)

    \
     
     \ /
      Last update: 2022-06-03 20:14    [W:4.255 / U:0.008 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site