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 10:04 AM Suren Baghdasaryan <surenb@google.com> wrote:
>
> 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.

Fix is posted at
https://lore.kernel.org/all/20230301022720.1380780-1-surenb@google.com/

> 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
> >