From: Jaegeuk Kim <jaegeuk@kernel.org> To: Eric Biggers <ebiggers@kernel.org> Cc: linux-kernel@vger.kernel.org, linux-f2fs-devel@lists.sourceforge.net Subject: Re: [PATCH 3/6] f2fs: do not expose unwritten blocks to user by DIO Date: Fri, 7 Jan 2022 17:52:48 -0800 [thread overview] Message-ID: <YdjucBBopEDuUb5b@google.com> (raw) In-Reply-To: <YdjJAS7Ua4aJEFhz@sol.localdomain> On 01/07, Eric Biggers wrote: > Hi Jaegeuk, > > On Tue, Jan 04, 2022 at 01:24:16PM -0800, Jaegeuk Kim wrote: > > DIO preallocates physical blocks before writing data, but if an error occurrs > > or power-cut happens, we can see block contents from the disk. This patch tries > > to fix it by 1) turning to buffered writes for DIO into holes, 2) truncating > > unwritten blocks from error or power-cut. > > > > Signed-off-by: Jaegeuk Kim <jaegeuk@kernel.org> > > --- > > fs/f2fs/data.c | 5 ++++- > > fs/f2fs/f2fs.h | 5 +++++ > > fs/f2fs/file.c | 27 ++++++++++++++++++--------- > > fs/f2fs/inode.c | 8 ++++++++ > > 4 files changed, 35 insertions(+), 10 deletions(-) > > Unfortunately, this patch doesn't completely fix the uninitialized data > exposure. The problem is that it only makes DIO writes fall back to buffered > writes for holes, and not for reserved blocks (NEW_ADDR). f2fs's reserved > blocks are *not* the same as the unwritten extents that other filesystems have; > f2fs's reserved blocks have to be turned into regular blocks before DIO can > write to them. That immediately exposes them to concurrent reads (at least > buffered reads, but I think DIO reads too). Isn't it resolved by i_size which gives the written blocks only? > > This can be reproduced using the aiodio_sparse program from LTP, as follows: > > aiodio_sparse -i 4 -a 8k -w 1024k -s 4096k -n 6 > > This was reported at > https://lore.kernel.org/r/20211226132851.GC34518@xsang-OptiPlex-9020 as a > regression from the commit "f2fs: use iomap for direct I/O", but it actually > failed before too. Note that it's nondeterministic; writing random data to the > block device before creating the filesystem helps with reproduction. > > I see only three possible solutions: > > * Make DIO writes to reserved blocks fall back to buffered writes, just like > writes to holes. This would mean that a file would have to be written to > before direct writes would work; fallocate() wouldn't be enough. Note that my > patch https://lore.kernel.org/r/20210728015154.171507-1-ebiggers@kernel.org > would have done this. > > * Or, change the f2fs on-disk format to support unwritten extents. > > * Or, split up block allocation into two parts, so that blocks could be > preliminarily allocated and not exposed to reads yet. This would be like > Ted's suggestion here: https://lore.kernel.org/r/YQS5eBljtztWwOFE@mit.edu > > - Eric
WARNING: multiple messages have this Message-ID (diff)
From: Jaegeuk Kim <jaegeuk@kernel.org> To: Eric Biggers <ebiggers@kernel.org> Cc: linux-kernel@vger.kernel.org, linux-f2fs-devel@lists.sourceforge.net Subject: Re: [f2fs-dev] [PATCH 3/6] f2fs: do not expose unwritten blocks to user by DIO Date: Fri, 7 Jan 2022 17:52:48 -0800 [thread overview] Message-ID: <YdjucBBopEDuUb5b@google.com> (raw) In-Reply-To: <YdjJAS7Ua4aJEFhz@sol.localdomain> On 01/07, Eric Biggers wrote: > Hi Jaegeuk, > > On Tue, Jan 04, 2022 at 01:24:16PM -0800, Jaegeuk Kim wrote: > > DIO preallocates physical blocks before writing data, but if an error occurrs > > or power-cut happens, we can see block contents from the disk. This patch tries > > to fix it by 1) turning to buffered writes for DIO into holes, 2) truncating > > unwritten blocks from error or power-cut. > > > > Signed-off-by: Jaegeuk Kim <jaegeuk@kernel.org> > > --- > > fs/f2fs/data.c | 5 ++++- > > fs/f2fs/f2fs.h | 5 +++++ > > fs/f2fs/file.c | 27 ++++++++++++++++++--------- > > fs/f2fs/inode.c | 8 ++++++++ > > 4 files changed, 35 insertions(+), 10 deletions(-) > > Unfortunately, this patch doesn't completely fix the uninitialized data > exposure. The problem is that it only makes DIO writes fall back to buffered > writes for holes, and not for reserved blocks (NEW_ADDR). f2fs's reserved > blocks are *not* the same as the unwritten extents that other filesystems have; > f2fs's reserved blocks have to be turned into regular blocks before DIO can > write to them. That immediately exposes them to concurrent reads (at least > buffered reads, but I think DIO reads too). Isn't it resolved by i_size which gives the written blocks only? > > This can be reproduced using the aiodio_sparse program from LTP, as follows: > > aiodio_sparse -i 4 -a 8k -w 1024k -s 4096k -n 6 > > This was reported at > https://lore.kernel.org/r/20211226132851.GC34518@xsang-OptiPlex-9020 as a > regression from the commit "f2fs: use iomap for direct I/O", but it actually > failed before too. Note that it's nondeterministic; writing random data to the > block device before creating the filesystem helps with reproduction. > > I see only three possible solutions: > > * Make DIO writes to reserved blocks fall back to buffered writes, just like > writes to holes. This would mean that a file would have to be written to > before direct writes would work; fallocate() wouldn't be enough. Note that my > patch https://lore.kernel.org/r/20210728015154.171507-1-ebiggers@kernel.org > would have done this. > > * Or, change the f2fs on-disk format to support unwritten extents. > > * Or, split up block allocation into two parts, so that blocks could be > preliminarily allocated and not exposed to reads yet. This would be like > Ted's suggestion here: https://lore.kernel.org/r/YQS5eBljtztWwOFE@mit.edu > > - Eric _______________________________________________ Linux-f2fs-devel mailing list Linux-f2fs-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel
next prev parent reply other threads:[~2022-01-08 1:52 UTC|newest] Thread overview: 30+ messages / expand[flat|nested] mbox.gz Atom feed top 2022-01-04 21:24 [PATCH 1/6] f2fs: rework write preallocations Jaegeuk Kim 2022-01-04 21:24 ` [f2fs-dev] " Jaegeuk Kim 2022-01-04 21:24 ` [PATCH 2/6] f2fs: reduce indentation in f2fs_file_write_iter() Jaegeuk Kim 2022-01-04 21:24 ` [f2fs-dev] " Jaegeuk Kim 2022-01-04 21:24 ` [PATCH 3/6] f2fs: do not expose unwritten blocks to user by DIO Jaegeuk Kim 2022-01-04 21:24 ` [f2fs-dev] " Jaegeuk Kim 2022-01-05 13:19 ` Chao Yu 2022-01-05 13:19 ` Chao Yu 2022-01-05 18:25 ` Jaegeuk Kim 2022-01-05 18:25 ` Jaegeuk Kim 2022-01-07 23:13 ` Eric Biggers 2022-01-07 23:13 ` [f2fs-dev] " Eric Biggers 2022-01-08 1:52 ` Jaegeuk Kim [this message] 2022-01-08 1:52 ` Jaegeuk Kim 2022-01-08 2:21 ` Eric Biggers 2022-01-08 2:21 ` [f2fs-dev] " Eric Biggers 2022-01-08 3:35 ` Jaegeuk Kim 2022-01-08 3:35 ` [f2fs-dev] " Jaegeuk Kim 2022-01-08 4:21 ` Eric Biggers 2022-01-08 4:21 ` Eric Biggers 2022-01-04 21:24 ` [PATCH 4/6] f2fs: fix the f2fs_file_write_iter tracepoint Jaegeuk Kim 2022-01-04 21:24 ` [f2fs-dev] " Jaegeuk Kim 2022-01-04 21:24 ` [f2fs-dev] [PATCH 5/6] f2fs: implement iomap operations Jaegeuk Kim 2022-01-04 21:24 ` Jaegeuk Kim 2022-01-05 13:20 ` [f2fs-dev] " Chao Yu 2022-01-05 13:20 ` Chao Yu 2022-01-05 18:25 ` Jaegeuk Kim 2022-01-05 18:25 ` Jaegeuk Kim 2022-01-04 21:24 ` [PATCH 6/6] f2fs: use iomap for direct I/O Jaegeuk Kim 2022-01-04 21:24 ` [f2fs-dev] " Jaegeuk Kim
Reply instructions: You may reply publicly to this message via plain-text email using any one of the following methods: * Save the following mbox file, import it into your mail client, and reply-to-all from there: mbox Avoid top-posting and favor interleaved quoting: https://en.wikipedia.org/wiki/Posting_style#Interleaved_style * Reply using the --to, --cc, and --in-reply-to switches of git-send-email(1): git send-email \ --in-reply-to=YdjucBBopEDuUb5b@google.com \ --to=jaegeuk@kernel.org \ --cc=ebiggers@kernel.org \ --cc=linux-f2fs-devel@lists.sourceforge.net \ --cc=linux-kernel@vger.kernel.org \ /path/to/YOUR_REPLY https://kernel.org/pub/software/scm/git/docs/git-send-email.html * If your mail client supports setting the In-Reply-To header via mailto: links, try the mailto: linkBe sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.