Skip to content

Commit 1b78957

Browse files
Florian Westphalummakynes
authored andcommitted
netfilter: arp_tables: init netns pointer in xt_tgchk_param struct
We get crash when the targets checkentry function tries to make use of the network namespace pointer for arptables. When the net pointer got added back in 2010, only ip/ip6/ebtables were changed to initialize it, so arptables has this set to NULL. This isn't a problem for normal arptables because no existing arptables target has a checkentry function that makes use of par->net. However, direct users of the setsockopt interface can provide any target they want as long as its registered for ARP or UNPSEC protocols. syzkaller managed to send a semi-valid arptables rule for RATEEST target which is enough to trigger NULL deref: kasan: GPF could be caused by NULL-ptr deref or user memory access general protection fault: 0000 [ctrliq#1] PREEMPT SMP KASAN RIP: xt_rateest_tg_checkentry+0x11d/0xb40 net/netfilter/xt_RATEEST.c:109 [..] xt_check_target+0x283/0x690 net/netfilter/x_tables.c:1019 check_target net/ipv4/netfilter/arp_tables.c:399 [inline] find_check_entry net/ipv4/netfilter/arp_tables.c:422 [inline] translate_table+0x1005/0x1d70 net/ipv4/netfilter/arp_tables.c:572 do_replace net/ipv4/netfilter/arp_tables.c:977 [inline] do_arpt_set_ctl+0x310/0x640 net/ipv4/netfilter/arp_tables.c:1456 Fixes: add6746 ("netfilter: add struct net * to target parameters") Reported-by: syzbot+d7358a458d8a81aee898@syzkaller.appspotmail.com Signed-off-by: Florian Westphal <fw@strlen.de> Acked-by: Cong Wang <xiyou.wangcong@gmail.com> Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
1 parent bd6f485 commit 1b78957

File tree

1 file changed

+16
-11
lines changed

1 file changed

+16
-11
lines changed

net/ipv4/netfilter/arp_tables.c

Lines changed: 16 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -384,10 +384,11 @@ next: ;
384384
return 1;
385385
}
386386

387-
static inline int check_target(struct arpt_entry *e, const char *name)
387+
static int check_target(struct arpt_entry *e, struct net *net, const char *name)
388388
{
389389
struct xt_entry_target *t = arpt_get_target(e);
390390
struct xt_tgchk_param par = {
391+
.net = net,
391392
.table = name,
392393
.entryinfo = e,
393394
.target = t->u.kernel.target,
@@ -399,8 +400,9 @@ static inline int check_target(struct arpt_entry *e, const char *name)
399400
return xt_check_target(&par, t->u.target_size - sizeof(*t), 0, false);
400401
}
401402

402-
static inline int
403-
find_check_entry(struct arpt_entry *e, const char *name, unsigned int size,
403+
static int
404+
find_check_entry(struct arpt_entry *e, struct net *net, const char *name,
405+
unsigned int size,
404406
struct xt_percpu_counter_alloc_state *alloc_state)
405407
{
406408
struct xt_entry_target *t;
@@ -419,7 +421,7 @@ find_check_entry(struct arpt_entry *e, const char *name, unsigned int size,
419421
}
420422
t->u.kernel.target = target;
421423

422-
ret = check_target(e, name);
424+
ret = check_target(e, net, name);
423425
if (ret)
424426
goto err;
425427
return 0;
@@ -512,7 +514,9 @@ static inline void cleanup_entry(struct arpt_entry *e)
512514
/* Checks and translates the user-supplied table segment (held in
513515
* newinfo).
514516
*/
515-
static int translate_table(struct xt_table_info *newinfo, void *entry0,
517+
static int translate_table(struct net *net,
518+
struct xt_table_info *newinfo,
519+
void *entry0,
516520
const struct arpt_replace *repl)
517521
{
518522
struct xt_percpu_counter_alloc_state alloc_state = { 0 };
@@ -569,7 +573,7 @@ static int translate_table(struct xt_table_info *newinfo, void *entry0,
569573
/* Finally, each sanity check must pass */
570574
i = 0;
571575
xt_entry_foreach(iter, entry0, newinfo->size) {
572-
ret = find_check_entry(iter, repl->name, repl->size,
576+
ret = find_check_entry(iter, net, repl->name, repl->size,
573577
&alloc_state);
574578
if (ret != 0)
575579
break;
@@ -974,7 +978,7 @@ static int do_replace(struct net *net, const void __user *user,
974978
goto free_newinfo;
975979
}
976980

977-
ret = translate_table(newinfo, loc_cpu_entry, &tmp);
981+
ret = translate_table(net, newinfo, loc_cpu_entry, &tmp);
978982
if (ret != 0)
979983
goto free_newinfo;
980984

@@ -1149,7 +1153,8 @@ compat_copy_entry_from_user(struct compat_arpt_entry *e, void **dstptr,
11491153
}
11501154
}
11511155

1152-
static int translate_compat_table(struct xt_table_info **pinfo,
1156+
static int translate_compat_table(struct net *net,
1157+
struct xt_table_info **pinfo,
11531158
void **pentry0,
11541159
const struct compat_arpt_replace *compatr)
11551160
{
@@ -1217,7 +1222,7 @@ static int translate_compat_table(struct xt_table_info **pinfo,
12171222
repl.num_counters = 0;
12181223
repl.counters = NULL;
12191224
repl.size = newinfo->size;
1220-
ret = translate_table(newinfo, entry1, &repl);
1225+
ret = translate_table(net, newinfo, entry1, &repl);
12211226
if (ret)
12221227
goto free_newinfo;
12231228

@@ -1270,7 +1275,7 @@ static int compat_do_replace(struct net *net, void __user *user,
12701275
goto free_newinfo;
12711276
}
12721277

1273-
ret = translate_compat_table(&newinfo, &loc_cpu_entry, &tmp);
1278+
ret = translate_compat_table(net, &newinfo, &loc_cpu_entry, &tmp);
12741279
if (ret != 0)
12751280
goto free_newinfo;
12761281

@@ -1546,7 +1551,7 @@ int arpt_register_table(struct net *net,
15461551
loc_cpu_entry = newinfo->entries;
15471552
memcpy(loc_cpu_entry, repl->entries, repl->size);
15481553

1549-
ret = translate_table(newinfo, loc_cpu_entry, repl);
1554+
ret = translate_table(net, newinfo, loc_cpu_entry, repl);
15501555
if (ret != 0)
15511556
goto out_free;
15521557

0 commit comments

Comments
 (0)