On Wed, May 17, 2023 at 02:45:22PM +0300, Dan Carpenter wrote:
> Hello Lorenzo Stoakes,
>
> The patch eca1a00155df: "mm/gup: remove vmas parameter from
> get_user_pages_remote()" from May 14, 2023, leads to the following
> Smatch static checker warning:
>
> mm/memory.c:5617 __access_remote_vm()
> error: uninitialized symbol 'vma'.
>
> mm/memory.c
> 5590 int __access_remote_vm(struct mm_struct *mm, unsigned long addr, void *buf,
> 5591 int len, unsigned int gup_flags)
> 5592 {
> 5593 void *old_buf = buf;
> 5594 int write = gup_flags & FOLL_WRITE;
> 5595
> 5596 if (mmap_read_lock_killable(mm))
> 5597 return 0;
> 5598
> 5599 /* ignore errors, just check how much was successfully transferred */
> 5600 while (len) {
> 5601 int bytes, offset;
> 5602 void *maddr;
> 5603 struct vm_area_struct *vma;
> 5604 struct page *page = get_user_page_vma_remote(mm, addr,
> 5605 gup_flags, &vma);
> 5606
> 5607 if (IS_ERR_OR_NULL(page)) {
>
> If page is either an error pointer or NULL then
>
> 5608 int ret = 0;
> 5609
> 5610 #ifndef CONFIG_HAVE_IOREMAP_PROT
> 5611 break;
> 5612 #else
> 5613 /*
> 5614 * Check if this is a VM_IO | VM_PFNMAP VMA, which
> 5615 * we can access using slightly different code.
> 5616 */
> --> 5617 if (!vma)
>
> that means vma is unitialized.
>
Ack yeah you're right, this is a product of carrying over the code with a
wrapper that behaves slightly differently.
I'll fix this + roll in the -fix patch stuff in a new respin tonight.
> 5618 break;
> 5619 if (vma->vm_ops && vma->vm_ops->access)
> 5620 ret = vma->vm_ops->access(vma, addr, buf,
> 5621 len, write);
> 5622 if (ret <= 0)
> 5623 break;
> 5624 bytes = ret;
> 5625 #endif
> 5626 } else {
> 5627 bytes = len;
> 5628 offset = addr & (PAGE_SIZE-1);
> 5629 if (bytes > PAGE_SIZE-offset)
> 5630 bytes = PAGE_SIZE-offset;
> 5631
> 5632 maddr = kmap(page);
> 5633 if (write) {
> 5634 copy_to_user_page(vma, page, addr,
> 5635 maddr + offset, buf, bytes);
> 5636 set_page_dirty_lock(page);
> 5637 } else {
> 5638 copy_from_user_page(vma, page, addr,
> 5639 buf, maddr + offset, bytes);
> 5640 }
> 5641 kunmap(page);
> 5642 put_page(page);
> 5643 }
> 5644 len -= bytes;
> 5645 buf += bytes;
> 5646 addr += bytes;
> 5647 }
> 5648 mmap_read_unlock(mm);
> 5649
> 5650 return buf - old_buf;
> 5651 }
>
> regards,
> dan carpenter
>
