linux-kernel - Re: [PATCH] proc: put check_mem_permission before __get_free_page in mem

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]

Message-ID: <alpine.LSU.2.00.1104252048190.6978@sister.anvils>
Date:	Mon, 25 Apr 2011 20:56:14 -0700 (PDT)
From:	Hugh Dickins <hughd@...gle.com>
To:	bookjovi@...il.com
cc:	Andrew Morton <akpm@...ux-foundation.org>,
	Al Viro <viro@...iv.linux.org.uk>,
	David Rientjes <rientjes@...gle.com>,
	KOSAKI Motohiro <kosaki.motohiro@...fujitsu.com>,
	Stephen Wilson <wilsons@...rt.ca>,
	open list <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH] proc: put check_mem_permission before __get_free_page
 in mem_read

On Sun, 17 Apr 2011, bookjovi@...il.com wrote:
> From: Jovi Zhang <bookjovi@...il.com>
> 
> It should be better if put check_mem_permission before __get_free_page
> in mem_read, to be same as function mem_write.
> 
> Signed-off-by: Jovi Zhang <bookjovi@...il.com>

Sorry to be contrary, but I disagree with this.  I'm all for consistency,
but is there a particular reason why you think the mem_write ordering is
right and mem_read wrong?

My reason for preferring the current mem_read ordering is this:

check_mem_permission gets a reference to the mm.  If we __get_free_page
after check_mem_permission, imagine what happens if the system is out
of memory, and the mm we're looking at is selected for killing by the
OOM killer: while we wait in __get_free_page for more memory, no memory
is freed from the selected mm because it cannot reach exit_mmap while
we hold that reference.

(I may be overstating the case: a little memory may be freed from the
exiting task's stack, and kswapd should still be able to pick some pages
off the mm.  But nonetheless, we would do better to let this mm go.)

No doubt there are plenty of other places in /proc which try to
allocate memory after taking a reference on an mm; but I think
we should be working to eliminate those rather than add to them.

Hugh
--
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/