linux-kernel - Re: [BUG] problem with nfs_invalidate

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: <1188402562.6580.74.camel@heimdal.trondhjem.org>
Date:	Wed, 29 Aug 2007 11:49:22 -0400
From:	Trond Myklebust <trond.myklebust@....uio.no>
To:	Ryusuke Konishi <konishi.ryusuke@....ntt.co.jp>,
	Andrew Morton <akpm@...ux-foundation.org>
Cc:	nfs@...ts.sourceforge.net, linux-kernel@...r.kernel.org
Subject: Re: [BUG] problem with nfs_invalidate_page

On Tue, 2007-08-28 at 17:32 +0900, Ryusuke Konishi wrote:
> On Mon, 27 Aug 2007 09:30:12 -0400, Trond Myklebust wrote:
> >It looks as if ecryptfs is dropping the page lock between the calls to
> >prepare_write() and commit_write(). That would be a bug.
> 
> No, ecryptfs is holding the page lock between the calls to 
> nfs_prepare_write() and nfs_commit_write().
> This is a regression since kernel 2.6.20;  kernel 2.6.19 does not 
> yield the BUG.
> 
> Please look at truncate_complete_page() and nfs_wb_page_priority() 
> which is called from nfs_invalidate_page().
> 
> The recent truncate_complete_page() clears the dirty flag from a page
> before calling a_ops->invalidatepage(),
> ^^^^^^
> static void
> truncate_complete_page(struct address_space *mapping, struct page *page)
> {
>         ...
> 	cancel_dirty_page(page, PAGE_CACHE_SIZE);  <--- Inserted here at kernel 2.6.20
> 
> 	if (PagePrivate(page))
> 		do_invalidatepage(page, 0);   ---> will call a_ops->invalidatepage()
>         ...
> }
> 
> and this is disturbing nfs_wb_page_priority() from calling 
> nfs_writepage_locked() that is expected to handle the pending
> request (=nfs_page) associated with the page.
> 
> int nfs_wb_page_priority(struct inode *inode, struct page *page, int how)
> {
> 	...
> 	if (clear_page_dirty_for_io(page)) {
> 		ret = nfs_writepage_locked(page, &wbc);
> 		if (ret < 0)
> 			goto out;
> 	}
>  	...
> }
> 
> Since truncate_complete_page() will get rid of the page after
> a_ops->invalidatepage() returns, the request (=nfs_page) associated
> with the page becomes a garbage in nfs_inode->nfs_page_tree.
> 
> This causes the collision of nfs_page and yields the BUG.
> 
> 
> Cheers,
> Ryusuke Konishi

OK. I see your point... Basically, you are saying that the new
->invalidatepage() semantics do not allow us to rely on the dirty status
of the page in order to figure out if we need to clean up.

Andrew, that was a fairly significant change in semantics...

Anyhow, well done debugging it! Does the following patch fix the Oops?

Trond


Download attachment "linux-2.6.23-004-fix_nfs_wb_page_priority.dif" of type "message/rfc822" (4331 bytes)