diff mbox series

[RFC] memcg: Enable accounting for nft objects

Message ID 81d734aa-7a0f-81b4-34fb-516b17673eac@virtuozzo.com (mailing list archive)
State New
Headers show
Series [RFC] memcg: Enable accounting for nft objects | expand

Commit Message

Vasily Averin Feb. 28, 2022, 6:39 a.m. UTC 
nftables replaces iptables but still lacks memcg accounting.

This patch account most part of nft-related allocation and should protect host from nft misuse
inside memcg-limited container.

Signed-off-by: Vasily Averin <vvs@virtuozzo.com>
---
  net/netfilter/core.c          |  2 +-
  net/netfilter/nf_tables_api.c | 51 +++++++++++++++++++----------------
  2 files changed, 29 insertions(+), 24 deletions(-)

Comments

Florian Westphal Feb. 28, 2022, 12:24 p.m. UTC | #1 
Vasily Averin <vvs@virtuozzo.com> wrote:
> nftables replaces iptables but still lacks memcg accounting.
> 
> This patch account most part of nft-related allocation and should protect host from nft misuse
> inside memcg-limited container.
> 
> Signed-off-by: Vasily Averin <vvs@virtuozzo.com>
> ---
>  net/netfilter/core.c          |  2 +-
>  net/netfilter/nf_tables_api.c | 51 +++++++++++++++++++----------------
>  2 files changed, 29 insertions(+), 24 deletions(-)
> 
> diff --git a/net/netfilter/core.c b/net/netfilter/core.c
> index 354cb472f386..6a2b57774999 100644
> --- a/net/netfilter/core.c
> +++ b/net/netfilter/core.c
> @@ -58,7 +58,7 @@ static struct nf_hook_entries *allocate_hook_entries_size(u16 num)
>  	if (num == 0)
>  		return NULL;
> -	e = kvzalloc(alloc, GFP_KERNEL);
> +	e = kvzalloc(alloc, GFP_KERNEL_ACCOUNT);

makes sense to me.

