Re: [PATCH bpf-next v4 2/4] bpf: Prevent extending tail callee prog with freplace prog

[Leon Hwang <hffilwlqm@xxxxxxxxx>]

On 29/9/24 21:27, Leon Hwang wrote:
> Alongside previous patch, the infinite loop issue caused by combination of
> tailcal and freplace can be prevented completely.
> 
> The previous patch can not prevent the use case that updates a prog to
> prog_array map and then extends subprog of the prog with freplace prog.
> 
> This patch fixes the case by preventing extending a prog, which has been
> updated to prog_array map, with freplace prog.
> 
> If a prog has been updated to prog_array map, it or its subprog can not
> be extended by freplace prog.
> 
> Signed-off-by: Leon Hwang <leon.hwang@xxxxxxxxx>
> ---
>  include/linux/bpf.h   |  3 ++-
>  kernel/bpf/arraymap.c |  9 ++++++++-
>  kernel/bpf/syscall.c  | 11 +++++++++++
>  3 files changed, 21 insertions(+), 2 deletions(-)
> 
> diff --git a/include/linux/bpf.h b/include/linux/bpf.h
> index aac6d2f42830c..dc19ad99e2857 100644
> --- a/include/linux/bpf.h
> +++ b/include/linux/bpf.h
> @@ -1484,7 +1484,8 @@ struct bpf_prog_aux {
>  	bool exception_cb;
>  	bool exception_boundary;
>  	bool is_extended; /* true if extended by freplace program */
> -	struct mutex ext_mutex; /* mutex for is_extended */
> +	u32 prog_array_member_cnt; /* counts how many times as member of prog_array */
> +	struct mutex ext_mutex; /* mutex for is_extended and prog_array_member_cnt */
>  	struct bpf_arena *arena;
>  	/* BTF_KIND_FUNC_PROTO for valid attach_btf_id */
>  	const struct btf_type *attach_func_proto;
> diff --git a/kernel/bpf/arraymap.c b/kernel/bpf/arraymap.c
> index 4a4de4f014be9..91b5bdf4dc72d 100644
> --- a/kernel/bpf/arraymap.c
> +++ b/kernel/bpf/arraymap.c
> @@ -957,6 +957,8 @@ static void *prog_fd_array_get_ptr(struct bpf_map *map,
>  
>  	mutex_lock(&prog->aux->ext_mutex);
>  	is_extended = prog->aux->is_extended;
> +	if (!is_extended)
> +		prog->aux->prog_array_member_cnt++;

prog_array_member_cnt must check U32_MAX before incrementing. Or it will
overflow u32. So it will be better like:

	mutex_lock(&prog->aux->ext_mutex);
	is_invalid = prog->aux->is_extended || prog->aux->prog_array_member_cnt
== U32_MAX;
	if (!is_invalid)
		prog->aux->prog_array_member_cnt++;
	mutex_unlock(&prog->aux->ext_mutex);
	if (is_invalid)
		goto out_put_prog;

Thanks,
Leon