Skip to contentSkip to conversation

mm: pagemap: limit scan to virtual region being asked

PATCH In review Shiraz Hashim 3 participants 5 messages Jan 13, 2015 – Feb 11, 2015

Subsystems (4)
lore
Share & tools
lore
#0 lore raw

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

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.

---
 fs/proc/task_mmu.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)
fs/proc/task_mmu.c +3 -1
index 246eae8..04362e4 100644
@@ -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)
Signature · 8 lines
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 [email protected].  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"[email protected]"> [email protected] </a>
#1 lore raw
Quoted 11 lines · 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.

Quoted 4 lines

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

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

Quoted 29 lines
---
 fs/proc/task_mmu.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)
fs/proc/task_mmu.c +3 -1
index 246eae8..04362e4 100644
@@ -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)
Signature · 8 lines
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 [email protected].  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"[email protected]"> [email protected] </a>

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in the body to [email protected]. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"[email protected]"> [email protected] </a>

#2 lore raw
Quoted 29 lines · Naoya Horiguchi
Quoted 11 lines · 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.

Quoted 4 lines

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

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).

Quoted 2 lines

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.

mm/pagewalk.c +5 -0
index ad83195..b16ea60 100644
@@ -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

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in the body to [email protected]. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"[email protected]"> [email protected] </a>

maintainer
#3 lore raw
In reply to Naoya Horiguchi
Quoted 19 lines · On Wed, 14 Jan 2015 01:08:40 +0000 Naoya Horiguchi <[email protected]>…
Quoted 11 lines · 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.

Quoted 4 lines

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

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

Quoted 9 lines

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.

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in the body to [email protected]. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"[email protected]"> [email protected] </a>

#4 lore raw
Quoted 28 lines · Andrew Morton
Quoted 26 lines · On Wed, 14 Jan 2015 01:08:40 +0000 Naoya Horiguchi <[email protected]>…
Quoted 11 lines · 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.

Quoted 4 lines

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

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).

Quoted 4 lines
Quoted 2 lines

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
--
To unsubscribe, send a message with 'unsubscribe linux-mm' in the body to [email protected]. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"[email protected]"> [email protected] </a>