There is module leak in the error path of the nf_tables_newrule. In order to solve this, a member nft_expr_type *type is added into the nft_expr_info. so that, we can make separated error path of the nft_expr_ops and the nft_expr_type. So that, the nf_tables_rule_destroy is not used in the error path of the nf_tables_newrule anymore. Steps to reproduce: $iptables-compat -I OUTPUT -m cpu --cpu 0 $iptables-compat -F $lsmod Module Size Used by xt_cpu 16384 1 Signed-off-by: Taehee Yoo <ap420073@xxxxxxxxx> --- net/netfilter/nf_tables_api.c | 46 +++++++++++++++++++++++++++++-------------- 1 file changed, 31 insertions(+), 15 deletions(-) diff --git a/net/netfilter/nf_tables_api.c b/net/netfilter/nf_tables_api.c index 9134cc4..981f35e 100644 --- a/net/netfilter/nf_tables_api.c +++ b/net/netfilter/nf_tables_api.c @@ -1814,6 +1814,7 @@ int nft_expr_dump(struct sk_buff *skb, unsigned int attr, struct nft_expr_info { const struct nft_expr_ops *ops; + const struct nft_expr_type *type; struct nlattr *tb[NFT_EXPR_MAXATTR + 1]; }; @@ -1853,6 +1854,7 @@ static int nf_tables_expr_parse(const struct nft_ctx *ctx, ops = type->ops; info->ops = ops; + info->type = type; return 0; err1: @@ -1895,9 +1897,14 @@ static int nf_tables_newexpr(const struct nft_ctx *ctx, static void nf_tables_expr_destroy(const struct nft_ctx *ctx, struct nft_expr *expr) { + struct module *module = expr->ops->type->owner; + if (expr->ops->destroy) expr->ops->destroy(ctx, expr); - module_put(expr->ops->type->owner); + if (expr->ops->type->release) + expr->ops->type->release(expr->ops); + + module_put(module); } struct nft_expr *nft_expr_init(const struct nft_ctx *ctx, @@ -1922,9 +1929,11 @@ struct nft_expr *nft_expr_init(const struct nft_ctx *ctx, return expr; err3: + if (info.type->release) + info.type->release(info.ops); kfree(expr); err2: - module_put(info.ops->type->owner); + module_put(info.type->owner); err1: return ERR_PTR(err); } @@ -2258,7 +2267,7 @@ static int nf_tables_newrule(struct net *net, struct sock *nlsk, struct nft_expr *expr; struct nft_ctx ctx; struct nlattr *tmp; - unsigned int size, i, n, ulen = 0, usize = 0; + unsigned int size, i, n_type, n_ops, ulen = 0, usize = 0; int err, rem; bool create; u64 handle, pos_handle; @@ -2307,20 +2316,20 @@ static int nf_tables_newrule(struct net *net, struct sock *nlsk, nft_ctx_init(&ctx, net, skb, nlh, family, table, chain, nla); - n = 0; + n_type = 0; size = 0; if (nla[NFTA_RULE_EXPRESSIONS]) { nla_for_each_nested(tmp, nla[NFTA_RULE_EXPRESSIONS], rem) { err = -EINVAL; if (nla_type(tmp) != NFTA_LIST_ELEM) goto err1; - if (n == NFT_RULE_MAXEXPRS) + if (n_type == NFT_RULE_MAXEXPRS) goto err1; - err = nf_tables_expr_parse(&ctx, tmp, &info[n]); + err = nf_tables_expr_parse(&ctx, tmp, &info[n_type]); if (err < 0) goto err1; - size += info[n].ops->size; - n++; + size += info[n_type].ops->size; + n_type++; } } /* Check for overflow of dlen field */ @@ -2352,11 +2361,10 @@ static int nf_tables_newrule(struct net *net, struct sock *nlsk, } expr = nft_expr_first(rule); - for (i = 0; i < n; i++) { - err = nf_tables_newexpr(&ctx, &info[i], expr); + for (n_ops = 0; n_ops < n_type; n_ops++) { + err = nf_tables_newexpr(&ctx, &info[n_ops], expr); if (err < 0) goto err2; - info[i].ops = NULL; expr = nft_expr_next(expr); } @@ -2397,11 +2405,19 @@ static int nf_tables_newrule(struct net *net, struct sock *nlsk, err3: list_del_rcu(&rule->list); err2: - nf_tables_rule_destroy(&ctx, rule); + expr = nft_expr_first(rule); + for (i = 0; i < n_ops; i++) { + if (expr->ops && expr->ops->destroy) + expr->ops->destroy(&ctx, expr); + expr = nft_expr_next(expr); + } + kfree(rule); + err1: - for (i = 0; i < n; i++) { - if (info[i].ops != NULL) - module_put(info[i].ops->type->owner); + for (i = 0; i < n_type; i++) { + if (info[i].type->release) + info[i].type->release(info[i].ops); + module_put(info[i].type->owner); } return err; } -- 2.9.3 -- To unsubscribe from this list: send the line "unsubscribe netfilter-devel" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html