Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S932386AbcDGTh5 (ORCPT ); Thu, 7 Apr 2016 15:37:57 -0400 Received: from mail-qg0-f54.google.com ([209.85.192.54]:35974 "EHLO mail-qg0-f54.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1755483AbcDGThz (ORCPT ); Thu, 7 Apr 2016 15:37:55 -0400 Subject: Re: [PATCH] ion: scatterlist offset not used for buffer map To: John Einar Reitan , gregkh@linuxfoundation.org References: <1460028599-22356-1-git-send-email-john.reitan@foss.arm.com> Cc: arve@android.com, devel@driverdev.osuosl.org, linux-kernel@vger.kernel.org From: Laura Abbott Message-ID: <5706B70E.4040004@redhat.com> Date: Thu, 7 Apr 2016 12:37:50 -0700 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:38.0) Gecko/20100101 Thunderbird/38.7.1 MIME-Version: 1.0 In-Reply-To: <1460028599-22356-1-git-send-email-john.reitan@foss.arm.com> Content-Type: text/plain; charset=windows-1252; format=flowed Content-Transfer-Encoding: 7bit Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 2139 Lines: 61 On 04/07/2016 04:29 AM, John Einar Reitan wrote: > ion's default user/kernel page mapping code don't honor the offset > option for scatterlists. It uses sg_page and expect the whole page to be > mapped, while the offset could dictate an offset within a large page. > > sg_phys correctly accounts for the offset, so should be used instead. > Can you be more specific about which heap and which allocation pattern is exposing this bug? > Signed-off-by: John Einar Reitan > --- > drivers/staging/android/ion/ion_heap.c | 11 +++++------ > 1 file changed, 5 insertions(+), 6 deletions(-) > > diff --git a/drivers/staging/android/ion/ion_heap.c b/drivers/staging/android/ion/ion_heap.c > index ca15a87..e83002dc 100644 > --- a/drivers/staging/android/ion/ion_heap.c > +++ b/drivers/staging/android/ion/ion_heap.c > @@ -47,7 +47,7 @@ void *ion_heap_map_kernel(struct ion_heap *heap, > > for_each_sg(table->sgl, sg, table->nents, i) { > int npages_this_entry = PAGE_ALIGN(sg->length) / PAGE_SIZE; > - struct page *page = sg_page(sg); > + struct page *page = pfn_to_page(PFN_DOWN(sg_phys(sg))); > > BUG_ON(i >= npages); > for (j = 0; j < npages_this_entry; j++) > @@ -79,7 +79,6 @@ int ion_heap_map_user(struct ion_heap *heap, struct ion_buffer *buffer, > int ret; > > for_each_sg(table->sgl, sg, table->nents, i) { > - struct page *page = sg_page(sg); > unsigned long remainder = vma->vm_end - addr; > unsigned long len = sg->length; > > @@ -87,18 +86,18 @@ int ion_heap_map_user(struct ion_heap *heap, struct ion_buffer *buffer, > offset -= sg->length; > continue; > } else if (offset) { > - page += offset / PAGE_SIZE; > len = sg->length - offset; > - offset = 0; > } > len = min(len, remainder); > - ret = remap_pfn_range(vma, addr, page_to_pfn(page), len, > - vma->vm_page_prot); > + ret = remap_pfn_range(vma, addr, PFN_DOWN(sg_phys(sg) + offset), > + len, vma->vm_page_prot); > if (ret) > return ret; > addr += len; > if (addr >= vma->vm_end) > return 0; > + > + offset = 0; > } > return 0; > } >