Re: [ammarfaizi2-block:akpm/mm/mm-unstable 82/99] mm/mmap.c:516 vma_prepare() error: we previously assumed 'vp->vma' could be null (see line 505)

[Suren Baghdasaryan <surenb@google.com>]

On Tue, Feb 28, 2023 at 6:24 AM Dan Carpenter <error27@gmail.com> wrote:
>
> tree:   https://github.com/ammarfaizi2/linux-block akpm/mm/mm-unstable
> head:   61edd3b68c3185673c9b05dfe48038692964c73b
> commit: f517f7ae341d933856cdf4d9d773027681ed5dff [82/99] mm/mmap: write-lock VMAs in vma_prepare before modifying them
> config: i386-randconfig-m021 (https://download.01.org/0day-ci/archive/20230228/202302281802.J93Nma7q-lkp@intel.com/config)
> compiler: gcc-11 (Debian 11.3.0-8) 11.3.0
>
> If you fix the issue, kindly add following tag where applicable
> | Reported-by: kernel test robot <lkp@intel.com>
> | Reported-by: Dan Carpenter <error27@gmail.com>
> | Link: https://lore.kernel.org/r/202302281802.J93Nma7q-lkp@intel.com/

Thanks for reporting!

>
> smatch warnings:
> mm/mmap.c:516 vma_prepare() error: we previously assumed 'vp->vma' could be null (see line 505)
>
> vim +516 mm/mmap.c
>
> 440703e082b9c7 Liam R. Howlett    2023-01-20  503  static inline void vma_prepare(struct vma_prepare *vp)
> 440703e082b9c7 Liam R. Howlett    2023-01-20  504  {
> f517f7ae341d93 Suren Baghdasaryan 2023-02-27 @505       if (vp->vma)
>
> Check presumes that vp->vma can be NULL.  On my other system (with
> yesterday's linux-next code) vp->vma can never be NULL.

I think the check here is not needed since vp->vma should always be
non-NULL. I'll double-check and will remove it if that is confirmed.
Thanks,
Suren.

>
> f517f7ae341d93 Suren Baghdasaryan 2023-02-27  506               vma_start_write(vp->vma);
> f517f7ae341d93 Suren Baghdasaryan 2023-02-27  507       if (vp->adj_next)
> f517f7ae341d93 Suren Baghdasaryan 2023-02-27  508               vma_start_write(vp->adj_next);
> f517f7ae341d93 Suren Baghdasaryan 2023-02-27  509       /* vp->insert is always a newly created VMA, no need for locking */
> f517f7ae341d93 Suren Baghdasaryan 2023-02-27  510       if (vp->remove)
> f517f7ae341d93 Suren Baghdasaryan 2023-02-27  511               vma_start_write(vp->remove);
> f517f7ae341d93 Suren Baghdasaryan 2023-02-27  512       if (vp->remove2)
> f517f7ae341d93 Suren Baghdasaryan 2023-02-27  513               vma_start_write(vp->remove2);
> f517f7ae341d93 Suren Baghdasaryan 2023-02-27  514
> 440703e082b9c7 Liam R. Howlett    2023-01-20  515       if (vp->file) {
> 440703e082b9c7 Liam R. Howlett    2023-01-20 @516               uprobe_munmap(vp->vma, vp->vma->vm_start, vp->vma->vm_end);
>                                                                                        ^^^^^^^
> Uncheck dereference.
>
> 440703e082b9c7 Liam R. Howlett    2023-01-20  517
> 440703e082b9c7 Liam R. Howlett    2023-01-20  518               if (vp->adj_next)
> 440703e082b9c7 Liam R. Howlett    2023-01-20  519                       uprobe_munmap(vp->adj_next, vp->adj_next->vm_start,
> 440703e082b9c7 Liam R. Howlett    2023-01-20  520                                     vp->adj_next->vm_end);
> 440703e082b9c7 Liam R. Howlett    2023-01-20  521
> 440703e082b9c7 Liam R. Howlett    2023-01-20  522               i_mmap_lock_write(vp->mapping);
> 440703e082b9c7 Liam R. Howlett    2023-01-20  523               if (vp->insert && vp->insert->vm_file) {
> 440703e082b9c7 Liam R. Howlett    2023-01-20  524                       /*
> 440703e082b9c7 Liam R. Howlett    2023-01-20  525                        * Put into interval tree now, so instantiated pages
> 440703e082b9c7 Liam R. Howlett    2023-01-20  526                        * are visible to arm/parisc __flush_dcache_page
> 440703e082b9c7 Liam R. Howlett    2023-01-20  527                        * throughout; but we cannot insert into address
> 440703e082b9c7 Liam R. Howlett    2023-01-20  528                        * space until vma start or end is updated.
> 440703e082b9c7 Liam R. Howlett    2023-01-20  529                        */
> 440703e082b9c7 Liam R. Howlett    2023-01-20  530                       __vma_link_file(vp->insert,
> 440703e082b9c7 Liam R. Howlett    2023-01-20  531                                       vp->insert->vm_file->f_mapping);
> 440703e082b9c7 Liam R. Howlett    2023-01-20  532               }
> 440703e082b9c7 Liam R. Howlett    2023-01-20  533       }
> 440703e082b9c7 Liam R. Howlett    2023-01-20  534
> 440703e082b9c7 Liam R. Howlett    2023-01-20  535       if (vp->anon_vma) {
> 440703e082b9c7 Liam R. Howlett    2023-01-20  536               anon_vma_lock_write(vp->anon_vma);
> 440703e082b9c7 Liam R. Howlett    2023-01-20  537               anon_vma_interval_tree_pre_update_vma(vp->vma);
>
> More unchecked dereferences.
>
> 440703e082b9c7 Liam R. Howlett    2023-01-20  538               if (vp->adj_next)
> 440703e082b9c7 Liam R. Howlett    2023-01-20  539                       anon_vma_interval_tree_pre_update_vma(vp->adj_next);
> 440703e082b9c7 Liam R. Howlett    2023-01-20  540       }
> 440703e082b9c7 Liam R. Howlett    2023-01-20  541
> 440703e082b9c7 Liam R. Howlett    2023-01-20  542       if (vp->file) {
> 440703e082b9c7 Liam R. Howlett    2023-01-20  543               flush_dcache_mmap_lock(vp->mapping);
> 440703e082b9c7 Liam R. Howlett    2023-01-20  544               vma_interval_tree_remove(vp->vma, &vp->mapping->i_mmap);
> 440703e082b9c7 Liam R. Howlett    2023-01-20  545               if (vp->adj_next)
> 440703e082b9c7 Liam R. Howlett    2023-01-20  546                       vma_interval_tree_remove(vp->adj_next,
> 440703e082b9c7 Liam R. Howlett    2023-01-20  547                                                &vp->mapping->i_mmap);
> 440703e082b9c7 Liam R. Howlett    2023-01-20  548       }
>
> --
> 0-DAY CI Kernel Test Service
> https://github.com/intel/lkp-tests
>