Commit e6b72ee8 authored by Artem Savkov's avatar Artem Savkov Committed by Pablo Neira Ayuso

netfilter: ebtables: fix race condition in frame_filter_net_init()

It is possible for ebt_in_hook to be triggered before ebt_table is assigned
resulting in a NULL-pointer dereference. Make sure hooks are
registered as the last step.

Fixes: aee12a0a ("ebtables: remove nf_hook_register usage")
Signed-off-by: default avatarArtem Savkov <asavkov@redhat.com>
Signed-off-by: default avatarPablo Neira Ayuso <pablo@netfilter.org>
parent 0d18779b
...@@ -108,9 +108,10 @@ struct ebt_table { ...@@ -108,9 +108,10 @@ struct ebt_table {
#define EBT_ALIGN(s) (((s) + (__alignof__(struct _xt_align)-1)) & \ #define EBT_ALIGN(s) (((s) + (__alignof__(struct _xt_align)-1)) & \
~(__alignof__(struct _xt_align)-1)) ~(__alignof__(struct _xt_align)-1))
extern struct ebt_table *ebt_register_table(struct net *net, extern int ebt_register_table(struct net *net,
const struct ebt_table *table, const struct ebt_table *table,
const struct nf_hook_ops *); const struct nf_hook_ops *ops,
struct ebt_table **res);
extern void ebt_unregister_table(struct net *net, struct ebt_table *table, extern void ebt_unregister_table(struct net *net, struct ebt_table *table,
const struct nf_hook_ops *); const struct nf_hook_ops *);
extern unsigned int ebt_do_table(struct sk_buff *skb, extern unsigned int ebt_do_table(struct sk_buff *skb,
......
...@@ -65,8 +65,8 @@ static int ebt_broute(struct sk_buff *skb) ...@@ -65,8 +65,8 @@ static int ebt_broute(struct sk_buff *skb)
static int __net_init broute_net_init(struct net *net) static int __net_init broute_net_init(struct net *net)
{ {
net->xt.broute_table = ebt_register_table(net, &broute_table, NULL); return ebt_register_table(net, &broute_table, NULL,
return PTR_ERR_OR_ZERO(net->xt.broute_table); &net->xt.broute_table);
} }
static void __net_exit broute_net_exit(struct net *net) static void __net_exit broute_net_exit(struct net *net)
......
...@@ -93,8 +93,8 @@ static const struct nf_hook_ops ebt_ops_filter[] = { ...@@ -93,8 +93,8 @@ static const struct nf_hook_ops ebt_ops_filter[] = {
static int __net_init frame_filter_net_init(struct net *net) static int __net_init frame_filter_net_init(struct net *net)
{ {
net->xt.frame_filter = ebt_register_table(net, &frame_filter, ebt_ops_filter); return ebt_register_table(net, &frame_filter, ebt_ops_filter,
return PTR_ERR_OR_ZERO(net->xt.frame_filter); &net->xt.frame_filter);
} }
static void __net_exit frame_filter_net_exit(struct net *net) static void __net_exit frame_filter_net_exit(struct net *net)
......
...@@ -93,8 +93,8 @@ static const struct nf_hook_ops ebt_ops_nat[] = { ...@@ -93,8 +93,8 @@ static const struct nf_hook_ops ebt_ops_nat[] = {
static int __net_init frame_nat_net_init(struct net *net) static int __net_init frame_nat_net_init(struct net *net)
{ {
net->xt.frame_nat = ebt_register_table(net, &frame_nat, ebt_ops_nat); return ebt_register_table(net, &frame_nat, ebt_ops_nat,
return PTR_ERR_OR_ZERO(net->xt.frame_nat); &net->xt.frame_nat);
} }
static void __net_exit frame_nat_net_exit(struct net *net) static void __net_exit frame_nat_net_exit(struct net *net)
......
...@@ -1169,9 +1169,8 @@ static void __ebt_unregister_table(struct net *net, struct ebt_table *table) ...@@ -1169,9 +1169,8 @@ static void __ebt_unregister_table(struct net *net, struct ebt_table *table)
kfree(table); kfree(table);
} }
struct ebt_table * int ebt_register_table(struct net *net, const struct ebt_table *input_table,
ebt_register_table(struct net *net, const struct ebt_table *input_table, const struct nf_hook_ops *ops, struct ebt_table **res)
const struct nf_hook_ops *ops)
{ {
struct ebt_table_info *newinfo; struct ebt_table_info *newinfo;
struct ebt_table *t, *table; struct ebt_table *t, *table;
...@@ -1183,7 +1182,7 @@ ebt_register_table(struct net *net, const struct ebt_table *input_table, ...@@ -1183,7 +1182,7 @@ ebt_register_table(struct net *net, const struct ebt_table *input_table,
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"); BUGPRINT("Bad table data for ebt_register_table!!!\n");
return ERR_PTR(-EINVAL); return -EINVAL;
} }
/* Don't add one table to multiple lists. */ /* Don't add one table to multiple lists. */
...@@ -1252,16 +1251,18 @@ ebt_register_table(struct net *net, const struct ebt_table *input_table, ...@@ -1252,16 +1251,18 @@ ebt_register_table(struct net *net, const struct ebt_table *input_table,
list_add(&table->list, &net->xt.tables[NFPROTO_BRIDGE]); list_add(&table->list, &net->xt.tables[NFPROTO_BRIDGE]);
mutex_unlock(&ebt_mutex); mutex_unlock(&ebt_mutex);
WRITE_ONCE(*res, table);
if (!ops) if (!ops)
return table; return 0;
ret = nf_register_net_hooks(net, ops, hweight32(table->valid_hooks)); ret = nf_register_net_hooks(net, ops, hweight32(table->valid_hooks));
if (ret) { if (ret) {
__ebt_unregister_table(net, table); __ebt_unregister_table(net, table);
return ERR_PTR(ret); *res = NULL;
} }
return table; return ret;
free_unlock: free_unlock:
mutex_unlock(&ebt_mutex); mutex_unlock(&ebt_mutex);
free_chainstack: free_chainstack:
...@@ -1276,7 +1277,7 @@ ebt_register_table(struct net *net, const struct ebt_table *input_table, ...@@ -1276,7 +1277,7 @@ ebt_register_table(struct net *net, const struct ebt_table *input_table,
free_table: free_table:
kfree(table); kfree(table);
out: out:
return ERR_PTR(ret); return ret;
} }
void ebt_unregister_table(struct net *net, struct ebt_table *table, void ebt_unregister_table(struct net *net, struct ebt_table *table,
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment