[PATCH 1/1] mm: pagemap: limit scan to virtual region being asked

[PATCH 1/1] mm: pagemap: limit scan to virtual region being asked

[Shiraz Hashim]

pagemap_read scans through the virtual address space of a
task till it prepares 'count' pagemaps or it reaches end
of task.

This presents a problem when the page walk doesn't happen
for vma with VM_PFNMAP set. In which case walk is silently
skipped and no pagemap is prepare, in turn making
pagemap_read to scan through task end, even crossing beyond
'count', landing into a different vma region. This leads to
wrong presentation of mappings for that vma.

Fix this by limiting end_vaddr to the end of the virtual
address region being scanned.

Signed-off-by: Shiraz Hashim <shashim@codeaurora.org>
---
 fs/proc/task_mmu.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/fs/proc/task_mmu.c b/fs/proc/task_mmu.c
index 246eae8..04362e4 100644
--- a/fs/proc/task_mmu.c
+++ b/fs/proc/task_mmu.c
@@ -1270,7 +1270,9 @@ static ssize_t pagemap_read(struct file *file, char __user *buf,
 	src = *ppos;
 	svpfn = src / PM_ENTRY_BYTES;
 	start_vaddr = svpfn << PAGE_SHIFT;
-	end_vaddr = TASK_SIZE_OF(task);
+	end_vaddr = start_vaddr + ((count / PM_ENTRY_BYTES) << PAGE_SHIFT);
+	if ((end_vaddr > TASK_SIZE_OF(task)) || (end_vaddr < start_vaddr))
+		end_vaddr = TASK_SIZE_OF(task);
 
 	/* watch out for wraparound */
 	if (svpfn > TASK_SIZE_OF(task) >> PAGE_SHIFT)
-- 

QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a
member of the Code Aurora Forum, hosted by The Linux Foundation

Re: [PATCH 1/1] mm: pagemap: limit scan to virtual region being asked

[Naoya Horiguchi]

On Tue, Jan 13, 2015 at 05:57:04PM +0530, Shiraz Hashim wrote:
> pagemap_read scans through the virtual address space of a
> task till it prepares 'count' pagemaps or it reaches end
> of task.
> 
> This presents a problem when the page walk doesn't happen
> for vma with VM_PFNMAP set. In which case walk is silently
> skipped and no pagemap is prepare, in turn making
> pagemap_read to scan through task end, even crossing beyond
> 'count', landing into a different vma region. This leads to
> wrong presentation of mappings for that vma.
> 
> Fix this by limiting end_vaddr to the end of the virtual
> address region being scanned.
> 
> Signed-off-by: Shiraz Hashim <shashim@codeaurora.org>

This patch works in some case, but there still seems a problem in another case.

Consider that we have two vmas within some narrow (PAGEMAP_WALK_SIZE) region.
One vma in lower address is VM_PFNMAP, and the other vma in higher address is not.
Then a single call of walk_page_range() skips the first vma and scans the
second vma, but the pagemap record of the second vma will be stored on the
wrong offset in the buffer, because we just skip vma(VM_PFNMAP) without calling
any callbacks (within which add_to_pagemap() increments pm.pos).

So calling pte_hole() for vma(VM_PFNMAP) looks a better fix to me.

Thanks,
Naoya Horiguchi

> ---
>  fs/proc/task_mmu.c | 4 +++-
>  1 file changed, 3 insertions(+), 1 deletion(-)
> 
> diff --git a/fs/proc/task_mmu.c b/fs/proc/task_mmu.c
> index 246eae8..04362e4 100644
> --- a/fs/proc/task_mmu.c
> +++ b/fs/proc/task_mmu.c
> @@ -1270,7 +1270,9 @@ static ssize_t pagemap_read(struct file *file, char __user *buf,
>  	src = *ppos;
>  	svpfn = src / PM_ENTRY_BYTES;
>  	start_vaddr = svpfn << PAGE_SHIFT;
> -	end_vaddr = TASK_SIZE_OF(task);
> +	end_vaddr = start_vaddr + ((count / PM_ENTRY_BYTES) << PAGE_SHIFT);
> +	if ((end_vaddr > TASK_SIZE_OF(task)) || (end_vaddr < start_vaddr))
> +		end_vaddr = TASK_SIZE_OF(task);
>  
>  	/* watch out for wraparound */
>  	if (svpfn > TASK_SIZE_OF(task) >> PAGE_SHIFT)
> -- 
> 
> QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a
> member of the Code Aurora Forum, hosted by The Linux Foundation
> 
> --
> To unsubscribe, send a message with 'unsubscribe linux-mm' in
> the body to majordomo@kvack.org.  For more info on Linux MM,
> see: http://www.linux-mm.org/ .
> Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

Re: [PATCH 1/1] mm: pagemap: limit scan to virtual region being asked

[Shiraz Hashim]

On Wed, Jan 14, 2015 at 01:08:40AM +0000, Naoya Horiguchi wrote:
> On Tue, Jan 13, 2015 at 05:57:04PM +0530, Shiraz Hashim wrote:
> > pagemap_read scans through the virtual address space of a
> > task till it prepares 'count' pagemaps or it reaches end
> > of task.
> > 
> > This presents a problem when the page walk doesn't happen
> > for vma with VM_PFNMAP set. In which case walk is silently
> > skipped and no pagemap is prepare, in turn making
> > pagemap_read to scan through task end, even crossing beyond
> > 'count', landing into a different vma region. This leads to
> > wrong presentation of mappings for that vma.
> > 
> > Fix this by limiting end_vaddr to the end of the virtual
> > address region being scanned.
> > 
> > Signed-off-by: Shiraz Hashim <shashim@codeaurora.org>
> 
> This patch works in some case, but there still seems a problem in
> another case.
> 
> Consider that we have two vmas within some narrow
> (PAGEMAP_WALK_SIZE) region.  One vma in lower address is VM_PFNMAP,
> and the other vma in higher address is not.  Then a single call of
> walk_page_range() skips the first vma and scans the second vma, but
> the pagemap record of the second vma will be stored on the wrong
> offset in the buffer, because we just skip vma(VM_PFNMAP) without
> calling any callbacks (within which add_to_pagemap() increments
> pm.pos).
> 
> So calling pte_hole() for vma(VM_PFNMAP) looks a better fix to me.
> 

Thanks. That makes sense, If you are okay, I can send following patch formally.

diff --git a/mm/pagewalk.c b/mm/pagewalk.c
index ad83195..b16ea60 100644
--- a/mm/pagewalk.c
+++ b/mm/pagewalk.c
@@ -200,6 +200,11 @@ int walk_page_range(unsigned long addr, unsigned long end,
                        if ((vma->vm_start <= addr) &&
                            (vma->vm_flags & VM_PFNMAP)) {
                                next = vma->vm_end;
+                               if (walk->pte_hole)
+                                       err = walk->pte_hole(addr, next, walk);
+                               if (err)
+                                       break;
+
                                pgd = pgd_offset(walk->mm, next);
                                continue;
                        }

regards
Shiraz

Re: [PATCH 1/1] mm: pagemap: limit scan to virtual region being asked

[Andrew Morton]

On Wed, 14 Jan 2015 01:08:40 +0000 Naoya Horiguchi <n-horiguchi@ah.jp.nec.com> wrote:

> On Tue, Jan 13, 2015 at 05:57:04PM +0530, Shiraz Hashim wrote:
> > pagemap_read scans through the virtual address space of a
> > task till it prepares 'count' pagemaps or it reaches end
> > of task.
> > 
> > This presents a problem when the page walk doesn't happen
> > for vma with VM_PFNMAP set. In which case walk is silently
> > skipped and no pagemap is prepare, in turn making
> > pagemap_read to scan through task end, even crossing beyond
> > 'count', landing into a different vma region. This leads to
> > wrong presentation of mappings for that vma.
> > 
> > Fix this by limiting end_vaddr to the end of the virtual
> > address region being scanned.
> > 
> > Signed-off-by: Shiraz Hashim <shashim@codeaurora.org>
> 
> This patch works in some case, but there still seems a problem in another case.
> 
> Consider that we have two vmas within some narrow (PAGEMAP_WALK_SIZE) region.
> One vma in lower address is VM_PFNMAP, and the other vma in higher address is not.
> Then a single call of walk_page_range() skips the first vma and scans the
> second vma, but the pagemap record of the second vma will be stored on the
> wrong offset in the buffer, because we just skip vma(VM_PFNMAP) without calling
> any callbacks (within which add_to_pagemap() increments pm.pos).
> 
> So calling pte_hole() for vma(VM_PFNMAP) looks a better fix to me.
> 

Can we get this finished off?  ASAP, please.

Re: [PATCH 1/1] mm: pagemap: limit scan to virtual region being asked

[Naoya Horiguchi]

On Wed, Feb 11, 2015 at 02:09:15PM -0800, Andrew Morton wrote:
> On Wed, 14 Jan 2015 01:08:40 +0000 Naoya Horiguchi <n-horiguchi@ah.jp.nec.com> wrote:
> 
> > On Tue, Jan 13, 2015 at 05:57:04PM +0530, Shiraz Hashim wrote:
> > > pagemap_read scans through the virtual address space of a
> > > task till it prepares 'count' pagemaps or it reaches end
> > > of task.
> > > 
> > > This presents a problem when the page walk doesn't happen
> > > for vma with VM_PFNMAP set. In which case walk is silently
> > > skipped and no pagemap is prepare, in turn making
> > > pagemap_read to scan through task end, even crossing beyond
> > > 'count', landing into a different vma region. This leads to
> > > wrong presentation of mappings for that vma.
> > > 
> > > Fix this by limiting end_vaddr to the end of the virtual
> > > address region being scanned.
> > > 
> > > Signed-off-by: Shiraz Hashim <shashim@codeaurora.org>
> > 
> > This patch works in some case, but there still seems a problem in another case.
> > 
> > Consider that we have two vmas within some narrow (PAGEMAP_WALK_SIZE) region.
> > One vma in lower address is VM_PFNMAP, and the other vma in higher address is not.
> > Then a single call of walk_page_range() skips the first vma and scans the
> > second vma, but the pagemap record of the second vma will be stored on the
> > wrong offset in the buffer, because we just skip vma(VM_PFNMAP) without calling
> > any callbacks (within which add_to_pagemap() increments pm.pos).
> > 
> > So calling pte_hole() for vma(VM_PFNMAP) looks a better fix to me.
> > 
> 
> Can we get this finished off?  ASAP, please.

Yes, I think so.
The patch "mm: pagewalk: call pte_hole() for VM_PFNMAP during walk_page_range"
(replacing this patch) is already in mainline.

Thanks,
Naoya Horiguchi