2007-11-14 18:31:26

by Robert Bragg

Subject: [PATCH] mm: Don't allow ioremapping of ranges larger than vmalloc space

When running with a 16M IOREMAP_MAX_ORDER (on armv7) we found that the vmlist
search routine in __get_vm_area_node can mistakenly allow a driver to ioremap
a range larger than vmalloc space.

If at the time of the ioremap all existing vmlist areas sit below the determined
alignment then the search routine continues past all entries and exits the for
loop - straight into the found: label - without ever testing for integer
wrapping or that the requested size fits.

We were seeing a driver successfully ioremap 128M of flash even though there was
only 120M of vmalloc space. From that point the system was left with
the remainder
of the first 16M of space to vmalloc/ioremap within.

Signed-off-by: Robert Bragg <[email protected]>

---

diff --git a/mm/vmalloc.c b/mm/vmalloc.c
index af77e17..06a7f3a 100644
--- a/mm/vmalloc.c
+++ b/mm/vmalloc.c
@@ -216,6 +216,10 @@ static struct vm_struct *__get_vm_area_node
if (addr > end - size)
goto out;
}
+ if ((size + addr) < addr)
+ goto out;
+ if (addr > end - size)
+ goto out;

found:
area->next = *p;

2007-11-14 21:41:11

by Nick Piggin

Subject: Re: [PATCH] mm: Don't allow ioremapping of ranges larger than vmalloc space

On Thursday 15 November 2007 05:31, Robert Bragg wrote:
> When running with a 16M IOREMAP_MAX_ORDER (on armv7) we found that the
> vmlist search routine in __get_vm_area_node can mistakenly allow a driver
> to ioremap a range larger than vmalloc space.
>
> If at the time of the ioremap all existing vmlist areas sit below the
> determined alignment then the search routine continues past all entries and
> exits the for loop - straight into the found: label - without ever testing
> for integer wrapping or that the requested size fits.
>
> We were seeing a driver successfully ioremap 128M of flash even though
> there was only 120M of vmalloc space. From that point the system was left
> with the remainder
> of the first 16M of space to vmalloc/ioremap within.
>
> Signed-off-by: Robert Bragg <[email protected]>
>
> ---
>
> diff --git a/mm/vmalloc.c b/mm/vmalloc.c
> index af77e17..06a7f3a 100644
> --- a/mm/vmalloc.c
> +++ b/mm/vmalloc.c
> @@ -216,6 +216,10 @@ static struct vm_struct *__get_vm_area_node
> if (addr > end - size)
> goto out;
> }
> + if ((size + addr) < addr)
> + goto out;
> + if (addr > end - size)
> + goto out;
>
> found:
> area->next = *p;

Yeah I noticed these missing checks when recently working in
this area as well. Weren't sure if I was missing something or
not.

Acked-by: Nick Piggin <[email protected]>