* [bug report] mm/gup: remove vmas parameter from get_user_pages_remote()
@ 2023-05-17 11:45 Dan Carpenter
2023-05-17 11:54 ` Lorenzo Stoakes
0 siblings, 1 reply; 3+ messages in thread
From: Dan Carpenter @ 2023-05-17 11:45 UTC (permalink / raw)
To: lstoakes; +Cc: linux-mm
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.
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
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [bug report] mm/gup: remove vmas parameter from get_user_pages_remote()
2023-05-17 11:45 [bug report] mm/gup: remove vmas parameter from get_user_pages_remote() Dan Carpenter
@ 2023-05-17 11:54 ` Lorenzo Stoakes
2023-05-17 19:29 ` Lorenzo Stoakes
0 siblings, 1 reply; 3+ messages in thread
From: Lorenzo Stoakes @ 2023-05-17 11:54 UTC (permalink / raw)
To: Dan Carpenter; +Cc: linux-mm
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
>
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [bug report] mm/gup: remove vmas parameter from get_user_pages_remote()
2023-05-17 11:54 ` Lorenzo Stoakes
@ 2023-05-17 19:29 ` Lorenzo Stoakes
0 siblings, 0 replies; 3+ messages in thread
From: Lorenzo Stoakes @ 2023-05-17 19:29 UTC (permalink / raw)
To: Dan Carpenter; +Cc: linux-mm
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@gmail.com/
>
> > 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
> >
>
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2023-05-17 19:29 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2023-05-17 11:45 [bug report] mm/gup: remove vmas parameter from get_user_pages_remote() Dan Carpenter
2023-05-17 11:54 ` Lorenzo Stoakes
2023-05-17 19:29 ` Lorenzo Stoakes
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox