Commit ab0891f5 authored by David S. Miller's avatar David S. Miller

Merge git://git.kernel.org/pub/scm/linux/kernel/git/pablo/nf

Pablo Neira Ayuso says:

====================
Netfilter fixes for net

The following patchset contains Netfilter fixes for your net tree:

1) Fix list corruption in device notifier in the masquerade
   infrastructure, from Florian Westphal.

2) Fix double-free of sets and use-after-free when deleting elements.

3) Don't bogusly return EBUSY when removing a set after flush command.

4) Use-after-free in dynamically allocate operations.

5) Don't report a new ruleset generation to userspace if transaction
   list is empty, this invalidates the userspace cache innecessarily.
   From Florian Westphal.
====================
Signed-off-by: default avatarDavid S. Miller <davem@davemloft.net>
parents ee74d0bd b8b27498
...@@ -382,6 +382,7 @@ void nft_unregister_set(struct nft_set_type *type); ...@@ -382,6 +382,7 @@ void nft_unregister_set(struct nft_set_type *type);
* @dtype: data type (verdict or numeric type defined by userspace) * @dtype: data type (verdict or numeric type defined by userspace)
* @objtype: object type (see NFT_OBJECT_* definitions) * @objtype: object type (see NFT_OBJECT_* definitions)
* @size: maximum set size * @size: maximum set size
* @use: number of rules references to this set
* @nelems: number of elements * @nelems: number of elements
* @ndeact: number of deactivated elements queued for removal * @ndeact: number of deactivated elements queued for removal
* @timeout: default timeout value in jiffies * @timeout: default timeout value in jiffies
...@@ -407,6 +408,7 @@ struct nft_set { ...@@ -407,6 +408,7 @@ struct nft_set {
u32 dtype; u32 dtype;
u32 objtype; u32 objtype;
u32 size; u32 size;
u32 use;
atomic_t nelems; atomic_t nelems;
u32 ndeact; u32 ndeact;
u64 timeout; u64 timeout;
...@@ -416,7 +418,8 @@ struct nft_set { ...@@ -416,7 +418,8 @@ struct nft_set {
unsigned char *udata; unsigned char *udata;
/* runtime data below here */ /* runtime data below here */
const struct nft_set_ops *ops ____cacheline_aligned; const struct nft_set_ops *ops ____cacheline_aligned;
u16 flags:14, u16 flags:13,
bound:1,
genmask:2; genmask:2;
u8 klen; u8 klen;
u8 dlen; u8 dlen;
...@@ -466,6 +469,10 @@ struct nft_set_binding { ...@@ -466,6 +469,10 @@ struct nft_set_binding {
u32 flags; u32 flags;
}; };
enum nft_trans_phase;
void nf_tables_deactivate_set(const struct nft_ctx *ctx, struct nft_set *set,
struct nft_set_binding *binding,
enum nft_trans_phase phase);
int nf_tables_bind_set(const struct nft_ctx *ctx, struct nft_set *set, int nf_tables_bind_set(const struct nft_ctx *ctx, struct nft_set *set,
struct nft_set_binding *binding); struct nft_set_binding *binding);
void nf_tables_unbind_set(const struct nft_ctx *ctx, struct nft_set *set, void nf_tables_unbind_set(const struct nft_ctx *ctx, struct nft_set *set,
...@@ -1344,15 +1351,12 @@ struct nft_trans_rule { ...@@ -1344,15 +1351,12 @@ struct nft_trans_rule {
struct nft_trans_set { struct nft_trans_set {
struct nft_set *set; struct nft_set *set;
u32 set_id; u32 set_id;
bool bound;
}; };
#define nft_trans_set(trans) \ #define nft_trans_set(trans) \
(((struct nft_trans_set *)trans->data)->set) (((struct nft_trans_set *)trans->data)->set)
#define nft_trans_set_id(trans) \ #define nft_trans_set_id(trans) \
(((struct nft_trans_set *)trans->data)->set_id) (((struct nft_trans_set *)trans->data)->set_id)
#define nft_trans_set_bound(trans) \
(((struct nft_trans_set *)trans->data)->bound)
struct nft_trans_chain { struct nft_trans_chain {
bool update; bool update;
......
...@@ -11,7 +11,8 @@ ...@@ -11,7 +11,8 @@
#include <net/netfilter/ipv6/nf_nat_masquerade.h> #include <net/netfilter/ipv6/nf_nat_masquerade.h>
static DEFINE_MUTEX(masq_mutex); static DEFINE_MUTEX(masq_mutex);
static unsigned int masq_refcnt __read_mostly; static unsigned int masq_refcnt4 __read_mostly;
static unsigned int masq_refcnt6 __read_mostly;
unsigned int unsigned int
nf_nat_masquerade_ipv4(struct sk_buff *skb, unsigned int hooknum, nf_nat_masquerade_ipv4(struct sk_buff *skb, unsigned int hooknum,
...@@ -141,8 +142,13 @@ int nf_nat_masquerade_ipv4_register_notifier(void) ...@@ -141,8 +142,13 @@ int nf_nat_masquerade_ipv4_register_notifier(void)
int ret = 0; int ret = 0;
mutex_lock(&masq_mutex); mutex_lock(&masq_mutex);
if (WARN_ON_ONCE(masq_refcnt4 == UINT_MAX)) {
ret = -EOVERFLOW;
goto out_unlock;
}
/* check if the notifier was already set */ /* check if the notifier was already set */
if (++masq_refcnt > 1) if (++masq_refcnt4 > 1)
goto out_unlock; goto out_unlock;
/* Register for device down reports */ /* Register for device down reports */
...@@ -160,7 +166,7 @@ int nf_nat_masquerade_ipv4_register_notifier(void) ...@@ -160,7 +166,7 @@ int nf_nat_masquerade_ipv4_register_notifier(void)
err_unregister: err_unregister:
unregister_netdevice_notifier(&masq_dev_notifier); unregister_netdevice_notifier(&masq_dev_notifier);
err_dec: err_dec:
masq_refcnt--; masq_refcnt4--;
out_unlock: out_unlock:
mutex_unlock(&masq_mutex); mutex_unlock(&masq_mutex);
return ret; return ret;
...@@ -171,7 +177,7 @@ void nf_nat_masquerade_ipv4_unregister_notifier(void) ...@@ -171,7 +177,7 @@ void nf_nat_masquerade_ipv4_unregister_notifier(void)
{ {
mutex_lock(&masq_mutex); mutex_lock(&masq_mutex);
/* check if the notifier still has clients */ /* check if the notifier still has clients */
if (--masq_refcnt > 0) if (--masq_refcnt4 > 0)
goto out_unlock; goto out_unlock;
unregister_netdevice_notifier(&masq_dev_notifier); unregister_netdevice_notifier(&masq_dev_notifier);
...@@ -321,25 +327,23 @@ int nf_nat_masquerade_ipv6_register_notifier(void) ...@@ -321,25 +327,23 @@ int nf_nat_masquerade_ipv6_register_notifier(void)
int ret = 0; int ret = 0;
mutex_lock(&masq_mutex); mutex_lock(&masq_mutex);
/* check if the notifier is already set */ if (WARN_ON_ONCE(masq_refcnt6 == UINT_MAX)) {
if (++masq_refcnt > 1) ret = -EOVERFLOW;
goto out_unlock; goto out_unlock;
}
ret = register_netdevice_notifier(&masq_dev_notifier); /* check if the notifier is already set */
if (ret) if (++masq_refcnt6 > 1)
goto err_dec; goto out_unlock;
ret = register_inet6addr_notifier(&masq_inet6_notifier); ret = register_inet6addr_notifier(&masq_inet6_notifier);
if (ret) if (ret)
goto err_unregister; goto err_dec;
mutex_unlock(&masq_mutex); mutex_unlock(&masq_mutex);
return ret; return ret;
err_unregister:
unregister_netdevice_notifier(&masq_dev_notifier);
err_dec: err_dec:
masq_refcnt--; masq_refcnt6--;
out_unlock: out_unlock:
mutex_unlock(&masq_mutex); mutex_unlock(&masq_mutex);
return ret; return ret;
...@@ -350,11 +354,10 @@ void nf_nat_masquerade_ipv6_unregister_notifier(void) ...@@ -350,11 +354,10 @@ void nf_nat_masquerade_ipv6_unregister_notifier(void)
{ {
mutex_lock(&masq_mutex); mutex_lock(&masq_mutex);
/* check if the notifier still has clients */ /* check if the notifier still has clients */
if (--masq_refcnt > 0) if (--masq_refcnt6 > 0)
goto out_unlock; goto out_unlock;
unregister_inet6addr_notifier(&masq_inet6_notifier); unregister_inet6addr_notifier(&masq_inet6_notifier);
unregister_netdevice_notifier(&masq_dev_notifier);
out_unlock: out_unlock:
mutex_unlock(&masq_mutex); mutex_unlock(&masq_mutex);
} }
......
...@@ -142,7 +142,7 @@ static void nft_set_trans_bind(const struct nft_ctx *ctx, struct nft_set *set) ...@@ -142,7 +142,7 @@ static void nft_set_trans_bind(const struct nft_ctx *ctx, struct nft_set *set)
list_for_each_entry_reverse(trans, &net->nft.commit_list, list) { list_for_each_entry_reverse(trans, &net->nft.commit_list, list) {
if (trans->msg_type == NFT_MSG_NEWSET && if (trans->msg_type == NFT_MSG_NEWSET &&
nft_trans_set(trans) == set) { nft_trans_set(trans) == set) {
nft_trans_set_bound(trans) = true; set->bound = true;
break; break;
} }
} }
...@@ -2162,9 +2162,11 @@ static int nf_tables_newexpr(const struct nft_ctx *ctx, ...@@ -2162,9 +2162,11 @@ static int nf_tables_newexpr(const struct nft_ctx *ctx,
static void nf_tables_expr_destroy(const struct nft_ctx *ctx, static void nf_tables_expr_destroy(const struct nft_ctx *ctx,
struct nft_expr *expr) struct nft_expr *expr)
{ {
const struct nft_expr_type *type = expr->ops->type;
if (expr->ops->destroy) if (expr->ops->destroy)
expr->ops->destroy(ctx, expr); expr->ops->destroy(ctx, expr);
module_put(expr->ops->type->owner); module_put(type->owner);
} }
struct nft_expr *nft_expr_init(const struct nft_ctx *ctx, struct nft_expr *nft_expr_init(const struct nft_ctx *ctx,
...@@ -3672,6 +3674,9 @@ static int nf_tables_newset(struct net *net, struct sock *nlsk, ...@@ -3672,6 +3674,9 @@ static int nf_tables_newset(struct net *net, struct sock *nlsk,
static void nft_set_destroy(struct nft_set *set) static void nft_set_destroy(struct nft_set *set)
{ {
if (WARN_ON(set->use > 0))
return;
set->ops->destroy(set); set->ops->destroy(set);
module_put(to_set_type(set->ops)->owner); module_put(to_set_type(set->ops)->owner);
kfree(set->name); kfree(set->name);
...@@ -3712,7 +3717,7 @@ static int nf_tables_delset(struct net *net, struct sock *nlsk, ...@@ -3712,7 +3717,7 @@ static int nf_tables_delset(struct net *net, struct sock *nlsk,
NL_SET_BAD_ATTR(extack, attr); NL_SET_BAD_ATTR(extack, attr);
return PTR_ERR(set); return PTR_ERR(set);
} }
if (!list_empty(&set->bindings) || if (set->use ||
(nlh->nlmsg_flags & NLM_F_NONREC && atomic_read(&set->nelems) > 0)) { (nlh->nlmsg_flags & NLM_F_NONREC && atomic_read(&set->nelems) > 0)) {
NL_SET_BAD_ATTR(extack, attr); NL_SET_BAD_ATTR(extack, attr);
return -EBUSY; return -EBUSY;
...@@ -3742,6 +3747,9 @@ int nf_tables_bind_set(const struct nft_ctx *ctx, struct nft_set *set, ...@@ -3742,6 +3747,9 @@ int nf_tables_bind_set(const struct nft_ctx *ctx, struct nft_set *set,
struct nft_set_binding *i; struct nft_set_binding *i;
struct nft_set_iter iter; struct nft_set_iter iter;
if (set->use == UINT_MAX)
return -EOVERFLOW;
if (!list_empty(&set->bindings) && nft_set_is_anonymous(set)) if (!list_empty(&set->bindings) && nft_set_is_anonymous(set))
return -EBUSY; return -EBUSY;
...@@ -3769,6 +3777,7 @@ int nf_tables_bind_set(const struct nft_ctx *ctx, struct nft_set *set, ...@@ -3769,6 +3777,7 @@ int nf_tables_bind_set(const struct nft_ctx *ctx, struct nft_set *set,
binding->chain = ctx->chain; binding->chain = ctx->chain;
list_add_tail_rcu(&binding->list, &set->bindings); list_add_tail_rcu(&binding->list, &set->bindings);
nft_set_trans_bind(ctx, set); nft_set_trans_bind(ctx, set);
set->use++;
return 0; return 0;
} }
...@@ -3788,6 +3797,25 @@ void nf_tables_unbind_set(const struct nft_ctx *ctx, struct nft_set *set, ...@@ -3788,6 +3797,25 @@ void nf_tables_unbind_set(const struct nft_ctx *ctx, struct nft_set *set,
} }
EXPORT_SYMBOL_GPL(nf_tables_unbind_set); EXPORT_SYMBOL_GPL(nf_tables_unbind_set);
void nf_tables_deactivate_set(const struct nft_ctx *ctx, struct nft_set *set,
struct nft_set_binding *binding,
enum nft_trans_phase phase)
{
switch (phase) {
case NFT_TRANS_PREPARE:
set->use--;
return;
case NFT_TRANS_ABORT:
case NFT_TRANS_RELEASE:
set->use--;
/* fall through */
default:
nf_tables_unbind_set(ctx, set, binding,
phase == NFT_TRANS_COMMIT);
}
}
EXPORT_SYMBOL_GPL(nf_tables_deactivate_set);
void nf_tables_destroy_set(const struct nft_ctx *ctx, struct nft_set *set) void nf_tables_destroy_set(const struct nft_ctx *ctx, struct nft_set *set)
{ {
if (list_empty(&set->bindings) && nft_set_is_anonymous(set)) if (list_empty(&set->bindings) && nft_set_is_anonymous(set))
...@@ -6536,6 +6564,11 @@ static int nf_tables_commit(struct net *net, struct sk_buff *skb) ...@@ -6536,6 +6564,11 @@ static int nf_tables_commit(struct net *net, struct sk_buff *skb)
struct nft_chain *chain; struct nft_chain *chain;
struct nft_table *table; struct nft_table *table;
if (list_empty(&net->nft.commit_list)) {
mutex_unlock(&net->nft.commit_mutex);
return 0;
}
/* 0. Validate ruleset, otherwise roll back for error reporting. */ /* 0. Validate ruleset, otherwise roll back for error reporting. */
if (nf_tables_validate(net) < 0) if (nf_tables_validate(net) < 0)
return -EAGAIN; return -EAGAIN;
...@@ -6709,8 +6742,7 @@ static void nf_tables_abort_release(struct nft_trans *trans) ...@@ -6709,8 +6742,7 @@ static void nf_tables_abort_release(struct nft_trans *trans)
nf_tables_rule_destroy(&trans->ctx, nft_trans_rule(trans)); nf_tables_rule_destroy(&trans->ctx, nft_trans_rule(trans));
break; break;
case NFT_MSG_NEWSET: case NFT_MSG_NEWSET:
if (!nft_trans_set_bound(trans)) nft_set_destroy(nft_trans_set(trans));
nft_set_destroy(nft_trans_set(trans));
break; break;
case NFT_MSG_NEWSETELEM: case NFT_MSG_NEWSETELEM:
nft_set_elem_destroy(nft_trans_elem_set(trans), nft_set_elem_destroy(nft_trans_elem_set(trans),
...@@ -6783,8 +6815,11 @@ static int __nf_tables_abort(struct net *net) ...@@ -6783,8 +6815,11 @@ static int __nf_tables_abort(struct net *net)
break; break;
case NFT_MSG_NEWSET: case NFT_MSG_NEWSET:
trans->ctx.table->use--; trans->ctx.table->use--;
if (!nft_trans_set_bound(trans)) if (nft_trans_set(trans)->bound) {
list_del_rcu(&nft_trans_set(trans)->list); nft_trans_destroy(trans);
break;
}
list_del_rcu(&nft_trans_set(trans)->list);
break; break;
case NFT_MSG_DELSET: case NFT_MSG_DELSET:
trans->ctx.table->use++; trans->ctx.table->use++;
...@@ -6792,8 +6827,11 @@ static int __nf_tables_abort(struct net *net) ...@@ -6792,8 +6827,11 @@ static int __nf_tables_abort(struct net *net)
nft_trans_destroy(trans); nft_trans_destroy(trans);
break; break;
case NFT_MSG_NEWSETELEM: case NFT_MSG_NEWSETELEM:
if (nft_trans_elem_set(trans)->bound) {
nft_trans_destroy(trans);
break;
}
te = (struct nft_trans_elem *)trans->data; te = (struct nft_trans_elem *)trans->data;
te->set->ops->remove(net, te->set, &te->elem); te->set->ops->remove(net, te->set, &te->elem);
atomic_dec(&te->set->nelems); atomic_dec(&te->set->nelems);
break; break;
......
...@@ -240,11 +240,15 @@ static void nft_dynset_deactivate(const struct nft_ctx *ctx, ...@@ -240,11 +240,15 @@ static void nft_dynset_deactivate(const struct nft_ctx *ctx,
{ {
struct nft_dynset *priv = nft_expr_priv(expr); struct nft_dynset *priv = nft_expr_priv(expr);
if (phase == NFT_TRANS_PREPARE) nf_tables_deactivate_set(ctx, priv->set, &priv->binding, phase);
return; }
static void nft_dynset_activate(const struct nft_ctx *ctx,
const struct nft_expr *expr)
{
struct nft_dynset *priv = nft_expr_priv(expr);
nf_tables_unbind_set(ctx, priv->set, &priv->binding, priv->set->use++;
phase == NFT_TRANS_COMMIT);
} }
static void nft_dynset_destroy(const struct nft_ctx *ctx, static void nft_dynset_destroy(const struct nft_ctx *ctx,
...@@ -292,6 +296,7 @@ static const struct nft_expr_ops nft_dynset_ops = { ...@@ -292,6 +296,7 @@ static const struct nft_expr_ops nft_dynset_ops = {
.eval = nft_dynset_eval, .eval = nft_dynset_eval,
.init = nft_dynset_init, .init = nft_dynset_init,
.destroy = nft_dynset_destroy, .destroy = nft_dynset_destroy,
.activate = nft_dynset_activate,
.deactivate = nft_dynset_deactivate, .deactivate = nft_dynset_deactivate,
.dump = nft_dynset_dump, .dump = nft_dynset_dump,
}; };
......
...@@ -127,11 +127,15 @@ static void nft_lookup_deactivate(const struct nft_ctx *ctx, ...@@ -127,11 +127,15 @@ static void nft_lookup_deactivate(const struct nft_ctx *ctx,
{ {
struct nft_lookup *priv = nft_expr_priv(expr); struct nft_lookup *priv = nft_expr_priv(expr);
if (phase == NFT_TRANS_PREPARE) nf_tables_deactivate_set(ctx, priv->set, &priv->binding, phase);
return; }
static void nft_lookup_activate(const struct nft_ctx *ctx,
const struct nft_expr *expr)
{
struct nft_lookup *priv = nft_expr_priv(expr);
nf_tables_unbind_set(ctx, priv->set, &priv->binding, priv->set->use++;
phase == NFT_TRANS_COMMIT);
} }
static void nft_lookup_destroy(const struct nft_ctx *ctx, static void nft_lookup_destroy(const struct nft_ctx *ctx,
...@@ -222,6 +226,7 @@ static const struct nft_expr_ops nft_lookup_ops = { ...@@ -222,6 +226,7 @@ static const struct nft_expr_ops nft_lookup_ops = {
.size = NFT_EXPR_SIZE(sizeof(struct nft_lookup)), .size = NFT_EXPR_SIZE(sizeof(struct nft_lookup)),
.eval = nft_lookup_eval, .eval = nft_lookup_eval,
.init = nft_lookup_init, .init = nft_lookup_init,
.activate = nft_lookup_activate,
.deactivate = nft_lookup_deactivate, .deactivate = nft_lookup_deactivate,
.destroy = nft_lookup_destroy, .destroy = nft_lookup_destroy,
.dump = nft_lookup_dump, .dump = nft_lookup_dump,
......
...@@ -162,11 +162,15 @@ static void nft_objref_map_deactivate(const struct nft_ctx *ctx, ...@@ -162,11 +162,15 @@ static void nft_objref_map_deactivate(const struct nft_ctx *ctx,
{ {
struct nft_objref_map *priv = nft_expr_priv(expr); struct nft_objref_map *priv = nft_expr_priv(expr);
if (phase == NFT_TRANS_PREPARE) nf_tables_deactivate_set(ctx, priv->set, &priv->binding, phase);
return; }
static void nft_objref_map_activate(const struct nft_ctx *ctx,
const struct nft_expr *expr)
{
struct nft_objref_map *priv = nft_expr_priv(expr);
nf_tables_unbind_set(ctx, priv->set, &priv->binding, priv->set->use++;
phase == NFT_TRANS_COMMIT);
} }
static void nft_objref_map_destroy(const struct nft_ctx *ctx, static void nft_objref_map_destroy(const struct nft_ctx *ctx,
...@@ -183,6 +187,7 @@ static const struct nft_expr_ops nft_objref_map_ops = { ...@@ -183,6 +187,7 @@ static const struct nft_expr_ops nft_objref_map_ops = {
.size = NFT_EXPR_SIZE(sizeof(struct nft_objref_map)), .size = NFT_EXPR_SIZE(sizeof(struct nft_objref_map)),
.eval = nft_objref_map_eval, .eval = nft_objref_map_eval,
.init = nft_objref_map_init, .init = nft_objref_map_init,
.activate = nft_objref_map_activate,
.deactivate = nft_objref_map_deactivate, .deactivate = nft_objref_map_deactivate,
.destroy = nft_objref_map_destroy, .destroy = nft_objref_map_destroy,
.dump = nft_objref_map_dump, .dump = nft_objref_map_dump,
......
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