Skip to content

Commit e6b72ee

Browse files
sm00thummakynes
sm00th
authored and
ummakynes
committed
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: aee12a0 ("ebtables: remove nf_hook_register usage") Signed-off-by: Artem Savkov <asavkov@redhat.com> Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
1 parent 0d18779 commit e6b72ee

File tree

5 files changed

+19
-17
lines changed

5 files changed

+19
-17
lines changed

‎include/linux/netfilter_bridge/ebtables.h

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -108,9 +108,10 @@ struct ebt_table {
108108

109109
#define EBT_ALIGN(s) (((s) + (__alignof__(struct _xt_align)-1)) & \
110110
~(__alignof__(struct _xt_align)-1))
111-
extern struct ebt_table *ebt_register_table(struct net *net,
112-
const struct ebt_table *table,
113-
const struct nf_hook_ops *);
111+
extern int ebt_register_table(struct net *net,
112+
const struct ebt_table *table,
113+
const struct nf_hook_ops *ops,
114+
struct ebt_table **res);
114115
extern void ebt_unregister_table(struct net *net, struct ebt_table *table,
115116
const struct nf_hook_ops *);
116117
extern unsigned int ebt_do_table(struct sk_buff *skb,

‎net/bridge/netfilter/ebtable_broute.c

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -65,8 +65,8 @@ static int ebt_broute(struct sk_buff *skb)
6565

6666
static int __net_init broute_net_init(struct net *net)
6767
{
68-
net->xt.broute_table = ebt_register_table(net, &broute_table, NULL);
69-
return PTR_ERR_OR_ZERO(net->xt.broute_table);
68+
return ebt_register_table(net, &broute_table, NULL,
69+
&net->xt.broute_table);
7070
}
7171

7272
static void __net_exit broute_net_exit(struct net *net)

‎net/bridge/netfilter/ebtable_filter.c

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -93,8 +93,8 @@ static const struct nf_hook_ops ebt_ops_filter[] = {
9393

9494
static int __net_init frame_filter_net_init(struct net *net)
9595
{
96-
net->xt.frame_filter = ebt_register_table(net, &frame_filter, ebt_ops_filter);
97-
return PTR_ERR_OR_ZERO(net->xt.frame_filter);
96+
return ebt_register_table(net, &frame_filter, ebt_ops_filter,
97+
&net->xt.frame_filter);
9898
}
9999

100100
static void __net_exit frame_filter_net_exit(struct net *net)

‎net/bridge/netfilter/ebtable_nat.c

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -93,8 +93,8 @@ static const struct nf_hook_ops ebt_ops_nat[] = {
9393

9494
static int __net_init frame_nat_net_init(struct net *net)
9595
{
96-
net->xt.frame_nat = ebt_register_table(net, &frame_nat, ebt_ops_nat);
97-
return PTR_ERR_OR_ZERO(net->xt.frame_nat);
96+
return ebt_register_table(net, &frame_nat, ebt_ops_nat,
97+
&net->xt.frame_nat);
9898
}
9999

100100
static void __net_exit frame_nat_net_exit(struct net *net)

‎net/bridge/netfilter/ebtables.c

Lines changed: 9 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -1169,9 +1169,8 @@ static void __ebt_unregister_table(struct net *net, struct ebt_table *table)
11691169
kfree(table);
11701170
}
11711171

1172-
struct ebt_table *
1173-
ebt_register_table(struct net *net, const struct ebt_table *input_table,
1174-
const struct nf_hook_ops *ops)
1172+
int ebt_register_table(struct net *net, const struct ebt_table *input_table,
1173+
const struct nf_hook_ops *ops, struct ebt_table **res)
11751174
{
11761175
struct ebt_table_info *newinfo;
11771176
struct ebt_table *t, *table;
@@ -1183,7 +1182,7 @@ ebt_register_table(struct net *net, const struct ebt_table *input_table,
11831182
repl->entries == NULL || repl->entries_size == 0 ||
11841183
repl->counters != NULL || input_table->private != NULL) {
11851184
BUGPRINT("Bad table data for ebt_register_table!!!\n");
1186-
return ERR_PTR(-EINVAL);
1185+
return -EINVAL;
11871186
}
11881187

11891188
/* Don't add one table to multiple lists. */
@@ -1252,16 +1251,18 @@ ebt_register_table(struct net *net, const struct ebt_table *input_table,
12521251
list_add(&table->list, &net->xt.tables[NFPROTO_BRIDGE]);
12531252
mutex_unlock(&ebt_mutex);
12541253

1254+
WRITE_ONCE(*res, table);
1255+
12551256
if (!ops)
1256-
return table;
1257+
return 0;
12571258

12581259
ret = nf_register_net_hooks(net, ops, hweight32(table->valid_hooks));
12591260
if (ret) {
12601261
__ebt_unregister_table(net, table);
1261-
return ERR_PTR(ret);
1262+
*res = NULL;
12621263
}
12631264

1264-
return table;
1265+
return ret;
12651266
free_unlock:
12661267
mutex_unlock(&ebt_mutex);
12671268
free_chainstack:
@@ -1276,7 +1277,7 @@ ebt_register_table(struct net *net, const struct ebt_table *input_table,
12761277
free_table:
12771278
kfree(table);
12781279
out:
1279-
return ERR_PTR(ret);
1280+
return ret;
12801281
}
12811282

12821283
void ebt_unregister_table(struct net *net, struct ebt_table *table,

0 commit comments

Comments
 (0)