linux-kernel - Re: [patch 1/2] mm: dont clear PG_uptodate in invalidate_complete

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]

Date:	Mon, 07 Jul 2008 12:12:52 +0200
From:	Miklos Szeredi <miklos@...redi.hu>
To:	nickpiggin@...oo.com.au
CC:	miklos@...redi.hu, jamie@...reable.org,
	torvalds@...ux-foundation.org, jens.axboe@...cle.com,
	linux-fsdevel@...r.kernel.org, linux-kernel@...r.kernel.org,
	linux-mm@...ck.org, akpm@...ux-foundation.org, hugh@...itas.com
Subject: Re: [patch 1/2] mm: dont clear PG_uptodate in invalidate_complete_page2()

On Mon, 07 Jul 2008, Miklos Szeredi wrote:
> On Mon, 7 Jul 2008, Nick Piggin wrote:
> > I don't know what became of this thread, but I agree with everyone else
> > you should not skip clearing PG_uptodate here. If nothing else, it
> > weakens some important assertions in the VM. But I agree that splice
> > should really try harder to work with it and we should be a little
> > careful about just changing things like this.
> 
> Sure, that's why I rfc'ed.
> 
> But I'd still like to know, what *are* those assumptions in the VM
> that would be weakened by this?

For one, currently some of the generic VM code assumes that after
synchronously reading in a page (i.e. ->readpage() then lock_page())
!PageUptodate() necessarily means an I/O error:

/**
 * read_cache_page - read into page cache, fill it if needed
...
 * If the page does not get brought uptodate, return -EIO.
 */

Which is wrong, the page could be invalidated between being broough
uptodate and being examined for being uptodate.  Then we'd be
returning EIO, which is definitely wrong.

AFAICS this could be a real (albeit rare) bug in NFS's readdir().

This is easily fixable in read_cache_page(), but what I'm trying to
say is that assumptions about PG_uptodate aren't all that clear to
begin with, so it would perhaps be useful to first think about this a
bit more.

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