linux-kernel - Re: [ 11/48] mm: correctly synchronize rss-counters at exit/exec

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.1207011150480.1905@eggly.anvils>
Date:	Sun, 1 Jul 2012 12:02:11 -0700 (PDT)
From:	Hugh Dickins <hughd@...gle.com>
To:	Ben Hutchings <ben@...adent.org.uk>
cc:	linux-kernel@...r.kernel.org, stable@...r.kernel.org,
	torvalds@...ux-foundation.org, akpm@...ux-foundation.org,
	alan@...rguk.ukuu.org.uk,
	Konstantin Khlebnikov <khlebnikov@...nvz.org>,
	Markus Trippelsdorf <markus@...ppelsdorf.de>,
	Oleg Nesterov <oleg@...hat.com>,
	KAMEZAWA Hiroyuki <kamezawa.hiroyu@...fujitsu.com>
Subject: Re: [ 11/48] mm: correctly synchronize rss-counters at exit/exec

On Sun, 1 Jul 2012, Ben Hutchings wrote:

> 3.2-stable review patch.  If anyone has any objections, please let me know.
> 
> ------------------
> 
> From: Konstantin Khlebnikov <khlebnikov@...nvz.org>
> 
> commit 4fe7efdbdfb1c7e7a7f31decfd831c0f31d37091 upstream.
> 
> do_exit() and exec_mmap() call sync_mm_rss() before mm_release() does
> put_user(clear_child_tid) which can update task->rss_stat and thus make
> mm->rss_stat inconsistent.  This triggers the "BUG:" printk in check_mm().
> 
> Let's fix this bug in the safest way, and optimize/cleanup this later.
> 
> Reported-by: Markus Trippelsdorf <markus@...ppelsdorf.de>
> Signed-off-by: Konstantin Khlebnikov <khlebnikov@...nvz.org>
> Cc: Oleg Nesterov <oleg@...hat.com>
> Cc: KAMEZAWA Hiroyuki <kamezawa.hiroyu@...fujitsu.com>
> Cc: Hugh Dickins <hughd@...gle.com>
> Signed-off-by: Andrew Morton <akpm@...ux-foundation.org>
> Signed-off-by: Linus Torvalds <torvalds@...ux-foundation.org>
> [bwh: Backported to 3.2: sync_mm_rss() still takes a struct task_struct *]
> Signed-off-by: Ben Hutchings <ben@...adent.org.uk>

If you or Konstantin or Oleg have done full diligence on this,
to ensure that it is really applicable to 3.2 (not just that
the patch applies without rejects), fair enough.

But I'd be cautious about it: it went through enough alternatives
and revisions that I wouldn't call it trivial; it's easy for me to
imagine that some of the affected paths were actually slightly different
in 3.2 days than they were in 3.4 days; and the disturbing warning that
these mods silence ("BUG: Bad rss-counter state ") did not exist before
3.4 - unless you've ported that too?

That's not to assert that we had no rss problem at all before 3.4,
but we've not heard of any trouble from it.  Caution tells me that
this patch might cause more trouble than it's worth.

Hugh

> ---
> --- a/fs/exec.c
> +++ b/fs/exec.c
> @@ -820,10 +820,10 @@
>  	/* Notify parent that we're no longer interested in the old VM */
>  	tsk = current;
>  	old_mm = current->mm;
> -	sync_mm_rss(tsk, old_mm);
>  	mm_release(tsk, old_mm);
>  
>  	if (old_mm) {
> +		sync_mm_rss(tsk, old_mm);
>  		/*
>  		 * Make sure that if there is a core dump in progress
>  		 * for the old mm, we get out and die instead of going
> --- a/kernel/exit.c
> +++ b/kernel/exit.c
> @@ -641,6 +641,7 @@
>  	mm_release(tsk, mm);
>  	if (!mm)
>  		return;
> +	sync_mm_rss(tsk, mm);
>  	/*
>  	 * Serialize with any possible pending coredump.
>  	 * We must hold mmap_sem around checking core_state
> 
> 
> 
--
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/