linux-kernel - Re: [PATCH] remove unused code from do_wp

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]

Date:	Tue, 11 Dec 2012 13:36:45 -0800
From:	Andrew Morton <akpm@...ux-foundation.org>
To:	dingel@...ux.vnet.ibm.com
Cc:	David Rientjes <rientjes@...gle.com>,
	Al Viro <viro@...iv.linux.org.uk>,
	Konstantin Khlebnikov <khlebnikov@...nvz.org>,
	Andrea Arcangeli <aarcange@...hat.com>, linux-mm@...ck.org,
	linux-kernel@...r.kernel.org
Subject: Re: [PATCH] remove unused code from do_wp_page

On Tue, 11 Dec 2012 15:44:50 +0100
dingel@...ux.vnet.ibm.com wrote:

> From: Dominik Dingel <dingel@...ux.vnet.ibm.com>
> 
> page_mkwrite is initalized with zero and only set once, from that point exists no way to get to the oom or oom_free_new labels.
> 
> Signed-off-by: Dominik Dingel <dingel@...ux.vnet.ibm.com>
> ---
>  mm/memory.c | 4 ----
>  1 file changed, 4 deletions(-)
> 
> diff --git a/mm/memory.c b/mm/memory.c
> index 221fc9f..c322708 100644
> --- a/mm/memory.c
> +++ b/mm/memory.c
> @@ -2795,10 +2795,6 @@ oom_free_new:
>  	page_cache_release(new_page);
>  oom:
>  	if (old_page) {
> -		if (page_mkwrite) {
> -			unlock_page(old_page);
> -			page_cache_release(old_page);
> -		}
>  		page_cache_release(old_page);
>  	}
>  	return VM_FAULT_OOM;

I hope you've checked all this carefully, including the "goto reuse"
and "goto gotten" paths.  I *think* it's OK, but geeze. And the oom path
surely gets very little testing.

do_wp_page() has become truly awful.  I'm wondering if we should
actually leave that code in there in case something changes in the
future and it becomes necessary.

With my compiler version this patch actually increases the size of
memory.o's text by 7 bytes.  Odd.

Ho hum.  You should also have done this:

--- a/mm/memory.c~mm-memoryc-remove-unused-code-from-do_wp_page-fix
+++ a/mm/memory.c
@@ -2780,9 +2780,8 @@ unlock:
 oom_free_new:
 	page_cache_release(new_page);
 oom:
-	if (old_page) {
+	if (old_page)
 		page_cache_release(old_page);
-	}
 	return VM_FAULT_OOM;
 
 unwritable_page:
_

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