linux-kernel - Re: [PATCH] mm/rmap: fix BUG at rmap

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: <20131219011440.GB25161@lge.com>
Date:	Thu, 19 Dec 2013 10:14:40 +0900
From:	Joonsoo Kim <iamjoonsoo.kim@....com>
To:	Andrew Morton <akpm@...ux-foundation.org>
Cc:	Wanpeng Li <liwanp@...ux.vnet.ibm.com>,
	Sasha Levin <sasha.levin@...cle.com>,
	Hugh Dickins <hughd@...gle.com>, linux-mm@...ck.org,
	linux-kernel@...r.kernel.org
Subject: Re: [PATCH] mm/rmap: fix BUG at rmap_walk

On Wed, Dec 18, 2013 at 05:04:29PM -0800, Andrew Morton wrote:
> On Thu, 19 Dec 2013 09:58:05 +0900 Joonsoo Kim <iamjoonsoo.kim@....com> wrote:
> 
> > On Wed, Dec 18, 2013 at 04:28:58PM -0800, Andrew Morton wrote:
> > > On Thu, 19 Dec 2013 08:16:35 +0800 Wanpeng Li <liwanp@...ux.vnet.ibm.com> wrote:
> > > 
> > > > page_get_anon_vma() called in page_referenced_anon() will lock and 
> > > > increase the refcount of anon_vma, page won't be locked for anonymous 
> > > > page. This patch fix it by skip check anonymous page locked.
> > > > 
> > > > [  588.698828] kernel BUG at mm/rmap.c:1663!
> > > 
> > > Why is all this suddenly happening.  Did we change something, or did a
> > > new test get added to trinity?
> > 
> > It is my fault.
> > I should remove this VM_BUG_ON() since rmap_walk() can be called
> > without holding PageLock() in this case.
> > 
> > I think that adding VM_BUG_ON() to each rmap_walk calllers is better
> > than this patch, because, now, rmap_walk() is called by many places and
> > each places has different contexts.
> 
> I don't think that putting the assertion into the caller makes a lot of
> sense, particularly if that code just did a lock_page()!  If a *callee*
> needs PageLocked() then that callee should assert that the page is
> locked.  So
> 
> 	VM_BUG_ON(!PageLocked(page));
> 
> means "this code requires that the page be locked".  And if that code
> requires PageLocked(), there must be reasons for this.  Let's also
> include an explanation of those reasons.

Yes, if this condition is invariant for rmap_walk(), we should put this on
rmap_walk(). But if not, we should put this on the other place. I will
investigate more and send good solution :)

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