On Thu, Dec 19, 2013 at 12:43:03AM -0500, Theodore Ts'o wrote: > On Wed, Dec 18, 2013 at 09:12:41PM -0700, Matthew Wilcox wrote: > > Now that I've spent the best part of a day looking at the ext4 code, I > > still don't think there's a problem here. With the way the XIP code is > > currently written (calling ext4_get_block with create=1), we won't get an > > uninitialised extent in the caller. Instead, we'll get one that's been > > zeroed (the zeroing is part of patch 3/3 and done only for xip files). > > If the block was originally allocated via fallocate(2), it will be > marked as uninitialized. When you call ext4_get_block(), if the block > has been allocated, it will be returned --- and ext4_map_block() as > called by ext4_get_block() does ****not*** clear the uninitialized > flag. It can't do so because it would be racy; you can only clear the > flag once the data blocks has been written. > > As far as patch 3/3, it clears the pages in the page cache, but it > doesn't zap them in the XIP storage device. But it only does this on > the code path when it allocated a block. But if the block has already > been previously mapped via fallocate(2), you never hit this section of > code. Umm. That sounds like the real bug then. Any page returned from get_xip_mem must be initialised, because we may be about to map it into userspace. We could have ext4_get_xip_mem() check buffer_unwritten(); if it's set, zero the blocks and call ext4_convert_unwritten_extents(). Would that work? -- Matthew Wilcox Intel Open Source Technology Centre "Bill, look, we understand that you're interested in selling us this operating system, but compare it to ours. We can't possibly take such a retrograde step." -- To unsubscribe from this list: send the line "unsubscribe linux-fsdevel" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
