Commit ad9f151e authored by Pablo Neira Ayuso's avatar Pablo Neira Ayuso
Browse files

netfilter: nf_tables: initialize set before expression setup 



nft_set_elem_expr_alloc() needs an initialized set if expression sets on
the NFT_EXPR_GC flag. Move set fields initialization before expression
setup.

[4512935.019450] ==================================================================
[4512935.019456] BUG: KASAN: null-ptr-deref in nft_set_elem_expr_alloc+0x84/0xd0 [nf_tables]
[4512935.019487] Read of size 8 at addr 0000000000000070 by task nft/23532
[4512935.019494] CPU: 1 PID: 23532 Comm: nft Not tainted 5.12.0-rc4+ #48
[...]
[4512935.019502] Call Trace:
[4512935.019505]  dump_stack+0x89/0xb4
[4512935.019512]  ? nft_set_elem_expr_alloc+0x84/0xd0 [nf_tables]
[4512935.019536]  ? nft_set_elem_expr_alloc+0x84/0xd0 [nf_tables]
[4512935.019560]  kasan_report.cold.12+0x5f/0xd8
[4512935.019566]  ? nft_set_elem_expr_alloc+0x84/0xd0 [nf_tables]
[4512935.019590]  nft_set_elem_expr_alloc+0x84/0xd0 [nf_tables]
[4512935.019615]  nf_tables_newset+0xc7f/0x1460 [nf_tables]

Reported-by: default avatar <syzbot+ce96ca2b1d0b37c6422d@syzkaller.appspotmail.com>
Fixes: 65038428 ("netfilter: nf_tables: allow to specify stateful expression in set definition")
Signed-off-by: default avatarPablo Neira Ayuso <pablo@netfilter.org>
parent f2386cf7

net/netfilter/nf_tables_api.c

+43 −42
Original line number Diff line number Diff line
@@ -4364,13 +4364,45 @@ static int nf_tables_newset(struct sk_buff *skb, const struct nfnl_info *info, 
	err = nf_tables_set_alloc_name(&ctx, set, name);

	kfree(name);

	if (err < 0)

		goto err_set_alloc_name;

		goto err_set_name;



	udata = NULL;

	if (udlen) {

		udata = set->data + size;

		nla_memcpy(udata, nla[NFTA_SET_USERDATA], udlen);

	}



	INIT_LIST_HEAD(&set->bindings);

	INIT_LIST_HEAD(&set->catchall_list);

	set->table = table;

	write_pnet(&set->net, net);

	set->ops = ops;

	set->ktype = ktype;

	set->klen = desc.klen;

	set->dtype = dtype;

	set->objtype = objtype;

	set->dlen = desc.dlen;

	set->flags = flags;

	set->size = desc.size;

	set->policy = policy;

	set->udlen = udlen;

	set->udata = udata;

	set->timeout = timeout;

	set->gc_int = gc_int;



	set->field_count = desc.field_count;

	for (i = 0; i < desc.field_count; i++)

		set->field_len[i] = desc.field_len[i];



	err = ops->init(set, &desc, nla);

	if (err < 0)

		goto err_set_init;



	if (nla[NFTA_SET_EXPR]) {

		expr = nft_set_elem_expr_alloc(&ctx, set, nla[NFTA_SET_EXPR]);

		if (IS_ERR(expr)) {

			err = PTR_ERR(expr);

			goto err_set_alloc_name;

			goto err_set_expr_alloc;

		}

		set->exprs[0] = expr;

		set->num_exprs++;
@@ -4381,75 +4413,44 @@ static int nf_tables_newset(struct sk_buff *skb, const struct nfnl_info *info, 


		if (!(flags & NFT_SET_EXPR)) {

			err = -EINVAL;

			goto err_set_alloc_name;

			goto err_set_expr_alloc;

		}

		i = 0;

		nla_for_each_nested(tmp, nla[NFTA_SET_EXPRESSIONS], left) {

			if (i == NFT_SET_EXPR_MAX) {

				err = -E2BIG;

				goto err_set_init;

				goto err_set_expr_alloc;

			}

			if (nla_type(tmp) != NFTA_LIST_ELEM) {

				err = -EINVAL;

				goto err_set_init;

				goto err_set_expr_alloc;

			}

			expr = nft_set_elem_expr_alloc(&ctx, set, tmp);

			if (IS_ERR(expr)) {

				err = PTR_ERR(expr);

				goto err_set_init;

				goto err_set_expr_alloc;

			}

			set->exprs[i++] = expr;

			set->num_exprs++;

		}

	}



	udata = NULL;

	if (udlen) {

		udata = set->data + size;

		nla_memcpy(udata, nla[NFTA_SET_USERDATA], udlen);

	}



	INIT_LIST_HEAD(&set->bindings);

	INIT_LIST_HEAD(&set->catchall_list);

	set->table = table;

	write_pnet(&set->net, net);

	set->ops   = ops;

	set->ktype = ktype;

	set->klen  = desc.klen;

	set->dtype = dtype;

	set->objtype = objtype;

	set->dlen  = desc.dlen;

	set->flags = flags;

	set->size  = desc.size;

	set->policy = policy;

	set->udlen  = udlen;

	set->udata  = udata;

	set->timeout = timeout;

	set->gc_int = gc_int;

	set->handle = nf_tables_alloc_handle(table);



	set->field_count = desc.field_count;

	for (i = 0; i < desc.field_count; i++)

		set->field_len[i] = desc.field_len[i];



	err = ops->init(set, &desc, nla);

	if (err < 0)

		goto err_set_init;



	err = nft_trans_set_add(&ctx, NFT_MSG_NEWSET, set);

	if (err < 0)

		goto err_set_trans;

		goto err_set_expr_alloc;



	list_add_tail_rcu(&set->list, &table->sets);

	table->use++;

	return 0;



err_set_trans:

	ops->destroy(set);

err_set_init:

err_set_expr_alloc:

	for (i = 0; i < set->num_exprs; i++)

		nft_expr_destroy(&ctx, set->exprs[i]);

err_set_alloc_name:



	ops->destroy(set);

err_set_init:

	kfree(set->name);

err_set_name:

	kvfree(set);