On Wed, May 17, 2023 at 12:54:00PM +0100, Lorenzo Stoakes wrote:
> 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.
Fixed in [0].
[0]:https://lore.kernel.org/all/d20128c849ecdbf4dd01cc828fcec32127ed939a.1684350871.git.lstoakes@xxxxxxxxx/
>
> > 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
> >
>
