Skip to content

Commit a72b6a1

Browse files
committed
Merge git://git.kernel.org/pub/scm/linux/kernel/git/pablo/nf
Pablo Neira Ayuso says: ==================== Netfilter updates for net The following patchset contains Netfilter fixes for net: 1) Fix use-after-free in ipset bitmap destroy path, from Cong Wang. 2) Missing init netns in entry cleanup path of arp_tables, from Florian Westphal. 3) Fix WARN_ON in set destroy path due to missing cleanup on transaction error. 4) Incorrect netlink sanity check in tunnel, from Florian Westphal. 5) Missing sanity check for erspan version netlink attribute, also from Florian. 6) Remove WARN in nft_request_module() that can be triggered from userspace, from Florian Westphal. 7) Memleak in NFTA_HOOK_DEVS netlink parser, from Dan Carpenter. 8) List poison from commit path for flowtables that are added and deleted in the same batch, from Florian Westphal. 9) Fix NAT ICMP packet corruption, from Eyal Birger. ==================== Signed-off-by: David S. Miller <[email protected]>
2 parents 93ad0f9 + 61177e9 commit a72b6a1

File tree

5 files changed

+54
-24
lines changed

5 files changed

+54
-24
lines changed

net/ipv4/netfilter/arp_tables.c

Lines changed: 10 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -496,12 +496,13 @@ static inline int check_entry_size_and_hooks(struct arpt_entry *e,
496496
return 0;
497497
}
498498

