diff mbox

ARM: remove the .vm_mm value from gate_vma.

Message ID 1368702394-1737-1-git-send-email-steve.capper@linaro.org (mailing list archive)
State New, archived
Headers show

Commit Message

Steve Capper May 16, 2013, 11:06 a.m. UTC
If one reads /proc/$PID/smaps, the mmap_sem belonging to the
address space of the task being examined is locked for reading.
All the pages of the vmas belonging to the task's address space
are then walked with this lock held.

If a gate_vma is present in the architecture, it too is examined
by the fs/proc/task_mmu.c code. As gate_vma doesn't belong to the
address space of the task though, its pages are not walked.

A recent cleanup (commit f6604efe) of the gate_vma initialisation
code set the vm_mm value to &init_mm. Unfortunately a non-NULL
vm_mm value in the gate_vma will cause the task_mmu code to attempt
to walk the pages of the gate_vma (with no mmap-sem lock held). If
one enables Transparent Huge Page support and vm debugging, this
will then cause OOPses as pmd_trans_huge_lock is called without
mmap_sem being locked.

This patch removes the .vm_mm value from gate_vma, restoring the
original behaviour of the task_mmu code.

Signed-off-by: Steve Capper <steve.capper@linaro.org>
---
 arch/arm/kernel/process.c | 1 -
 1 file changed, 1 deletion(-)

Comments

Russell King - ARM Linux May 16, 2013, 2:36 p.m. UTC | #1
On Thu, May 16, 2013 at 12:06:34PM +0100, Steve Capper wrote:
> If one reads /proc/$PID/smaps, the mmap_sem belonging to the
> address space of the task being examined is locked for reading.
> All the pages of the vmas belonging to the task's address space
> are then walked with this lock held.
> 
> If a gate_vma is present in the architecture, it too is examined
> by the fs/proc/task_mmu.c code. As gate_vma doesn't belong to the
> address space of the task though, its pages are not walked.
> 
> A recent cleanup (commit f6604efe) of the gate_vma initialisation
> code set the vm_mm value to &init_mm. Unfortunately a non-NULL
> vm_mm value in the gate_vma will cause the task_mmu code to attempt
> to walk the pages of the gate_vma (with no mmap-sem lock held). If
> one enables Transparent Huge Page support and vm debugging, this
> will then cause OOPses as pmd_trans_huge_lock is called without
> mmap_sem being locked.
> 
> This patch removes the .vm_mm value from gate_vma, restoring the
> original behaviour of the task_mmu code.

This looks fine - x86 also sets .vm_mm to NULL here.  So please put
it in my patch system.  Does this need to be applied to stable
kernels as well?  If so, please mark it with Cc: <stable@vger.kernel.org>
(but do not send it to that address).  Also, it'd be worth checking
which stable kernels it needs to be applied to.

Thanks.
Steve Capper May 16, 2013, 4:22 p.m. UTC | #2
On Thu, May 16, 2013 at 03:36:38PM +0100, Russell King - ARM Linux wrote:
> On Thu, May 16, 2013 at 12:06:34PM +0100, Steve Capper wrote:

[ ... ]

> 
> This looks fine - x86 also sets .vm_mm to NULL here.  So please put
> it in my patch system.  Does this need to be applied to stable
> kernels as well?  If so, please mark it with Cc: <stable@vger.kernel.org>
> (but do not send it to that address).  Also, it'd be worth checking
> which stable kernels it needs to be applied to.
> 
> Thanks.

Thank you Russell,
I couldn't see any branches in linux-stable that need this patch.
The patch is now in your system with id 7727/1.

Cheers,
Will Deacon May 17, 2013, 8:45 a.m. UTC | #3
On Thu, May 16, 2013 at 12:06:34PM +0100, Steve Capper wrote:
> If one reads /proc/$PID/smaps, the mmap_sem belonging to the
> address space of the task being examined is locked for reading.
> All the pages of the vmas belonging to the task's address space
> are then walked with this lock held.
> 
> If a gate_vma is present in the architecture, it too is examined
> by the fs/proc/task_mmu.c code. As gate_vma doesn't belong to the
> address space of the task though, its pages are not walked.
> 
> A recent cleanup (commit f6604efe) of the gate_vma initialisation
> code set the vm_mm value to &init_mm. Unfortunately a non-NULL
> vm_mm value in the gate_vma will cause the task_mmu code to attempt
> to walk the pages of the gate_vma (with no mmap-sem lock held). If
> one enables Transparent Huge Page support and vm debugging, this
> will then cause OOPses as pmd_trans_huge_lock is called without
> mmap_sem being locked.
> 
> This patch removes the .vm_mm value from gate_vma, restoring the
> original behaviour of the task_mmu code.
> 
> Signed-off-by: Steve Capper <steve.capper@linaro.org>
> ---
>  arch/arm/kernel/process.c | 1 -
>  1 file changed, 1 deletion(-)
> 
> diff --git a/arch/arm/kernel/process.c b/arch/arm/kernel/process.c
> index f219703..282de48 100644
> --- a/arch/arm/kernel/process.c
> +++ b/arch/arm/kernel/process.c
> @@ -411,7 +411,6 @@ static struct vm_area_struct gate_vma = {
>  	.vm_start	= 0xffff0000,
>  	.vm_end		= 0xffff0000 + PAGE_SIZE,
>  	.vm_flags	= VM_READ | VM_EXEC | VM_MAYREAD | VM_MAYEXEC,
> -	.vm_mm		= &init_mm,
>  };
>  
>  static int __init gate_vma_init(void)

Thanks to the wonders of Mimecrap, which decided to sit on most of my email
yesterday, I only just received this patch.

I see it's in the patch system, but for the record:

  Acked-by: Will Deacon <will.deacon@arm.com>

Cheers,

Will
diff mbox

Patch

diff --git a/arch/arm/kernel/process.c b/arch/arm/kernel/process.c
index f219703..282de48 100644
--- a/arch/arm/kernel/process.c
+++ b/arch/arm/kernel/process.c
@@ -411,7 +411,6 @@  static struct vm_area_struct gate_vma = {
 	.vm_start	= 0xffff0000,
 	.vm_end		= 0xffff0000 + PAGE_SIZE,
 	.vm_flags	= VM_READ | VM_EXEC | VM_MAYREAD | VM_MAYEXEC,
-	.vm_mm		= &init_mm,
 };
 
 static int __init gate_vma_init(void)