> diff --git a/net/netfilter/nf_tables_api.c b/net/netfilter/nf_tables_api.c
> index 5fa16990da95..5e1987ec9715 100644
> --- a/net/netfilter/nf_tables_api.c
> +++ b/net/netfilter/nf_tables_api.c
> @@ -149,7 +149,7 @@ static struct nft_trans *nft_trans_alloc_gfp(const struct nft_ctx *ctx,
>  {
>  	struct nft_trans *trans;
> -	trans = kzalloc(sizeof(struct nft_trans) + size, gfp);
> +	trans = kzalloc(sizeof(struct nft_trans) + size, gfp | __GFP_ACCOUNT);

trans_alloc is temporary in nature, they are always free'd by the
time syscall returns (else, bug).

> @@ -1084,6 +1084,7 @@ static int nf_tables_newtable(struct sk_buff *skb, const struct nfnl_info *info,
>  	struct nft_table *table;
>  	struct nft_ctx ctx;
>  	u32 flags = 0;
> +	gfp_t gfp = GFP_KERNEL_ACCOUNT;
>  	int err;
>  	lockdep_assert_held(&nft_net->commit_mutex);
> @@ -1113,16 +1114,16 @@ static int nf_tables_newtable(struct sk_buff *skb, const struct nfnl_info *info,
>  	}
>  	err = -ENOMEM;
> -	table = kzalloc(sizeof(*table), GFP_KERNEL);
> +	table = kzalloc(sizeof(*table), gfp);

Why gfp temporary variable?  Readability? The subsititution looks correct.

Rest looks good, you might need to update nft_limit_init() and a few
other stateful expressions that alloc internal data too.
diff mbox series

Patch

diff --git a/net/netfilter/core.c b/net/netfilter/core.c
index 354cb472f386..6a2b57774999 100644
--- a/net/netfilter/core.c
+++ b/net/netfilter/core.c
@@ -58,7 +58,7 @@  static struct nf_hook_entries *allocate_hook_entries_size(u16 num)
  	if (num == 0)
  		return NULL;
  
-	e = kvzalloc(alloc, GFP_KERNEL);
+	e = kvzalloc(alloc, GFP_KERNEL_ACCOUNT);
  	if (e)
  		e->num_hook_entries = num;
  	return e;
diff --git a/net/netfilter/nf_tables_api.c b/net/netfilter/nf_tables_api.c
index 5fa16990da95..5e1987ec9715 100644
--- a/net/netfilter/nf_tables_api.c
+++ b/net/netfilter/nf_tables_api.c
@@ -149,7 +149,7 @@  static struct nft_trans *nft_trans_alloc_gfp(const struct nft_ctx *ctx,
  {
  	struct nft_trans *trans;
  
-	trans = kzalloc(sizeof(struct nft_trans) + size, gfp);
+	trans = kzalloc(sizeof(struct nft_trans) + size, gfp | __GFP_ACCOUNT);
  	if (trans == NULL)
  		return NULL;
  
@@ -1084,6 +1084,7 @@  static int nf_tables_newtable(struct sk_buff *skb, const struct nfnl_info *info,
  	struct nft_table *table;
  	struct nft_ctx ctx;
  	u32 flags = 0;
+	gfp_t gfp = GFP_KERNEL_ACCOUNT;
  	int err;
  
  	lockdep_assert_held(&nft_net->commit_mutex);
@@ -1113,16 +1114,16 @@  static int nf_tables_newtable(struct sk_buff *skb, const struct nfnl_info *info,
  	}
  
  	err = -ENOMEM;
-	table = kzalloc(sizeof(*table), GFP_KERNEL);
+	table = kzalloc(sizeof(*table), gfp);
  	if (table == NULL)
  		goto err_kzalloc;
  
-	table->name = nla_strdup(attr, GFP_KERNEL);
+	table->name = nla_strdup(attr, gfp);
  	if (table->name == NULL)
  		goto err_strdup;
  
  	if (nla[NFTA_TABLE_USERDATA]) {
-		table->udata = nla_memdup(nla[NFTA_TABLE_USERDATA], GFP_KERNEL);
+		table->udata = nla_memdup(nla[NFTA_TABLE_USERDATA], gfp);
  		if (table->udata == NULL)
  			goto err_table_udata;
  
@@ -1803,7 +1804,7 @@  static struct nft_hook *nft_netdev_hook_alloc(struct net *net,
  	struct nft_hook *hook;
  	int err;
  
-	hook = kmalloc(sizeof(struct nft_hook), GFP_KERNEL);
+	hook = kmalloc(sizeof(struct nft_hook), GFP_KERNEL_ACCOUNT);
  	if (!hook) {
  		err = -ENOMEM;
  		goto err_hook_alloc;
@@ -2026,7 +2027,7 @@  static struct nft_rule_blob *nf_tables_chain_alloc_rules(unsigned int size)
  	if (size > INT_MAX)
  		return NULL;
  
-	blob = kvmalloc(size, GFP_KERNEL);
+	blob = kvmalloc(size, GFP_KERNEL_ACCOUNT);
  	if (!blob)
  		return NULL;
  
@@ -2110,6 +2111,7 @@  static int nf_tables_addchain(struct nft_ctx *ctx, u8 family, u8 genmask,
  	struct nft_trans *trans;
  	struct nft_chain *chain;
  	unsigned int data_size;
+	gfp_t gfp = GFP_KERNEL_ACCOUNT;
  	int err;
  
  	if (table->use == UINT_MAX)
@@ -2126,7 +2128,7 @@  static int nf_tables_addchain(struct nft_ctx *ctx, u8 family, u8 genmask,
  		if (err < 0)
  			return err;
  
-		basechain = kzalloc(sizeof(*basechain), GFP_KERNEL);
+		basechain = kzalloc(sizeof(*basechain), gfp);
  		if (basechain == NULL) {
  			nft_chain_release_hook(&hook);
  			return -ENOMEM;
@@ -2156,7 +2158,7 @@  static int nf_tables_addchain(struct nft_ctx *ctx, u8 family, u8 genmask,
  		if (flags & NFT_CHAIN_HW_OFFLOAD)
  			return -EOPNOTSUPP;
  
-		chain = kzalloc(sizeof(*chain), GFP_KERNEL);
+		chain = kzalloc(sizeof(*chain), gfp);
  		if (chain == NULL)
  			return -ENOMEM;
  
@@ -2169,7 +2171,7 @@  static int nf_tables_addchain(struct nft_ctx *ctx, u8 family, u8 genmask,
  	chain->table = table;
  
  	if (nla[NFTA_CHAIN_NAME]) {
-		chain->name = nla_strdup(nla[NFTA_CHAIN_NAME], GFP_KERNEL);
+		chain->name = nla_strdup(nla[NFTA_CHAIN_NAME], gfp);
  	} else {
  		if (!(flags & NFT_CHAIN_BINDING)) {
  			err = -EINVAL;
@@ -2177,7 +2179,7 @@  static int nf_tables_addchain(struct nft_ctx *ctx, u8 family, u8 genmask,
  		}
  
  		snprintf(name, sizeof(name), "__chain%llu", ++chain_id);
-		chain->name = kstrdup(name, GFP_KERNEL);
+		chain->name = kstrdup(name, gfp);
  	}
  
  	if (!chain->name) {
@@ -2186,7 +2188,7 @@  static int nf_tables_addchain(struct nft_ctx *ctx, u8 family, u8 genmask,
  	}
  
  	if (nla[NFTA_CHAIN_USERDATA]) {
-		chain->udata = nla_memdup(nla[NFTA_CHAIN_USERDATA], GFP_KERNEL);
+		chain->udata = nla_memdup(nla[NFTA_CHAIN_USERDATA], gfp);
  		if (chain->udata == NULL) {
  			err = -ENOMEM;
  			goto err_destroy_chain;
@@ -2349,7 +2351,7 @@  static int nf_tables_updchain(struct nft_ctx *ctx, u8 genmask, u8 policy,
  		char *name;
  
  		err = -ENOMEM;
-		name = nla_strdup(nla[NFTA_CHAIN_NAME], GFP_KERNEL);
+		name = nla_strdup(nla[NFTA_CHAIN_NAME], GFP_KERNEL_ACCOUNT);
  		if (!name)
  			goto err;
  
@@ -2797,7 +2799,7 @@  static struct nft_expr *nft_expr_init(const struct nft_ctx *ctx,
  		goto err1;
  
  	err = -ENOMEM;
-	expr = kzalloc(expr_info.ops->size, GFP_KERNEL);
+	expr = kzalloc(expr_info.ops->size, GFP_KERNEL_ACCOUNT);
  	if (expr == NULL)
  		goto err2;
  
@@ -3405,7 +3407,7 @@  static int nf_tables_newrule(struct sk_buff *skb, const struct nfnl_info *info,
  	}
  
  	err = -ENOMEM;
-	rule = kzalloc(sizeof(*rule) + size + usize, GFP_KERNEL);
+	rule = kzalloc(sizeof(*rule) + size + usize, GFP_KERNEL_ACCOUNT);
  	if (rule == NULL)
  		goto err_release_expr;
  
@@ -3818,7 +3820,7 @@  static int nf_tables_set_alloc_name(struct nft_ctx *ctx, struct nft_set *set,
  		free_page((unsigned long)inuse);
  	}
  
-	set->name = kasprintf(GFP_KERNEL, name, min + n);
+	set->name = kasprintf(GFP_KERNEL_ACCOUNT, name, min + n);
  	if (!set->name)
  		return -ENOMEM;
  
@@ -4239,6 +4241,7 @@  static int nf_tables_newset(struct sk_buff *skb, const struct nfnl_info *info,
  	int err, i;
  	u16 udlen;
  	u64 size;
+	gfp_t gfp = GFP_KERNEL_ACCOUNT;
  
  	if (nla[NFTA_SET_TABLE] == NULL ||
  	    nla[NFTA_SET_NAME] == NULL ||
@@ -4382,11 +4385,12 @@  static int nf_tables_newset(struct sk_buff *skb, const struct nfnl_info *info,
  	alloc_size = sizeof(*set) + size + udlen;
  	if (alloc_size < size || alloc_size > INT_MAX)
  		return -ENOMEM;
-	set = kvzalloc(alloc_size, GFP_KERNEL);
+
+	set = kvzalloc(alloc_size, gfp);
  	if (!set)
  		return -ENOMEM;
  
-	name = nla_strdup(nla[NFTA_SET_NAME], GFP_KERNEL);
+	name = nla_strdup(nla[NFTA_SET_NAME], gfp);
  	if (!name) {
  		err = -ENOMEM;
  		goto err_set_name;
@@ -5921,7 +5925,7 @@  static int nft_add_set_elem(struct nft_ctx *ctx, struct nft_set *set,
  	err = -ENOMEM;
  	elem.priv = nft_set_elem_init(set, &tmpl, elem.key.val.data,
  				      elem.key_end.val.data, elem.data.val.data,
-				      timeout, expiration, GFP_KERNEL);
+				      timeout, expiration, GFP_KERNEL_ACCOUNT);
  	if (elem.priv == NULL)
  		goto err_parse_data;
  
@@ -6165,7 +6169,7 @@  static int nft_del_setelem(struct nft_ctx *ctx, struct nft_set *set,
  	err = -ENOMEM;
  	elem.priv = nft_set_elem_init(set, &tmpl, elem.key.val.data,
  				      elem.key_end.val.data, NULL, 0, 0,
-				      GFP_KERNEL);
+				      GFP_KERNEL_ACCOUNT);
  	if (elem.priv == NULL)
  		goto fail_elem;
  
@@ -6477,7 +6481,7 @@  static struct nft_object *nft_obj_init(const struct nft_ctx *ctx,
  	}
  
  	err = -ENOMEM;
-	obj = kzalloc(sizeof(*obj) + ops->size, GFP_KERNEL);
+	obj = kzalloc(sizeof(*obj) + ops->size, GFP_KERNEL_ACCOUNT);
  	if (!obj)
  		goto err2;
  
@@ -6638,7 +6642,7 @@  static int nf_tables_newobj(struct sk_buff *skb, const struct nfnl_info *info,
  	obj->key.table = table;
  	obj->handle = nf_tables_alloc_handle(table);
  
-	obj->key.name = nla_strdup(nla[NFTA_OBJ_NAME], GFP_KERNEL);
+	obj->key.name = nla_strdup(nla[NFTA_OBJ_NAME], GFP_KERNEL_ACCOUNT);
  	if (!obj->key.name) {
  		err = -ENOMEM;
  		goto err_strdup;
@@ -7364,6 +7368,7 @@  static int nf_tables_newflowtable(struct sk_buff *skb,
  	struct net *net = info->net;
  	struct nft_table *table;
  	struct nft_ctx ctx;
+	gfp_t gfp = GFP_KERNEL_ACCOUNT;
  	int err;
  
  	if (!nla[NFTA_FLOWTABLE_TABLE] ||
@@ -7399,7 +7404,7 @@  static int nf_tables_newflowtable(struct sk_buff *skb,
  
  	nft_ctx_init(&ctx, net, skb, info->nlh, family, table, NULL, nla);
  
-	flowtable = kzalloc(sizeof(*flowtable), GFP_KERNEL);
+	flowtable = kzalloc(sizeof(*flowtable), gfp);
  	if (!flowtable)
  		return -ENOMEM;
  
@@ -7407,7 +7412,7 @@  static int nf_tables_newflowtable(struct sk_buff *skb,
  	flowtable->handle = nf_tables_alloc_handle(table);
  	INIT_LIST_HEAD(&flowtable->hook_list);
  
-	flowtable->name = nla_strdup(nla[NFTA_FLOWTABLE_NAME], GFP_KERNEL);
+	flowtable->name = nla_strdup(nla[NFTA_FLOWTABLE_NAME], gfp);
  	if (!flowtable->name) {
  		err = -ENOMEM;
  		goto err1;