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

From: Andrew Morton
Date: Fri Dec 26 2008 - 19:40:00 EST


On Thu, 25 Dec 2008 22:02:35 +0100 Adam Lackorzynski <adam@xxxxxxxxxxxxxxxxxxxx> 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;
_

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/