Re: [PATCH v2 bpf] bpf: Fix remap of arena.

[Pengfei Xu <pengfei.xu@intel.com>]

The issue cannot be reproduced in the new patch below.

Best Regards,
Thanks!

On 2024-06-17 at 10:18:12 -0700, Alexei Starovoitov wrote:
> From: Alexei Starovoitov <ast@kernel.org>
> 
> The bpf arena logic didn't account for mremap operation. Add a refcnt for
> multiple mmap events to prevent use-after-free in arena_vm_close.
> 
> Reported-by: Pengfei Xu <pengfei.xu@intel.com>
> Closes: https://lore.kernel.org/bpf/Zmuw29IhgyPNKnIM@xpf.sh.intel.com/
> Fixes: 317460317a02 ("bpf: Introduce bpf_arena.")
> Signed-off-by: Alexei Starovoitov <ast@kernel.org>
> ---
>  kernel/bpf/arena.c | 18 ++++++++++++++++--
>  1 file changed, 16 insertions(+), 2 deletions(-)
> 
> diff --git a/kernel/bpf/arena.c b/kernel/bpf/arena.c
> index 583ee4fe48ef..fe35b45cf6f8 100644
> --- a/kernel/bpf/arena.c
> +++ b/kernel/bpf/arena.c
> @@ -212,6 +212,7 @@ static u64 arena_map_mem_usage(const struct bpf_map *map)
>  struct vma_list {
>  	struct vm_area_struct *vma;
>  	struct list_head head;
> +	atomic_t mmap_count;
>  };
>  
>  static int remember_vma(struct bpf_arena *arena, struct vm_area_struct *vma)
> @@ -221,20 +222,32 @@ static int remember_vma(struct bpf_arena *arena, struct vm_area_struct *vma)
>  	vml = kmalloc(sizeof(*vml), GFP_KERNEL);
>  	if (!vml)
>  		return -ENOMEM;
> +	atomic_set(&vml->mmap_count, 1);
>  	vma->vm_private_data = vml;
>  	vml->vma = vma;
>  	list_add(&vml->head, &arena->vma_list);
>  	return 0;
>  }
>  
> +static void arena_vm_open(struct vm_area_struct *vma)
> +{
> +	struct bpf_map *map = vma->vm_file->private_data;
> +	struct bpf_arena *arena = container_of(map, struct bpf_arena, map);
> +	struct vma_list *vml = vma->vm_private_data;
> +
> +	atomic_inc(&vml->mmap_count);
> +}
> +
>  static void arena_vm_close(struct vm_area_struct *vma)
>  {
>  	struct bpf_map *map = vma->vm_file->private_data;
>  	struct bpf_arena *arena = container_of(map, struct bpf_arena, map);
> -	struct vma_list *vml;
> +	struct vma_list *vml = vma->vm_private_data;
>  
> +	if (!atomic_dec_and_test(&vml->mmap_count))
> +		return;
>  	guard(mutex)(&arena->lock);
> -	vml = vma->vm_private_data;
> +	/* update link list under lock */
>  	list_del(&vml->head);
>  	vma->vm_private_data = NULL;
>  	kfree(vml);
> @@ -287,6 +300,7 @@ static vm_fault_t arena_vm_fault(struct vm_fault *vmf)
>  }
>  
>  static const struct vm_operations_struct arena_vm_ops = {
> +	.open		= arena_vm_open,
>  	.close		= arena_vm_close,
>  	.fault          = arena_vm_fault,
>  };
> -- 
> 2.43.0
>