[PATCH] mm: Check we have the right vma in access_process_vm()

[PATCH] mm: Check we have the right vma in access_process_vm()

[Michael Ellerman]

In access_process_vm() we need to check that we have found the right
vma, not the following vma, before we try to access it. Otherwise
we might call the vma's access routine with an address which does
not fall inside the vma.

Signed-off-by: Michael Ellerman <michael@ellerman.id.au>
---
 mm/memory.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/mm/memory.c b/mm/memory.c
index 5823698..7e6f17b 100644
--- a/mm/memory.c
+++ b/mm/memory.c
@@ -3619,7 +3619,7 @@ int access_process_vm(struct task_struct *tsk, unsigned long addr, void *buf, in
 			 */
 #ifdef CONFIG_HAVE_IOREMAP_PROT
 			vma = find_vma(mm, addr);
-			if (!vma)
+			if (!vma || vma->vm_start > addr)
 				break;
 			if (vma->vm_ops && vma->vm_ops->access)
 				ret = vma->vm_ops->access(vma, addr, buf,
-- 
1.7.1

--
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/ .
Fight unfair telecom internet charges in Canada: sign http://stopthemeter.ca/
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

Re: [PATCH] mm: Check we have the right vma in access_process_vm()

[Michel Lespinasse]

On Mon, Apr 4, 2011 at 11:24 PM, Michael Ellerman
<michael@ellerman.id.au> wrote:
> In access_process_vm() we need to check that we have found the right
> vma, not the following vma, before we try to access it. Otherwise
> we might call the vma's access routine with an address which does
> not fall inside the vma.
>
> Signed-off-by: Michael Ellerman <michael@ellerman.id.au>

Please note that the code has moved into __access_remote_vm() in
current linus tree. Also, should len be truncated before calling
vma->vm_ops->access() so that we can guarantee it won't overflow past
the end of the vma ?

> diff --git a/mm/memory.c b/mm/memory.c
> index 5823698..7e6f17b 100644
> --- a/mm/memory.c
> +++ b/mm/memory.c
> @@ -3619,7 +3619,7 @@ int access_process_vm(struct task_struct *tsk, unsigned long addr, void *buf, in
>                         */
>  #ifdef CONFIG_HAVE_IOREMAP_PROT
>                        vma = find_vma(mm, addr);
> -                       if (!vma)
> +                       if (!vma || vma->vm_start > addr)
>                                break;
>                        if (vma->vm_ops && vma->vm_ops->access)
>                                ret = vma->vm_ops->access(vma, addr, buf,
> --
> 1.7.1

-- 
Michel "Walken" Lespinasse
A program is never fully debugged until the last user dies.

--
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/ .
Fight unfair telecom internet charges in Canada: sign http://stopthemeter.ca/
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

Re: [PATCH] mm: Check we have the right vma in access_process_vm()

[Michael Ellerman]

[-- Attachment #1: Type: text/plain, Size: 1009 bytes --]

On Mon, 2011-04-04 at 23:42 -0700, Michel Lespinasse wrote:
> On Mon, Apr 4, 2011 at 11:24 PM, Michael Ellerman
> <michael@ellerman.id.au> wrote:
> > In access_process_vm() we need to check that we have found the right
> > vma, not the following vma, before we try to access it. Otherwise
> > we might call the vma's access routine with an address which does
> > not fall inside the vma.
> >
> > Signed-off-by: Michael Ellerman <michael@ellerman.id.au>
> 
> Please note that the code has moved into __access_remote_vm() in
> current linus tree.

Ah good point, if git hadn't done such a good job of merging it I would
have noticed :)

I'll send a new version with a corrected changelog.

> Also, should len be truncated before calling vma->vm_ops->access() so
> that we can guarantee it won't overflow past the end of the vma ?

The access implementations I've looked at check len, but I guess it
could be truncated on the way in. But maybe that's being paranoid, I
dunno.

cheers


[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 198 bytes --]