linux-kernel - Re: [PATCH] 2.6.28, vmalloc.c, vmap_page

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:	Sat, 27 Dec 2008 04:36:10 +0100
From:	Johannes Weiner <hannes@...xchg.org>
To:	Andrew Morton <akpm@...ux-foundation.org>
Cc:	Adam Lackorzynski <adam@...inf.tu-dresden.de>,
	linux-kernel@...r.kernel.org, Nick Piggin <nickpiggin@...oo.com.au>
Subject: Re: [PATCH] 2.6.28, vmalloc.c, vmap_page_range

On Fri, Dec 26, 2008 at 04:39:46PM -0800, Andrew Morton wrote:
> On Thu, 25 Dec 2008 22:02:35 +0100 Adam Lackorzynski <adam@...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;

Ugh, start_addr is an awful name.  How about start?  I know it doesn't
hold the same amount of information but it's a local API, the
pgd_offset_k() should make the unit unambiguous, it goes better with
the end parameter and it's unique enough for this short function.

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