Re: [bug report] mm/mremap: complete refactor of move_vma()

[Lorenzo Stoakes <lorenzo.stoakes@oracle.com>]

On Wed, Jun 25, 2025 at 10:22:49AM -0500, Dan Carpenter wrote:
> Hello Lorenzo Stoakes,
>
> The patch b714ccb02a76: "mm/mremap: complete refactor of move_vma()"
> from Mar 10, 2025, leads to the following static checker warning:
>
> 	mm/mremap.c:1920 move_vma()
> 	error: uninitialized symbol 'new_vma'.
>
> mm/mremap.c
>     1895 static unsigned long move_vma(struct vma_remap_struct *vrm)
>     1896 {
>     1897 	struct mm_struct *mm = current->mm;
>     1898 	struct vm_area_struct *new_vma;
>     1899 	unsigned long hiwater_vm;
>     1900 	int err;
>     1901
>     1902 	err = prep_move_vma(vrm);
>     1903 	if (err)
>     1904 		return err;
>     1905
>     1906 	/* If accounted, charge the number of bytes the operation will use. */
>     1907 	if (!vrm_charge(vrm))
>     1908 		return -ENOMEM;
>     1909
>     1910 	/* We don't want racing faults. */
>     1911 	vma_start_write(vrm->vma);
>     1912
>     1913 	/* Perform copy step. */
>     1914 	err = copy_vma_and_data(vrm, &new_vma);
>     1915 	/*
>     1916 	 * If we established the copied-to VMA, we attempt to recover from the
>     1917 	 * error by setting the destination VMA to the source VMA and unmapping
>     1918 	 * it below.
>     1919 	 */
> --> 1920 	if (err && !new_vma)
>                             ^^^^^^^
> new_vma isn't set on the first error path in copy_vma_and_data().

Hmm, the first error path is:

	if (!new_vma) {
		vrm_uncharge(vrm);
		*new_vma_ptr = NULL;
		return -ENOMEM;
	}

Which explicitly sets new_vma = NULL (new_vma_ptr = &new_vma)

The final return there is:

	*new_vma_ptr = new_vma;
	return err;

Which also sets it.

So I'm not sure what's wrong here? Am I missing something?

>
>     1921 		return err;
>     1922
>     1923 	/*
>     1924 	 * If we failed to move page tables we still do total_vm increment
>     1925 	 * since do_munmap() will decrement it by old_len == new_len.
>     1926 	 *
>     1927 	 * Since total_vm is about to be raised artificially high for a
>
> regards,
> dan carpenter
>