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

Submitted by Michael Ellerman on April 5, 2011, 6:24 a.m.

Details

Message ID c4f5166f98cb703742191eb74f583bb8011f9cdf.1301984663.git.michael@ellerman.id.au
State Changes Requested
Headers show

Commit Message

Michael Ellerman April 5, 2011, 6:24 a.m.
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(-)

Comments

Michel Lespinasse April 5, 2011, 6:42 a.m.
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
Michael Ellerman April 8, 2011, 7:17 a.m.
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

Patch hide | download patch | download mbox

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,