Commit e7c677bd authored by Alexei Starovoitov's avatar Alexei Starovoitov
Browse files

Merge branch 'fixes for bpf map iterator' 

Hou Tao says:

====================

From: Hou Tao <houtao1@huawei.com>

Hi,

The patchset constitues three fixes for bpf map iterator:

(1) patch 1~4: fix user-after-free during reading map iterator fd
It is possible when both the corresponding link fd and map fd are
closed bfore reading the iterator fd. I had squashed these four patches
into one, but it was not friendly for stable backport, so I break these
fixes into four single patches in the end. Patch 7 is its testing patch.

(2) patch 5: fix invalidity check for values in sk local storage map
Patch 8 adds two tests for it.

(3) patch 6: reject sleepable program for non-resched map iterator
Patch 9 add a test for it.

Please check the individual patches for more details. And comments are
always welcome.

Regards,
Tao

Changes since v2:
* patch 1~6: update commit messages (from Yonghong & Martin)
* patch 7: add more detailed comments (from Yonghong)
* patch 8: use NULL directly instead of (void *)0

v1: https://lore.kernel.org/bpf/20220806074019.2756957-1-houtao@huaweicloud.com


====================

Signed-off-by: default avatarAlexei Starovoitov <ast@kernel.org>
parents 86f44fce c5c0981f

kernel/bpf/arraymap.c

+6 −0
Original line number Diff line number Diff line
@@ -649,6 +649,11 @@ static int bpf_iter_init_array_map(void *priv_data, 
		seq_info->percpu_value_buf = value_buf;

	}



	/* bpf_iter_attach_map() acquires a map uref, and the uref may be

	 * released before or in the middle of iterating map elements, so

	 * acquire an extra map uref for iterator.

	 */

	bpf_map_inc_with_uref(map);

	seq_info->map = map;

	return 0;

}
@@ -657,6 +662,7 @@ static void bpf_iter_fini_array_map(void *priv_data) 
{

	struct bpf_iter_seq_array_map_info *seq_info = priv_data;



	bpf_map_put_with_uref(seq_info->map);

	kfree(seq_info->percpu_value_buf);

}

kernel/bpf/bpf_iter.c

+10 −1
Original line number Diff line number Diff line
@@ -68,13 +68,18 @@ static void bpf_iter_done_stop(struct seq_file *seq) 
	iter_priv->done_stop = true;

}



static inline bool bpf_iter_target_support_resched(const struct bpf_iter_target_info *tinfo)

{

	return tinfo->reg_info->feature & BPF_ITER_RESCHED;

}



static bool bpf_iter_support_resched(struct seq_file *seq)

{

	struct bpf_iter_priv_data *iter_priv;



	iter_priv = container_of(seq->private, struct bpf_iter_priv_data,

				 target_private);

	return iter_priv->tinfo->reg_info->feature & BPF_ITER_RESCHED;

	return bpf_iter_target_support_resched(iter_priv->tinfo);

}



/* maximum visited objects before bailing out */
@@ -537,6 +542,10 @@ int bpf_iter_link_attach(const union bpf_attr *attr, bpfptr_t uattr, 
	if (!tinfo)

		return -ENOENT;



	/* Only allow sleepable program for resched-able iterator */

	if (prog->aux->sleepable && !bpf_iter_target_support_resched(tinfo))

		return -EINVAL;



	link = kzalloc(sizeof(*link), GFP_USER | __GFP_NOWARN);

	if (!link)

		return -ENOMEM;

kernel/bpf/hashtab.c

+2 −0
Original line number Diff line number Diff line
@@ -2060,6 +2060,7 @@ static int bpf_iter_init_hash_map(void *priv_data, 
		seq_info->percpu_value_buf = value_buf;

	}



	bpf_map_inc_with_uref(map);

	seq_info->map = map;

	seq_info->htab = container_of(map, struct bpf_htab, map);

	return 0;
@@ -2069,6 +2070,7 @@ static void bpf_iter_fini_hash_map(void *priv_data) 
{

	struct bpf_iter_seq_hash_map_info *seq_info = priv_data;



	bpf_map_put_with_uref(seq_info->map);

	kfree(seq_info->percpu_value_buf);

}

net/core/bpf_sk_storage.c

+10 −2
Original line number Diff line number Diff line
@@ -875,10 +875,18 @@ static int bpf_iter_init_sk_storage_map(void *priv_data, 
{

	struct bpf_iter_seq_sk_storage_map_info *seq_info = priv_data;



	bpf_map_inc_with_uref(aux->map);

	seq_info->map = aux->map;

	return 0;

}



static void bpf_iter_fini_sk_storage_map(void *priv_data)

{

	struct bpf_iter_seq_sk_storage_map_info *seq_info = priv_data;



	bpf_map_put_with_uref(seq_info->map);

}



static int bpf_iter_attach_map(struct bpf_prog *prog,

			       union bpf_iter_link_info *linfo,

			       struct bpf_iter_aux_info *aux)
@@ -896,7 +904,7 @@ static int bpf_iter_attach_map(struct bpf_prog *prog, 
	if (map->map_type != BPF_MAP_TYPE_SK_STORAGE)

		goto put_map;



	if (prog->aux->max_rdonly_access > map->value_size) {

	if (prog->aux->max_rdwr_access > map->value_size) {

		err = -EACCES;

		goto put_map;

	}
@@ -924,7 +932,7 @@ static const struct seq_operations bpf_sk_storage_map_seq_ops = { 
static const struct bpf_iter_seq_info iter_seq_info = {

	.seq_ops		= &bpf_sk_storage_map_seq_ops,

	.init_seq_private	= bpf_iter_init_sk_storage_map,

	.fini_seq_private	= NULL,

	.fini_seq_private	= bpf_iter_fini_sk_storage_map,

	.seq_priv_size		= sizeof(struct bpf_iter_seq_sk_storage_map_info),

};

net/core/sock_map.c

+19 −1
Original line number Diff line number Diff line
@@ -783,13 +783,22 @@ static int sock_map_init_seq_private(void *priv_data, 
{

	struct sock_map_seq_info *info = priv_data;



	bpf_map_inc_with_uref(aux->map);

	info->map = aux->map;

	return 0;

}



static void sock_map_fini_seq_private(void *priv_data)

{

	struct sock_map_seq_info *info = priv_data;



	bpf_map_put_with_uref(info->map);

}



static const struct bpf_iter_seq_info sock_map_iter_seq_info = {

	.seq_ops		= &sock_map_seq_ops,

	.init_seq_private	= sock_map_init_seq_private,

	.fini_seq_private	= sock_map_fini_seq_private,

	.seq_priv_size		= sizeof(struct sock_map_seq_info),

};
@@ -1373,14 +1382,23 @@ static int sock_hash_init_seq_private(void *priv_data, 
{

	struct sock_hash_seq_info *info = priv_data;



	bpf_map_inc_with_uref(aux->map);

	info->map = aux->map;

	info->htab = container_of(aux->map, struct bpf_shtab, map);

	return 0;

}



static void sock_hash_fini_seq_private(void *priv_data)

{

	struct sock_hash_seq_info *info = priv_data;



	bpf_map_put_with_uref(info->map);

}



static const struct bpf_iter_seq_info sock_hash_iter_seq_info = {

	.seq_ops		= &sock_hash_seq_ops,

	.init_seq_private	= sock_hash_init_seq_private,

	.fini_seq_private	= sock_hash_fini_seq_private,

	.seq_priv_size		= sizeof(struct sock_hash_seq_info),

};