The buffer->pages[] has "buffer->pagecount" elements so this > comparison has to be changed to >= to avoid reading beyond the end of the array. The buffer->pages[] array is allocated in cma_heap_allocate().
Fixes: a5d2d29e24be ("dma-buf: heaps: Move heap-helper logic into the cma_heap implementation") Signed-off-by: Dan Carpenter dan.carpenter@linaro.org --- drivers/dma-buf/heaps/cma_heap.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/dma-buf/heaps/cma_heap.c b/drivers/dma-buf/heaps/cma_heap.c index ee899f8e6721..bea7e574f916 100644 --- a/drivers/dma-buf/heaps/cma_heap.c +++ b/drivers/dma-buf/heaps/cma_heap.c @@ -165,7 +165,7 @@ static vm_fault_t cma_heap_vm_fault(struct vm_fault *vmf) struct vm_area_struct *vma = vmf->vma; struct cma_heap_buffer *buffer = vma->vm_private_data;
- if (vmf->pgoff > buffer->pagecount) + if (vmf->pgoff >= buffer->pagecount) return VM_FAULT_SIGBUS;
vmf->page = buffer->pages[vmf->pgoff];
On Mon, Oct 2, 2023 at 12:04 AM Dan Carpenter dan.carpenter@linaro.org wrote:
The buffer->pages[] has "buffer->pagecount" elements so this > comparison has to be changed to >= to avoid reading beyond the end of the array. The buffer->pages[] array is allocated in cma_heap_allocate().
Fixes: a5d2d29e24be ("dma-buf: heaps: Move heap-helper logic into the cma_heap implementation") Signed-off-by: Dan Carpenter dan.carpenter@linaro.org
drivers/dma-buf/heaps/cma_heap.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/dma-buf/heaps/cma_heap.c b/drivers/dma-buf/heaps/cma_heap.c index ee899f8e6721..bea7e574f916 100644 --- a/drivers/dma-buf/heaps/cma_heap.c +++ b/drivers/dma-buf/heaps/cma_heap.c @@ -165,7 +165,7 @@ static vm_fault_t cma_heap_vm_fault(struct vm_fault *vmf) struct vm_area_struct *vma = vmf->vma; struct cma_heap_buffer *buffer = vma->vm_private_data;
if (vmf->pgoff > buffer->pagecount)
if (vmf->pgoff >= buffer->pagecount) return VM_FAULT_SIGBUS;
Hi Dan,
Your fix looks correct to me, but I'm curious if you observed this problem on a device? The mmap in dma-buf.c looks like it prevents creating a mapping that is too large, and I think an access beyond the VMA should segfault before reaching here.
Thanks, T.J.
On Mon, Oct 02, 2023 at 10:16:24AM -0700, T.J. Mercier wrote:
On Mon, Oct 2, 2023 at 12:04 AM Dan Carpenter dan.carpenter@linaro.org wrote:
The buffer->pages[] has "buffer->pagecount" elements so this > comparison has to be changed to >= to avoid reading beyond the end of the array. The buffer->pages[] array is allocated in cma_heap_allocate().
Fixes: a5d2d29e24be ("dma-buf: heaps: Move heap-helper logic into the cma_heap implementation") Signed-off-by: Dan Carpenter dan.carpenter@linaro.org
drivers/dma-buf/heaps/cma_heap.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/dma-buf/heaps/cma_heap.c b/drivers/dma-buf/heaps/cma_heap.c index ee899f8e6721..bea7e574f916 100644 --- a/drivers/dma-buf/heaps/cma_heap.c +++ b/drivers/dma-buf/heaps/cma_heap.c @@ -165,7 +165,7 @@ static vm_fault_t cma_heap_vm_fault(struct vm_fault *vmf) struct vm_area_struct *vma = vmf->vma; struct cma_heap_buffer *buffer = vma->vm_private_data;
if (vmf->pgoff > buffer->pagecount)
if (vmf->pgoff >= buffer->pagecount) return VM_FAULT_SIGBUS;Hi Dan,
Your fix looks correct to me, but I'm curious if you observed this problem on a device? The mmap in dma-buf.c looks like it prevents creating a mapping that is too large, and I think an access beyond the VMA should segfault before reaching here.
This is from static analysis and not from testing. You could be correct that this bug can't affect real life.
regards, dan carpenter
On Tue, Oct 3, 2023 at 1:30 AM Dan Carpenter dan.carpenter@linaro.org wrote:
On Mon, Oct 02, 2023 at 10:16:24AM -0700, T.J. Mercier wrote:
On Mon, Oct 2, 2023 at 12:04 AM Dan Carpenter dan.carpenter@linaro.org wrote:
The buffer->pages[] has "buffer->pagecount" elements so this > comparison has to be changed to >= to avoid reading beyond the end of the array. The buffer->pages[] array is allocated in cma_heap_allocate().
Fixes: a5d2d29e24be ("dma-buf: heaps: Move heap-helper logic into the cma_heap implementation") Signed-off-by: Dan Carpenter dan.carpenter@linaro.org
drivers/dma-buf/heaps/cma_heap.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/dma-buf/heaps/cma_heap.c b/drivers/dma-buf/heaps/cma_heap.c index ee899f8e6721..bea7e574f916 100644 --- a/drivers/dma-buf/heaps/cma_heap.c +++ b/drivers/dma-buf/heaps/cma_heap.c @@ -165,7 +165,7 @@ static vm_fault_t cma_heap_vm_fault(struct vm_fault *vmf) struct vm_area_struct *vma = vmf->vma; struct cma_heap_buffer *buffer = vma->vm_private_data;
if (vmf->pgoff > buffer->pagecount)
if (vmf->pgoff >= buffer->pagecount) return VM_FAULT_SIGBUS;Hi Dan,
Your fix looks correct to me, but I'm curious if you observed this problem on a device? The mmap in dma-buf.c looks like it prevents creating a mapping that is too large, and I think an access beyond the VMA should segfault before reaching here.
This is from static analysis and not from testing. You could be correct that this bug can't affect real life.
regards, dan carpenter
Ok, thanks Dan.
Reviewed-by: T.J. Mercier tjmercier@google.com
linaro-mm-sig@lists.linaro.org
-
Dan Carpenter -
T.J. Mercier