linux-kernel - Re: [PATCH] mm: fix page_mkclean_one (was: 2.6.19 file content corruption on ext3)

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:	Tue, 26 Dec 2006 21:31:01 +1100
From:	Nick Piggin <nickpiggin@...oo.com.au>
To:	Linus Torvalds <torvalds@...l.org>
CC:	Andrei Popa <andrei.popa@...eo.ro>,
	Peter Zijlstra <a.p.zijlstra@...llo.nl>,
	"David S. Miller" <davem@...emloft.net>,
	Andrew Morton <akpm@...l.org>,
	Gordon Farquharson <gordonfarquharson@...il.com>,
	Martin Michlmayr <tbm@...ius.com>,
	Hugh Dickins <hugh@...itas.com>,
	Arjan van de Ven <arjan@...radead.org>,
	Linux Kernel Mailing List <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH] mm: fix page_mkclean_one (was: 2.6.19 file content corruption
 on ext3)

Linus Torvalds wrote:
> 
> On Sun, 24 Dec 2006, Linus Torvalds wrote:
> 
>>Peter, tell me I'm crazy, but with the new rules, the following condition 
>>is a bug:
>>
>> - shared mapping
>> - writable
>> - not already marked dirty in the PTE
> 
> 
> Ok, so how about this diff.
> 
> I'm actually feeling good about this one. It really looks like 
> "do_no_page()" was simply buggy, and that this explains everything.

Still trying to catch up here, so I'm not going to reply to any old
stuff and just start at the tip of the thread... Other than to say
that I really like cancel_page_dirty ;)

I think your patch is quite right so that's a good catch. But I'm not
too surprised that it does not help the problem, because I don't
think we have started shedding any old pte_dirty tests at
unmap/reclaim-time, have we? So the dirty bit isn't going to get lost,
as such.

I was hoping that you've almost narrowed it down to the filesystem
writeback code, with the last few mails?

Nick

> Please please please test. Throw all the other patches away (with the 
> possible exception of the "update_mmu_cache()" sanity checker, which is 
> still interesting in case some _other_ place does this too).
> 
> Don't do the "wait_on_page_writeback()" thing, because it changes timings 
> and might hide thngs for the wrong reasons.  Just apply this on top of a 
> known failing kernel, and test.
> 
> 			Linus
> 
> ---
> diff --git a/mm/memory.c b/mm/memory.c
> index 563792f..cf429c4 100644
> --- a/mm/memory.c
> +++ b/mm/memory.c
> @@ -2247,21 +2249,23 @@ retry:
>  	if (pte_none(*page_table)) {
>  		flush_icache_page(vma, new_page);
>  		entry = mk_pte(new_page, vma->vm_page_prot);
> -		if (write_access)
> -			entry = maybe_mkwrite(pte_mkdirty(entry), vma);
> -		set_pte_at(mm, address, page_table, entry);
>  		if (anon) {
>  			inc_mm_counter(mm, anon_rss);
>  			lru_cache_add_active(new_page);
>  			page_add_new_anon_rmap(new_page, vma, address);
> +			if (write_access)
> +				entry = maybe_mkwrite(pte_mkdirty(entry), vma);
>  		} else {
>  			inc_mm_counter(mm, file_rss);
>  			page_add_file_rmap(new_page);
> +			entry = pte_wrprotect(entry);
>  			if (write_access) {
>  				dirty_page = new_page;
>  				get_page(dirty_page);
> +				entry = maybe_mkwrite(pte_mkdirty(entry), vma);
>  			}
>  		}
> +		set_pte_at(mm, address, page_table, entry);
>  	} else {
>  		/* One of our sibling threads was faster, back out. */
>  		page_cache_release(new_page);
> 


-- 
SUSE Labs, Novell Inc.

Send instant messages to your online friends http://au.messenger.yahoo.com 
-
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/