Re: [PATCH bpf-next] bpf: check max_entries before allocating memory

[Martin KaFai Lau <martin.lau@linux.dev>]

On 10/26/22 1:50 AM, Florian Lehner wrote:
> For maps of type BPF_MAP_TYPE_CPUMAP memory is allocated first before
> checking the max_entries argument. If then max_entries is greater than
> NR_CPUS additional work needs to be done to free allocated memory before
> an error is returned.
> This changes moves the check on max_entries before the allocation
> happens.
> 
> Signed-off-by: Florian Lehner <dev@der-flo.net>
> ---
>   kernel/bpf/cpumap.c | 19 ++++++++-----------
>   1 file changed, 8 insertions(+), 11 deletions(-)
> 
> diff --git a/kernel/bpf/cpumap.c b/kernel/bpf/cpumap.c
> index b5ba34ddd4b6..87e9f89a8140 100644
> --- a/kernel/bpf/cpumap.c
> +++ b/kernel/bpf/cpumap.c
> @@ -97,29 +97,26 @@ static struct bpf_map *cpu_map_alloc(union bpf_attr *attr)
>   	    attr->map_flags & ~BPF_F_NUMA_NODE)
>   		return ERR_PTR(-EINVAL);
>   
> +	/* Pre-limit array size based on NR_CPUS, not final CPU check */
> +	if (attr->max_entries > NR_CPUS)
> +		return ERR_PTR(-E2BIG);
> +
>   	cmap = bpf_map_area_alloc(sizeof(*cmap), NUMA_NO_NODE);
>   	if (!cmap)
>   		return ERR_PTR(-ENOMEM);
>   
>   	bpf_map_init_from_attr(&cmap->map, attr);
>   
> -	/* Pre-limit array size based on NR_CPUS, not final CPU check */
> -	if (cmap->map.max_entries > NR_CPUS) {
> -		err = -E2BIG;
> -		goto free_cmap;
> -	}
> -
>   	/* Alloc array for possible remote "destination" CPUs */
>   	cmap->cpu_map = bpf_map_area_alloc(cmap->map.max_entries *
>   					   sizeof(struct bpf_cpu_map_entry *),
>   					   cmap->map.numa_node);
> -	if (!cmap->cpu_map)
> -		goto free_cmap;
> +	if (!cmap->cpu_map) {
> +		bpf_map_area_free(cmap);
> +		return ERR_PTR(err);

lgtm.  May as well take this chance to remove the "err" variable and directly 
return ERR_PTR(-ENOMEM) instead.

> +	}
>   
>   	return &cmap->map;
> -free_cmap:
> -	bpf_map_area_free(cmap);
> -	return ERR_PTR(err);
>   }
>   
>   static void get_cpu_map_entry(struct bpf_cpu_map_entry *rcpu)