On Wed, Sep 08, 2021 at 06:30:52PM +0000, Liam Howlett wrote:
> /* Look up the first VMA which satisfies addr < vm_end, NULL if none. */
> -struct vm_area_struct *find_vma(struct mm_struct *mm, unsigned long addr)
> +struct vm_area_struct *find_vma_non_owner(struct mm_struct *mm,
> + unsigned long addr)
> {
> struct rb_node *rb_node;
> struct vm_area_struct *vma;
>
> - mmap_assert_locked(mm);
> + VM_BUG_ON_MM(!rwsem_is_locked(&mm->mmap_lock), mm);
> /* Check the cache first. */
> vma = vmacache_find(mm, addr);
> if (likely(vma))
> @@ -2325,6 +2326,11 @@ struct vm_area_struct *find_vma(struct mm_struct *mm, unsigned long addr)
> return vma;
> }
>
> +struct vm_area_struct *find_vma(struct mm_struct *mm, unsigned long addr)
> +{
> + lockdep_assert_held(&mm->mmap_lock);
> + return find_vma_non_owner(mm, addr);
> +}
> EXPORT_SYMBOL(find_vma);
>
> /*
>
>
> Although this leaks more into the mm API and was referred to as ugly
> previously, it does provide a working solution and still maintains the
> same level of checking.
I think it is no better than before.
The solution must be to not break lockdep in the BPF side. If Peter's
reworked algorithm is not OK then BPF should drop/acquire the lockdep
when it punts the unlock to the WQ.
'non-owner' is just a nice way of saying 'the caller is messing with
lockdep', it is not a sane way to design APIs
Jason
