Skip to content

Commit

Permalink
Merge git://git.kernel.org/pub/scm/linux/kernel/git/pablo/nf
Browse files Browse the repository at this point in the history
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]>
  • Loading branch information
davem330 committed Jan 17, 2020
2 parents 93ad0f9 + 61177e9 commit a72b6a1
Show file tree
Hide file tree
Showing 5 changed files with 54 additions and 24 deletions.
19 changes: 10 additions & 9 deletions net/ipv4/netfilter/arp_tables.c
Original file line number Diff line number Diff line change
Expand Up @@ -496,12 +496,13 @@ static inline int check_entry_size_and_hooks(struct arpt_entry *e,
return 0;
}

static inline void cleanup_entry(struct arpt_entry *e)
static void cleanup_entry(struct arpt_entry *e, struct net *net)
{
struct xt_tgdtor_param par;
struct xt_entry_target *t;

t = arpt_get_target(e);
par.net = net;
par.target = t->u.kernel.target;
par.targinfo = t->data;
par.family = NFPROTO_ARP;
Expand Down Expand Up @@ -584,7 +585,7 @@ static int translate_table(struct net *net,
xt_entry_foreach(iter, entry0, newinfo->size) {
if (i-- == 0)
break;
cleanup_entry(iter);
cleanup_entry(iter, net);
}
return ret;
}
Expand Down Expand Up @@ -927,7 +928,7 @@ static int __do_replace(struct net *net, const char *name,
/* Decrease module usage counts and free resource */
loc_cpu_old_entry = oldinfo->entries;
xt_entry_foreach(iter, loc_cpu_old_entry, oldinfo->size)
cleanup_entry(iter);
cleanup_entry(iter, net);

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

free_newinfo_untrans:
xt_entry_foreach(iter, loc_cpu_entry, newinfo->size)
cleanup_entry(iter);
cleanup_entry(iter, net);
free_newinfo:
xt_free_table_info(newinfo);
return ret;
Expand Down Expand Up @@ -1287,7 +1288,7 @@ static int compat_do_replace(struct net *net, void __user *user,

free_newinfo_untrans:
xt_entry_foreach(iter, loc_cpu_entry, newinfo->size)
cleanup_entry(iter);
cleanup_entry(iter, net);
free_newinfo:
xt_free_table_info(newinfo);
return ret;
Expand Down Expand Up @@ -1514,7 +1515,7 @@ static int do_arpt_get_ctl(struct sock *sk, int cmd, void __user *user, int *len
return ret;
}

static void __arpt_unregister_table(struct xt_table *table)
static void __arpt_unregister_table(struct net *net, struct xt_table *table)
{
struct xt_table_info *private;
void *loc_cpu_entry;
Expand All @@ -1526,7 +1527,7 @@ static void __arpt_unregister_table(struct xt_table *table)
/* Decrease module usage counts and free resources */
loc_cpu_entry = private->entries;
xt_entry_foreach(iter, loc_cpu_entry, private->size)
cleanup_entry(iter);
cleanup_entry(iter, net);
if (private->number > private->initial_entries)
module_put(table_owner);
xt_free_table_info(private);
Expand Down Expand Up @@ -1566,7 +1567,7 @@ int arpt_register_table(struct net *net,

ret = nf_register_net_hooks(net, ops, hweight32(table->valid_hooks));
if (ret != 0) {
__arpt_unregister_table(new_table);
__arpt_unregister_table(net, new_table);
*res = NULL;
}

Expand All @@ -1581,7 +1582,7 @@ void arpt_unregister_table(struct net *net, struct xt_table *table,
const struct nf_hook_ops *ops)
{
nf_unregister_net_hooks(net, ops, hweight32(table->valid_hooks));
__arpt_unregister_table(table);
__arpt_unregister_table(net, table);
}

/* The built-in targets: standard (NULL) and error. */
Expand Down
2 changes: 1 addition & 1 deletion net/netfilter/ipset/ip_set_bitmap_gen.h
Original file line number Diff line number Diff line change
Expand Up @@ -60,9 +60,9 @@ mtype_destroy(struct ip_set *set)
if (SET_WITH_TIMEOUT(set))
del_timer_sync(&map->gc);

ip_set_free(map->members);
if (set->dsize && set->extensions & IPSET_EXT_DESTROY)
mtype_ext_cleanup(set);
ip_set_free(map->members);
ip_set_free(map);

set->data = NULL;
Expand Down
13 changes: 13 additions & 0 deletions net/netfilter/nf_nat_proto.c
Original file line number Diff line number Diff line change
Expand Up @@ -233,6 +233,19 @@ icmp_manip_pkt(struct sk_buff *skb,
return false;

hdr = (struct icmphdr *)(skb->data + hdroff);
switch (hdr->type) {
case ICMP_ECHO:
case ICMP_ECHOREPLY:
case ICMP_TIMESTAMP:
case ICMP_TIMESTAMPREPLY:
case ICMP_INFO_REQUEST:
case ICMP_INFO_REPLY:
case ICMP_ADDRESS:
case ICMP_ADDRESSREPLY:
break;
default:
return true;
}
inet_proto_csum_replace2(&hdr->checksum, skb,
hdr->un.echo.id, tuple->src.u.icmp.id, false);
hdr->un.echo.id = tuple->src.u.icmp.id;
Expand Down
39 changes: 26 additions & 13 deletions net/netfilter/nf_tables_api.c
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,8 @@
#include <net/net_namespace.h>
#include <net/sock.h>

#define NFT_MODULE_AUTOLOAD_LIMIT (MODULE_NAME_LEN - sizeof("nft-expr-255-"))

static LIST_HEAD(nf_tables_expressions);
static LIST_HEAD(nf_tables_objects);
static LIST_HEAD(nf_tables_flowtables);
Expand Down Expand Up @@ -564,33 +566,34 @@ __nf_tables_chain_type_lookup(const struct nlattr *nla, u8 family)
}

/*
* Loading a module requires dropping mutex that guards the
* transaction.
* We first need to abort any pending transactions as once
* mutex is unlocked a different client could start a new
* transaction. It must not see any 'future generation'
* changes * as these changes will never happen.
* Loading a module requires dropping mutex that guards the transaction.
* A different client might race to start a new transaction meanwhile. Zap the
* list of pending transaction and then restore it once the mutex is grabbed
* again. Users of this function return EAGAIN which implicitly triggers the
* transaction abort path to clean up the list of pending transactions.
*/
#ifdef CONFIG_MODULES
static int __nf_tables_abort(struct net *net);

static void nft_request_module(struct net *net, const char *fmt, ...)
{
char module_name[MODULE_NAME_LEN];
LIST_HEAD(commit_list);
va_list args;
int ret;

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

va_start(args, fmt);
ret = vsnprintf(module_name, MODULE_NAME_LEN, fmt, args);
va_end(args);
if (WARN(ret >= MODULE_NAME_LEN, "truncated: '%s' (len %d)", module_name, ret))
if (ret >= MODULE_NAME_LEN)
return;

mutex_unlock(&net->nft.commit_mutex);
request_module("%s", module_name);
mutex_lock(&net->nft.commit_mutex);

WARN_ON_ONCE(!list_empty(&net->nft.commit_list));
list_splice(&commit_list, &net->nft.commit_list);
}
#endif

Expand Down Expand Up @@ -1045,12 +1048,18 @@ static int nft_flush_table(struct nft_ctx *ctx)
}

list_for_each_entry_safe(flowtable, nft, &ctx->table->flowtables, list) {
if (!nft_is_active_next(ctx->net, flowtable))
continue;

err = nft_delflowtable(ctx, flowtable);
if (err < 0)
goto out;
}

list_for_each_entry_safe(obj, ne, &ctx->table->objects, list) {
if (!nft_is_active_next(ctx->net, obj))
continue;

err = nft_delobj(ctx, obj);
if (err < 0)
goto out;
Expand Down Expand Up @@ -1241,7 +1250,8 @@ static const struct nla_policy nft_chain_policy[NFTA_CHAIN_MAX + 1] = {
.len = NFT_CHAIN_MAXNAMELEN - 1 },
[NFTA_CHAIN_HOOK] = { .type = NLA_NESTED },
[NFTA_CHAIN_POLICY] = { .type = NLA_U32 },
[NFTA_CHAIN_TYPE] = { .type = NLA_STRING },
[NFTA_CHAIN_TYPE] = { .type = NLA_STRING,
.len = NFT_MODULE_AUTOLOAD_LIMIT },
[NFTA_CHAIN_COUNTERS] = { .type = NLA_NESTED },
[NFTA_CHAIN_FLAGS] = { .type = NLA_U32 },
};
Expand Down Expand Up @@ -1676,6 +1686,7 @@ static int nf_tables_parse_netdev_hooks(struct net *net,
goto err_hook;
}
if (nft_hook_list_find(hook_list, hook)) {
kfree(hook);
err = -EEXIST;
goto err_hook;
}
Expand Down Expand Up @@ -2355,7 +2366,8 @@ static const struct nft_expr_type *nft_expr_type_get(struct net *net,
}

static const struct nla_policy nft_expr_policy[NFTA_EXPR_MAX + 1] = {
[NFTA_EXPR_NAME] = { .type = NLA_STRING },
[NFTA_EXPR_NAME] = { .type = NLA_STRING,
.len = NFT_MODULE_AUTOLOAD_LIMIT },
[NFTA_EXPR_DATA] = { .type = NLA_NESTED },
};

Expand Down Expand Up @@ -4198,7 +4210,8 @@ static const struct nla_policy nft_set_elem_policy[NFTA_SET_ELEM_MAX + 1] = {
[NFTA_SET_ELEM_USERDATA] = { .type = NLA_BINARY,
.len = NFT_USERDATA_MAXLEN },
[NFTA_SET_ELEM_EXPR] = { .type = NLA_NESTED },
[NFTA_SET_ELEM_OBJREF] = { .type = NLA_STRING },
[NFTA_SET_ELEM_OBJREF] = { .type = NLA_STRING,
.len = NFT_OBJ_MAXNAMELEN - 1 },
};

static const struct nla_policy nft_set_elem_list_policy[NFTA_SET_ELEM_LIST_MAX + 1] = {
Expand Down
5 changes: 4 additions & 1 deletion net/netfilter/nft_tunnel.c
Original file line number Diff line number Diff line change
Expand Up @@ -76,7 +76,7 @@ static int nft_tunnel_get_init(const struct nft_ctx *ctx,
struct nft_tunnel *priv = nft_expr_priv(expr);
u32 len;

if (!tb[NFTA_TUNNEL_KEY] &&
if (!tb[NFTA_TUNNEL_KEY] ||
!tb[NFTA_TUNNEL_DREG])
return -EINVAL;

Expand Down Expand Up @@ -266,6 +266,9 @@ static int nft_tunnel_obj_erspan_init(const struct nlattr *attr,
if (err < 0)
return err;

if (!tb[NFTA_TUNNEL_KEY_ERSPAN_VERSION])
return -EINVAL;

version = ntohl(nla_get_be32(tb[NFTA_TUNNEL_KEY_ERSPAN_VERSION]));
switch (version) {
case ERSPAN_VERSION:
Expand Down

0 comments on commit a72b6a1

Please sign in to comment.