linux-kernel - Re: [PATCH 3/3] mm: adjust rss counters for migration entiries

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.1201251453550.2141@eggly.anvils>
Date:	Wed, 25 Jan 2012 15:01:38 -0800 (PST)
From:	Hugh Dickins <hughd@...gle.com>
To:	Konstantin Khlebnikov <khlebnikov@...nvz.org>
cc:	Andrew Morton <akpm@...ux-foundation.org>,
	KAMEZAWA Hiroyuki <kamezawa.hiroyu@...fujitsu.com>,
	"linux-mm@...ck.org" <linux-mm@...ck.org>,
	"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH 3/3] mm: adjust rss counters for migration entiries

On Tue, 24 Jan 2012, Konstantin Khlebnikov wrote:
> Hugh Dickins wrote:
> > On Wed, 18 Jan 2012, Andrew Morton wrote:
> > > : From: Konstantin Khlebnikov<khlebnikov@...nvz.org>
> > > : Subject: mm: add rss counters consistency check
> > > :
> > > : Warn about non-zero rss counters at final mmdrop.
> > > :
> > > : This check will prevent reoccurences of bugs such as that fixed in "mm:
> > > : fix rss count leakage during migration".
> > > :
> > > : I didn't hide this check under CONFIG_VM_DEBUG because it rather small
> > > and
> > > : rss counters cover whole page-table management, so this is a good
> > > : invariant.
> > > :
> > > : Signed-off-by: Konstantin Khlebnikov<khlebnikov@...nvz.org>
> > > : Cc: Hugh Dickins<hughd@...gle.com>
> > > : Cc: KAMEZAWA Hiroyuki<kamezawa.hiroyu@...fujitsu.com>
> > 
> > I'd be happier with this one if you do hide the check under
> > CONFIG_VM_DEBUG - or even under CONFIG_DEBUG_VM if you want it to
> > be compiled in sometimes ;)  I suppose NR_MM_COUNTERS is only 3,
> > so it isn't a huge overhead; but I think you're overestimating the
> > importance of these counters, and it would look better under DEBUG_VM.
> 
> Theoretically, some drivers can touch page tables,
> for example if they do that outside of vma we can get some kind of strange
> memory leaks.

I don't understand you on that.  Sure, drivers could do all kinds of
damage, but if they're touching pagetables outside of the vmas, then
this check on rss at exit isn't going to catch them.

But the message I get is that you want to leave the check (which would
have been better at the end of exit_mmap, I think, but never mind)
outside of CONFIG_DEBUG_VM: okay, I don't feel strongly enough.

> > > : From: Konstantin Khlebnikov<khlebnikov@...nvz.org>
> > > : Subject: mm: postpone migrated page mapping reset
> > > :
> > > : Postpone resetting page->mapping until the final
> > > remove_migration_ptes().
> > > : Otherwise the expression PageAnon(migration_entry_to_page(entry)) does
> > > not
> > > : work.
> > > :
> > > : Signed-off-by: Konstantin Khlebnikov<khlebnikov@...nvz.org>
> > > : Cc: Hugh Dickins<hughd@...gle.com>
> > > : Cc: KAMEZAWA Hiroyuki<kamezawa.hiroyu@...fujitsu.com>
> > 
> > Isn't this one actually an essential part of the fix?  It should have
> > been part of the same patch, but you split them apart, now Andrew has
> > reordered them and pushed one part to 3.3, but this needs to go in too?
> > 
> 
> Oops. I missed that. Yes. race-fix does not work for anon-memory without that
> patch.
> But this is non-fatal, there are no new bugs.

Non-fatal and no new bug, yes, but it makes the fix which has already
gone in rather less of a fix than was intended (it'll get the total right,
but misreport anon as file).  Andrew, please add this one to your next
push to Linus - thanks.

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/