499-
static inline void cleanup_entry(struct arpt_entry *e)
499+
static void cleanup_entry(struct arpt_entry *e, struct net *net)
500500
{
501501
struct xt_tgdtor_param par;
502502
struct xt_entry_target *t;
503503

504504
t = arpt_get_target(e);
505+
par.net = net;
505506
par.target = t->u.kernel.target;
506507
par.targinfo = t->data;
507508
par.family = NFPROTO_ARP;
@@ -584,7 +585,7 @@ static int translate_table(struct net *net,
584585
xt_entry_foreach(iter, entry0, newinfo->size) {
585586
if (i-- == 0)
586587
break;
587-
cleanup_entry(iter);
588+
cleanup_entry(iter, net);
588589
}
589590
return ret;
590591
}
@@ -927,7 +928,7 @@ static int __do_replace(struct net *net, const char *name,
927928
/* Decrease module usage counts and free resource */
928929
loc_cpu_old_entry = oldinfo->entries;
929930
xt_entry_foreach(iter, loc_cpu_old_entry, oldinfo->size)
930-
cleanup_entry(iter);
931+
cleanup_entry(iter, net);
931932

932933
xt_free_table_info(oldinfo);
933934
if (copy_to_user(counters_ptr, counters,
@@ -990,7 +991,7 @@ static int do_replace(struct net *net, const void __user *user,
990991

991992
free_newinfo_untrans:
992993
xt_entry_foreach(iter, loc_cpu_entry, newinfo->size)
993-
cleanup_entry(iter);
994+
cleanup_entry(iter, net);
994995
free_newinfo:
995996
xt_free_table_info(newinfo);
996997
return ret;
@@ -1287,7 +1288,7 @@ static int compat_do_replace(struct net *net, void __user *user,
12871288

12881289
free_newinfo_untrans:
12891290
xt_entry_foreach(iter, loc_cpu_entry, newinfo->size)
1290-
cleanup_entry(iter);
1291+
cleanup_entry(iter, net);
12911292
free_newinfo:
12921293
xt_free_table_info(newinfo);
12931294
return ret;
@@ -1514,7 +1515,7 @@ static int do_arpt_get_ctl(struct sock *sk, int cmd, void __user *user, int *len
15141515
return ret;
15151516
}
15161517

1517-
static void __arpt_unregister_table(struct xt_table *table)
1518+
static void __arpt_unregister_table(struct net *net, struct xt_table *table)
15181519
{
15191520
struct xt_table_info *private;
15201521
void *loc_cpu_entry;
@@ -1526,7 +1527,7 @@ static void __arpt_unregister_table(struct xt_table *table)
15261527
/* Decrease module usage counts and free resources */
15271528
loc_cpu_entry = private->entries;
15281529
xt_entry_foreach(iter, loc_cpu_entry, private->size)
1529-
cleanup_entry(iter);
1530+
cleanup_entry(iter, net);
15301531
if (private->number > private->initial_entries)
15311532
module_put(table_owner);
15321533
xt_free_table_info(private);
@@ -1566,7 +1567,7 @@ int arpt_register_table(struct net *net,
15661567

15671568
ret = nf_register_net_hooks(net, ops, hweight32(table->valid_hooks));
15681569
if (ret != 0) {
1569-
__arpt_unregister_table(new_table);
1570+
__arpt_unregister_table(net, new_table);
15701571
*res = NULL;
15711572
}
15721573

@@ -1581,7 +1582,7 @@ void arpt_unregister_table(struct net *net, struct xt_table *table,
15811582
const struct nf_hook_ops *ops)
15821583
{
15831584
nf_unregister_net_hooks(net, ops, hweight32(table->valid_hooks));
1584-
__arpt_unregister_table(table);
1585+
__arpt_unregister_table(net, table);
15851586
}
15861587

15871588
/* The built-in targets: standard (NULL) and error. */

net/netfilter/ipset/ip_set_bitmap_gen.h

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -60,9 +60,9 @@ mtype_destroy(struct ip_set *set)
6060
if (SET_WITH_TIMEOUT(set))
6161
del_timer_sync(&map->gc);
6262

63-
ip_set_free(map->members);
6463
if (set->dsize && set->extensions & IPSET_EXT_DESTROY)
6564
mtype_ext_cleanup(set);
65+
ip_set_free(map->members);
6666
ip_set_free(map);
6767

6868
set->data = NULL;

net/netfilter/nf_nat_proto.c

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -233,6 +233,19 @@ icmp_manip_pkt(struct sk_buff *skb,
233233
return false;
234234

235235
hdr = (struct icmphdr *)(skb->data + hdroff);
236+
switch (hdr->type) {
237+
case ICMP_ECHO:
238+
case ICMP_ECHOREPLY:
239+
case ICMP_TIMESTAMP:
240+
case ICMP_TIMESTAMPREPLY:
241+
case ICMP_INFO_REQUEST:
242+
case ICMP_INFO_REPLY:
243+
case ICMP_ADDRESS:
244+
case ICMP_ADDRESSREPLY:
245+
break;
246+
default:
247+
return true;
248+
}
236249
inet_proto_csum_replace2(&hdr->checksum, skb,
237250
hdr->un.echo.id, tuple->src.u.icmp.id, false);
238251
hdr->un.echo.id = tuple->src.u.icmp.id;

net/netfilter/nf_tables_api.c

Lines changed: 26 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,8 @@
2222
#include <net/net_namespace.h>
2323
#include <net/sock.h>
2424

25+
#define NFT_MODULE_AUTOLOAD_LIMIT (MODULE_NAME_LEN - sizeof("nft-expr-255-"))
26+
2527
static LIST_HEAD(nf_tables_expressions);
2628
static LIST_HEAD(nf_tables_objects);
2729
static LIST_HEAD(nf_tables_flowtables);
@@ -564,33 +566,34 @@ __nf_tables_chain_type_lookup(const struct nlattr *nla, u8 family)
564566
}
565567

566568
/*
567-
* Loading a module requires dropping mutex that guards the
568-
* transaction.
569-
* We first need to abort any pending transactions as once
570-
* mutex is unlocked a different client could start a new
571-
* transaction. It must not see any 'future generation'
572-
* changes * as these changes will never happen.
569+
* Loading a module requires dropping mutex that guards the transaction.
570+
* A different client might race to start a new transaction meanwhile. Zap the
571+
* list of pending transaction and then restore it once the mutex is grabbed
572+
* again. Users of this function return EAGAIN which implicitly triggers the
573+
* transaction abort path to clean up the list of pending transactions.
573574
*/
574575
#ifdef CONFIG_MODULES
575-
static int __nf_tables_abort(struct net *net);
576-
577576
static void nft_request_module(struct net *net, const char *fmt, ...)
578577
{
579578
char module_name[MODULE_NAME_LEN];
579+
LIST_HEAD(commit_list);
580580
va_list args;
581581
int ret;
582582

583-
__nf_tables_abort(net);
583+
list_splice_init(&net->nft.commit_list, &commit_list);
584584

585585
va_start(args, fmt);
586586
ret = vsnprintf(module_name, MODULE_NAME_LEN, fmt, args);
587587
va_end(args);
588-
if (WARN(ret >= MODULE_NAME_LEN, "truncated: '%s' (len %d)", module_name, ret))
588+
if (ret >= MODULE_NAME_LEN)
589589
return;
590590

591591
mutex_unlock(&net->nft.commit_mutex);
592592
request_module("%s", module_name);
593593
mutex_lock(&net->nft.commit_mutex);
594+
595+
WARN_ON_ONCE(!list_empty(&net->nft.commit_list));
596+
list_splice(&commit_list, &net->nft.commit_list);
594597
}
595598
#endif
596599

@@ -1045,12 +1048,18 @@ static int nft_flush_table(struct nft_ctx *ctx)
10451048
}
10461049

10471050
list_for_each_entry_safe(flowtable, nft, &ctx->table->flowtables, list) {
1051+
if (!nft_is_active_next(ctx->net, flowtable))
1052+
continue;
1053+
10481054
err = nft_delflowtable(ctx, flowtable);
10491055
if (err < 0)
10501056
goto out;
10511057
}
10521058

10531059
list_for_each_entry_safe(obj, ne, &ctx->table->objects, list) {
1060+
if (!nft_is_active_next(ctx->net, obj))
1061+
continue;
1062+
10541063
err = nft_delobj(ctx, obj);
10551064
if (err < 0)
10561065
goto out;
@@ -1241,7 +1250,8 @@ static const struct nla_policy nft_chain_policy[NFTA_CHAIN_MAX + 1] = {
12411250
.len = NFT_CHAIN_MAXNAMELEN - 1 },
12421251
[NFTA_CHAIN_HOOK] = { .type = NLA_NESTED },
12431252
[NFTA_CHAIN_POLICY] = { .type = NLA_U32 },
1244-
[NFTA_CHAIN_TYPE] = { .type = NLA_STRING },
1253+
[NFTA_CHAIN_TYPE] = { .type = NLA_STRING,
1254+
.len = NFT_MODULE_AUTOLOAD_LIMIT },
12451255
[NFTA_CHAIN_COUNTERS] = { .type = NLA_NESTED },
12461256
[NFTA_CHAIN_FLAGS] = { .type = NLA_U32 },
12471257
};
@@ -1676,6 +1686,7 @@ static int nf_tables_parse_netdev_hooks(struct net *net,
16761686
goto err_hook;
16771687
}
16781688
if (nft_hook_list_find(hook_list, hook)) {
1689+
kfree(hook);
16791690
err = -EEXIST;
16801691
goto err_hook;
16811692
}
@@ -2355,7 +2366,8 @@ static const struct nft_expr_type *nft_expr_type_get(struct net *net,
23552366
}
23562367

23572368
static const struct nla_policy nft_expr_policy[NFTA_EXPR_MAX + 1] = {
2358-
[NFTA_EXPR_NAME] = { .type = NLA_STRING },
2369+
[NFTA_EXPR_NAME] = { .type = NLA_STRING,
2370+
.len = NFT_MODULE_AUTOLOAD_LIMIT },
23592371
[NFTA_EXPR_DATA] = { .type = NLA_NESTED },
23602372
};
23612373

@@ -4198,7 +4210,8 @@ static const struct nla_policy nft_set_elem_policy[NFTA_SET_ELEM_MAX + 1] = {
41984210
[NFTA_SET_ELEM_USERDATA] = { .type = NLA_BINARY,
41994211
.len = NFT_USERDATA_MAXLEN },
42004212
[NFTA_SET_ELEM_EXPR] = { .type = NLA_NESTED },
4201-
[NFTA_SET_ELEM_OBJREF] = { .type = NLA_STRING },
4213+
[NFTA_SET_ELEM_OBJREF] = { .type = NLA_STRING,
4214+
.len = NFT_OBJ_MAXNAMELEN - 1 },
42024215
};
42034216

42044217
static const struct nla_policy nft_set_elem_list_policy[NFTA_SET_ELEM_LIST_MAX + 1] = {

net/netfilter/nft_tunnel.c

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -76,7 +76,7 @@ static int nft_tunnel_get_init(const struct nft_ctx *ctx,
7676
struct nft_tunnel *priv = nft_expr_priv(expr);
7777
u32 len;
7878

79-
if (!tb[NFTA_TUNNEL_KEY] &&
79+
if (!tb[NFTA_TUNNEL_KEY] ||
8080
!tb[NFTA_TUNNEL_DREG])
8181
return -EINVAL;
8282

@@ -266,6 +266,9 @@ static int nft_tunnel_obj_erspan_init(const struct nlattr *attr,
266266
if (err < 0)
267267
return err;
268268

269+
if (!tb[NFTA_TUNNEL_KEY_ERSPAN_VERSION])
270+
return -EINVAL;
271+
269272
version = ntohl(nla_get_be32(tb[NFTA_TUNNEL_KEY_ERSPAN_VERSION]));
270273
switch (version) {
271274
case ERSPAN_VERSION:

0 commit comments

Comments
 (0)