linux-kernel - Re: [PATCH] Revert "writeback: limit write_cache

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: <AANLkTil1R9I94pVXLE50JAMPIwpiaZuroJAg4wcwyBUb@mail.gmail.com>
Date:	Mon, 28 Jun 2010 18:12:35 -0700
From:	Linus Torvalds <torvalds@...ux-foundation.org>
To:	Dave Chinner <david@...morbit.com>,
	Linus Torvalds <torvalds@...ux-foundation.org>,
	Linux Kernel <linux-kernel@...r.kernel.org>,
	ocfs2-devel@....oracle.com, Tao Ma <tao.ma@...cle.com>,
	Dave Chinner <dchinner@...hat.com>,
	Christoph Hellwig <hch@....de>, Mark Fasheh <mfasheh@...e.com>
Subject: Re: [PATCH] Revert "writeback: limit write_cache_pages integrity 
	scanning to current EOF"

On Mon, Jun 28, 2010 at 5:54 PM, Joel Becker <Joel.Becker@...cle.com> wrote:
> On Tue, Jun 29, 2010 at 10:24:21AM +1000, Dave Chinner wrote:
>>
>> Looking at ocfs2_writepage(), it simply calls
>> block_write_full_page(), which does:
>>
>>       /* Is the page fully outside i_size? (truncate in progress) */
>>       offset = i_size & (PAGE_CACHE_SIZE-1);
>>       if (page->index >= end_index+1 || !offset) {
>>               /*
>>                * The page may have dirty, unmapped buffers.  For example,
>>                * they may have been added in ext3_writepage().  Make them
>>                * freeable here, so the page does not leak.
>>                */
>>               do_invalidatepage(page, 0);
>>               unlock_page(page);
>>               return 0; /* don't care */
>>       }
>>
>> i.e. pages beyond EOF get invalidated.  If it somehow gets through
>> that check, __block_write_full_page() will avoid writing dirty
>> bufferheads beyond EOF because the write is "racing with truncate".
>
>        Your contention is that we've never gotten those tail blocks to
> disk.  Instead, our code either handles the future extensions of i_size
> or we've just gotten lucky with our testing.  Our current BUG trigger is
> because we have a new check that catches this case.  Does that summarize
> your position correctly?

Maybe Dave has some more exhaustive answer, but his point that
block_write_full_page() already just drops the page does seem to be
very valid.  Which makes me suspect that it would be better to remove
the ocfs2 BUG_ON() as a stop-gap measure, rather than reverting the
commit. It seems to be true that the "don't bother flushing past EOF"
commit really just uncovered an older bug.

So maybe ocfs2 should just replace the bug-on with invalidating the
page (perhaps with a WARN_ONCE() to make sure the problem doesn't get
forgotten about?)

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