| 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
| ||
|
Message-ID: <45879BEF.9060001@yahoo.com.au>
Date: Tue, 19 Dec 2006 18:59:43 +1100
From: Nick Piggin <nickpiggin@...oo.com.au>
To: Peter Zijlstra <a.p.zijlstra@...llo.nl>
CC: Linus Torvalds <torvalds@...l.org>, Andrew Morton <akpm@...l.org>,
andrei.popa@...eo.ro,
Linux Kernel Mailing List <linux-kernel@...r.kernel.org>,
Hugh Dickins <hugh@...itas.com>,
Florian Weimer <fw@...eb.enyo.de>,
Marc Haber <mh+linux-kernel@...schlus.de>,
Martin Michlmayr <tbm@...ius.com>
Subject: Re: 2.6.19 file content corruption on ext3
Peter Zijlstra wrote:
> On Tue, 2006-12-19 at 15:36 +1100, Nick Piggin wrote:
>
>
>>plain text document attachment (fs-fix.patch)
>>Index: linux-2.6/fs/buffer.c
>>===================================================================
>>--- linux-2.6.orig/fs/buffer.c 2006-12-19 15:15:46.000000000 +1100
>>+++ linux-2.6/fs/buffer.c 2006-12-19 15:36:01.000000000 +1100
>>@@ -2852,7 +2852,17 @@ int try_to_free_buffers(struct page *pag
>> * This only applies in the rare case where try_to_free_buffers
>> * succeeds but the page is not freed.
>> */
>>- clear_page_dirty(page);
>>+
>>+ /*
>>+ * If the page has been dirtied via the user mappings, then
>>+ * clean buffers does not indicate the page data is actually
>>+ * clean! Only clear the page dirty bit if there are no dirty
>>+ * ptes either.
>>+ *
>>+ * If there are dirty ptes, then the page must be uptodate, so
>>+ * the above concern does not apply.
>>+ */
>>+ clear_page_dirty_sync_ptes(page);
>> }
>> out:
>> if (buffers_to_free) {
>>Index: linux-2.6/include/linux/page-flags.h
>>===================================================================
>>--- linux-2.6.orig/include/linux/page-flags.h 2006-12-19 15:17:18.000000000 +1100
>>+++ linux-2.6/include/linux/page-flags.h 2006-12-19 15:34:24.000000000 +1100
>>@@ -254,6 +254,7 @@ static inline void SetPageUptodate(struc
>> struct page; /* forward declaration */
>>
>> int test_clear_page_dirty(struct page *page);
>>+int test_clear_page_dirty_sync_ptes(struct page *page);
>> int test_clear_page_writeback(struct page *page);
>> int test_set_page_writeback(struct page *page);
>>
>>@@ -262,6 +263,11 @@ static inline void clear_page_dirty(stru
>> test_clear_page_dirty(page);
>> }
>>
>>+static inline void clear_page_dirty_sync_ptes(struct page *page)
>>+{
>>+ test_clear_page_dirty_sync_ptes(page);
>>+}
>>+
>> static inline void set_page_writeback(struct page *page)
>> {
>> test_set_page_writeback(page);
>>Index: linux-2.6/mm/page-writeback.c
>>===================================================================
>>--- linux-2.6.orig/mm/page-writeback.c 2006-12-19 15:17:53.000000000 +1100
>>+++ linux-2.6/mm/page-writeback.c 2006-12-19 15:33:29.000000000 +1100
>>@@ -844,9 +844,10 @@ EXPORT_SYMBOL(set_page_dirty_lock);
>>
>> /*
>> * Clear a page's dirty flag, while caring for dirty memory accounting.
>>+ * Does not clear pte dirty bits.
>> * Returns true if the page was previously dirty.
>> */
>>-int test_clear_page_dirty(struct page *page)
>>+static int test_clear_page_dirty_leave_ptes(struct page *page)
>> {
>> struct address_space *mapping = page_mapping(page);
>> unsigned long flags;
>>@@ -862,10 +863,8 @@ int test_clear_page_dirty(struct page *p
>> * We can continue to use `mapping' here because the
>> * page is locked, which pins the address_space
>> */
>>- if (mapping_cap_account_dirty(mapping)) {
>>- page_mkclean(page);
>>+ if (mapping_cap_account_dirty(mapping))
>> dec_zone_page_state(page, NR_FILE_DIRTY);
>>- }
>> return 1;
>> }
>> write_unlock_irqrestore(&mapping->tree_lock, flags);
>>@@ -873,9 +872,43 @@ int test_clear_page_dirty(struct page *p
>> }
>> return TestClearPageDirty(page);
>> }
>>+
>>+/*
>>+ * As above, but does clear dirty bits from ptes
>>+ */
>>+int test_clear_page_dirty(struct page *page)
>>+{
>>+ struct address_space *mapping = page_mapping(page);
>>+
>>+ if (test_clear_page_dirty_leave_ptes(page)) {
>>+ if (mapping_cap_account_dirty(mapping))
>>+ page_mkclean(page);
>>+ return 1;
>>+ }
>>+ return 0;
>>+}
>> EXPORT_SYMBOL(test_clear_page_dirty);
>>
>> /*
>>+ * As above, but redirties page if any dirty ptes are found (and then only
>>+ * if the mapping accounts dirty pages, otherwise dirty ptes are left dirty
>>+ * but the page is cleaned).
>>+ */
>>+int test_clear_page_dirty_sync_ptes(struct page *page)
>>+{
>>+ struct address_space *mapping = page_mapping(page);
>>+
>>+ if (test_clear_page_dirty_leave_ptes(page)) {
>>+ if (mapping_cap_account_dirty(mapping)) {
>>+ if (page_mkclean(page))
>>+ set_page_dirty(page);
>>+ }
>>+ return 1;
>>+ }
>>+ return 0;
>>+}
>>+
>>+/*
>> * Clear a page's dirty flag, while caring for dirty memory accounting.
>> * Returns true if the page was previously dirty.
>> *
>
>
> Hmm, not quite; It certainly look better than the extra ,[01] tagged to
> test_clear_page_dirty() though. Although I would have expected it the
> other way around - test_clear_pages_dirty_sync_ptes to be the default
> case and test_clear_pages_dirty_clean_ptes to be used in
> clear_page_dirty_for_io().
>
> Anyway it has the same issues as the others. See what happens when you
> run two test_clear_page_dirty_sync_ptes() consecutively, you still loose
> PG_dirty even though the page might actually be dirty.
How can this happen? We'll only test_clear_page_dirty_sync_ptes again
after buffers have been reattached, and subsequently cleaned. And in
that case if the ptes are still clean at this point then the page really
is clean.
--
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/
Powered by blists - more mailing lists