Re: [PATCH] 2.6.28, vmalloc.c, vmap_page_range

[Andrew Morton <akpm@linux-foundation.org>]

On Thu, 25 Dec 2008 22:02:35 +0100 Adam Lackorzynski <adam@os.inf.tu-dresden.de> wrote:

> Hi,
> 
> in 2.6.28, the flush_cache_vmap in vmap_page_range() is called with the end of
> the range twice. The following patch fixes this for me.
> 

Did this bug have any observeable runtime effects?  If so, what were
they?

> ---
>  vmalloc.c |    4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> --- linux-2.6.28/mm/vmalloc.c	2008-12-25 00:26:37.000000000 +0100
> +++ linux-2.6.28.a/mm/vmalloc.c	2008-12-25 21:45:43.118725744 +0100
> @@ -155,7 +155,7 @@
>  				pgprot_t prot, struct page **pages)
>  {
>  	pgd_t *pgd;
> -	unsigned long next;
> +	unsigned long next, start = addr;
>  	int err = 0;
>  	int nr = 0;
>  
> @@ -167,7 +167,7 @@
>  		if (err)
>  			break;
>  	} while (pgd++, addr = next, addr != end);
> -	flush_cache_vmap(addr, end);
> +	flush_cache_vmap(start, end);
>  
>  	if (unlikely(err))
>  		return err;

Well yeah.  This is what happens when functions modify their incoming
arguments.  It's a bad programming practice which leads directly to
exactly this sort of bug.

How about we fix that?


--- a/mm/vmalloc.c~vmallocc-fix-flushing-in-vmap_page_range
+++ a/mm/vmalloc.c
@@ -151,11 +151,12 @@ static int vmap_pud_range(pgd_t *pgd, un
  *
  * Ie. pte at addr+N*PAGE_SIZE shall point to pfn corresponding to pages[N]
  */
-static int vmap_page_range(unsigned long addr, unsigned long end,
+static int vmap_page_range(unsigned long start_addr, unsigned long end,
 				pgprot_t prot, struct page **pages)
 {
 	pgd_t *pgd;
 	unsigned long next;
+	unsigned long addr = start_addr;
 	int err = 0;
 	int nr = 0;
 
@@ -167,7 +168,7 @@ static int vmap_page_range(unsigned long
 		if (err)
 			break;
 	} while (pgd++, addr = next, addr != end);
-	flush_cache_vmap(addr, end);
+	flush_cache_vmap(start_addr, end);
 
 	if (unlikely(err))
 		return err;
_