linux-kernel - RE: Hugepage regression

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] [day] [month] [year] [list]

Message-ID: <Pine.LNX.4.64.0610131744580.14935@blonde.wat.veritas.com>
Date:	Fri, 13 Oct 2006 18:03:08 +0100 (BST)
From:	Hugh Dickins <hugh@...itas.com>
To:	"Chen, Kenneth W" <kenneth.w.chen@...el.com>
cc:	"'David Gibson'" <david@...son.dropbear.id.au>,
	Andrew Morton <akpm@...l.org>, linux-kernel@...r.kernel.org
Subject: RE: Hugepage regression

On Tue, 10 Oct 2006, Chen, Kenneth W wrote:
> Hugh Dickins wrote on Tuesday, October 10, 2006 1:10 PM
> > > can we reverse that order (call unmap_region
> > > and then nulls out vma->vmfile and fput)?
> > 
> > I'm pretty sure we cannot: the ordering is quite intentional, that if
> > a driver ->mmap failed, then it'd be wrong to call down to driver in
> > the unmap_region (if a driver is nicely behaved, that unmap_region
> > shouldn't be unnecessary; but some do rely on us clearing ptes there).

Looking at it again, my explanation seems wrong: I can't see any danger
of calling down to the _driver_ there (there's no remove_vma): rather,
it's __remove_shared_vm_struct we're avoiding by setting vm_file NULL.

> 
> Even not something like the following?  I believe you that nullifying
> vma->vm_file can not be done after unmap_region(),

Yet in your patch below, you do nullify vm_file _after_ unmap_region.

> I just want to make sure we are talking the same thing.

So I'm not sure if we are!

> It looks OK to me to defer the fput in the do_mmap_pgoff clean up path.

Yes, it would be quite okay to defer the fput until after the
unmap_region; but there's no point in making that change, is there?

Hugh (sorry, I was off sick for a couple of days)

> 
> 
> --- ./mm/mmap.c.orig	2006-10-10 15:58:17.000000000 -0700
> +++ ./mm/mmap.c	2006-10-10 15:59:02.000000000 -0700
> @@ -1159,11 +1159,12 @@ out:	
>  unmap_and_free_vma:
>  	if (correct_wcount)
>  		atomic_inc(&inode->i_writecount);
> -	vma->vm_file = NULL;
> -	fput(file);
>  
>  	/* Undo any partial mapping done by a device driver. */
>  	unmap_region(mm, vma, prev, vma->vm_start, vma->vm_end);
> +
> +	vma->vm_file = NULL;
> +	fput(file);
>  	charged = 0;
>  free_vma:
>  	kmem_cache_free(vm_area_cachep, vma);
> 
-
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/