Hello John Stultz,
The patch 7b87ea704fd9: "dma-buf: heaps: Add heap helpers" from Oct 21, 2019, leads to the following static checker warning:
drivers/dma-buf/heaps/heap-helpers.c:165 dma_heap_vm_fault() warn: uncapped user index 'buffer->pages[vmf->pgoff]'
drivers/dma-buf/heaps/heap-helpers.c 160 static vm_fault_t dma_heap_vm_fault(struct vm_fault *vmf) 161 { 162 struct vm_area_struct *vma = vmf->vma; 163 struct heap_helper_buffer *buffer = vma->vm_private_data; 164 165 vmf->page = buffer->pages[vmf->pgoff]; ^^^^^^^^^^ Smatch for some reason thinks this needs to be checked. Smatch also gets confused by these fault handlers and thinks there is some recursion involved...
166 get_page(vmf->page); 167 168 return 0; 169 } 170 171 static const struct vm_operations_struct dma_heap_vm_ops = { 172 .fault = dma_heap_vm_fault, 173 }; 174
regards, dan carpenter
On Thu, Oct 31, 2019 at 4:07 AM Dan Carpenter dan.carpenter@oracle.com wrote:
The patch 7b87ea704fd9: "dma-buf: heaps: Add heap helpers" from Oct 21, 2019, leads to the following static checker warning:
drivers/dma-buf/heaps/heap-helpers.c:165 dma_heap_vm_fault() warn: uncapped user index 'buffer->pages[vmf->pgoff]'drivers/dma-buf/heaps/heap-helpers.c 160 static vm_fault_t dma_heap_vm_fault(struct vm_fault *vmf) 161 { 162 struct vm_area_struct *vma = vmf->vma; 163 struct heap_helper_buffer *buffer = vma->vm_private_data; 164 165 vmf->page = buffer->pages[vmf->pgoff]; ^^^^^^^^^^ Smatch for some reason thinks this needs to be checked. Smatch also gets confused by these fault handlers and thinks there is some recursion involved...
Hrm, I'm not totally sure I grok what it's looking for, but I'm guessing it's suggesting we check that pgoff isn't larger then the pagecount? I'll add that check in, but let me know if I've misunderstood.
Thanks for the report! -john
On Fri, Nov 01, 2019 at 12:31:53PM -0700, John Stultz wrote:
On Thu, Oct 31, 2019 at 4:07 AM Dan Carpenter dan.carpenter@oracle.com wrote:
The patch 7b87ea704fd9: "dma-buf: heaps: Add heap helpers" from Oct 21, 2019, leads to the following static checker warning:
drivers/dma-buf/heaps/heap-helpers.c:165 dma_heap_vm_fault() warn: uncapped user index 'buffer->pages[vmf->pgoff]'drivers/dma-buf/heaps/heap-helpers.c 160 static vm_fault_t dma_heap_vm_fault(struct vm_fault *vmf) 161 { 162 struct vm_area_struct *vma = vmf->vma; 163 struct heap_helper_buffer *buffer = vma->vm_private_data; 164 165 vmf->page = buffer->pages[vmf->pgoff]; ^^^^^^^^^^ Smatch for some reason thinks this needs to be checked. Smatch also gets confused by these fault handlers and thinks there is some recursion involved...
Hrm, I'm not totally sure I grok what it's looking for, but I'm guessing it's suggesting we check that pgoff isn't larger then the pagecount? I'll add that check in, but let me know if I've misunderstood.
Yeah, that's right. Or maybe pgoff is checked somewhere else?
regards, dan carpenter
linaro-mm-sig@lists.linaro.org
-
Dan Carpenter -
John Stultz