All the mail mirrored from lore.kernel.org
 help / color / mirror / Atom feed
From: Eric Biggers <ebiggers@kernel.org>
To: linux-f2fs-devel@lists.sourceforge.net,
	Jaegeuk Kim <jaegeuk@kernel.org>, Chao Yu <chao@kernel.org>
Cc: linux-fsdevel@vger.kernel.org, stable@vger.kernel.org
Subject: Re: [PATCH] f2fs: remove broken support for allocating DIO writes
Date: Fri, 30 Jul 2021 12:17:26 -0700	[thread overview]
Message-ID: <YQRQRh1zUHSIzcC/@gmail.com> (raw)
In-Reply-To: <20210728015154.171507-1-ebiggers@kernel.org>

On Tue, Jul 27, 2021 at 06:51:54PM -0700, Eric Biggers wrote:
> From: Eric Biggers <ebiggers@google.com>
> 
> Currently, non-overwrite DIO writes are fundamentally unsafe on f2fs as
> they require preallocating blocks, but f2fs doesn't support unwritten
> blocks and therefore has to preallocate the blocks as regular blocks.
> f2fs has no way to reliably roll back such preallocations, so as a
> result, f2fs will leak uninitialized blocks to users if a DIO write
> doesn't fully complete.  This can be easily reproduced by issuing a DIO
> write that will fail due to misalignment, e.g.:
> 
> 	rm -f file
> 	truncate -s 1000000 file
> 	dd if=/dev/zero bs=999999 oflag=direct conv=notrunc of=file
> 	od -tx1 file  # shows uninitialized disk blocks
> 
> Until a proper design for non-overwrite DIO writes on f2fs can be
> designed and implemented, remove support for them and make them fall
> back to buffered I/O.  This is what other filesystems that don't support
> unwritten blocks, e.g. ext2, also do, at least for non-extending DIO
> writes.  However, f2fs can't do extending DIO writes either, as f2fs
> appears to have no mechanism for guaranteeing that leftover allocated
> blocks past EOF will get truncated.  (f2fs does have an orphan list, but
> it's only used for deleting inodes, not truncating them.)
> 
> This patch doesn't attempt to remove the F2FS_GET_BLOCK_{DIO,PRE_DIO}
> cases in f2fs_map_blocks(); that can be cleaned up later.
> 
> Fixes: bfad7c2d4033 ("f2fs: introduce a new direct_IO write path")
> Cc: stable@vger.kernel.org
> Signed-off-by: Eric Biggers <ebiggers@google.com>
> ---

Any opinion on this patch?  This really needs to be fixed one way or another.
Probably before the conversion to iomap, as this fix will need to be backported.

- Eric

WARNING: multiple messages have this Message-ID (diff)
From: Eric Biggers <ebiggers@kernel.org>
To: linux-f2fs-devel@lists.sourceforge.net,
	Jaegeuk Kim <jaegeuk@kernel.org>, Chao Yu <chao@kernel.org>
Cc: linux-fsdevel@vger.kernel.org, stable@vger.kernel.org
Subject: Re: [f2fs-dev] [PATCH] f2fs: remove broken support for allocating DIO writes
Date: Fri, 30 Jul 2021 12:17:26 -0700	[thread overview]
Message-ID: <YQRQRh1zUHSIzcC/@gmail.com> (raw)
In-Reply-To: <20210728015154.171507-1-ebiggers@kernel.org>

On Tue, Jul 27, 2021 at 06:51:54PM -0700, Eric Biggers wrote:
> From: Eric Biggers <ebiggers@google.com>
> 
> Currently, non-overwrite DIO writes are fundamentally unsafe on f2fs as
> they require preallocating blocks, but f2fs doesn't support unwritten
> blocks and therefore has to preallocate the blocks as regular blocks.
> f2fs has no way to reliably roll back such preallocations, so as a
> result, f2fs will leak uninitialized blocks to users if a DIO write
> doesn't fully complete.  This can be easily reproduced by issuing a DIO
> write that will fail due to misalignment, e.g.:
> 
> 	rm -f file
> 	truncate -s 1000000 file
> 	dd if=/dev/zero bs=999999 oflag=direct conv=notrunc of=file
> 	od -tx1 file  # shows uninitialized disk blocks
> 
> Until a proper design for non-overwrite DIO writes on f2fs can be
> designed and implemented, remove support for them and make them fall
> back to buffered I/O.  This is what other filesystems that don't support
> unwritten blocks, e.g. ext2, also do, at least for non-extending DIO
> writes.  However, f2fs can't do extending DIO writes either, as f2fs
> appears to have no mechanism for guaranteeing that leftover allocated
> blocks past EOF will get truncated.  (f2fs does have an orphan list, but
> it's only used for deleting inodes, not truncating them.)
> 
> This patch doesn't attempt to remove the F2FS_GET_BLOCK_{DIO,PRE_DIO}
> cases in f2fs_map_blocks(); that can be cleaned up later.
> 
> Fixes: bfad7c2d4033 ("f2fs: introduce a new direct_IO write path")
> Cc: stable@vger.kernel.org
> Signed-off-by: Eric Biggers <ebiggers@google.com>
> ---

Any opinion on this patch?  This really needs to be fixed one way or another.
Probably before the conversion to iomap, as this fix will need to be backported.

- Eric


_______________________________________________
Linux-f2fs-devel mailing list
Linux-f2fs-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel

  reply	other threads:[~2021-07-30 19:17 UTC|newest]

Thread overview: 42+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-07-28  1:51 [f2fs-dev] [PATCH] f2fs: remove broken support for allocating DIO writes Eric Biggers
2021-07-28  1:51 ` Eric Biggers
2021-07-30 19:17 ` Eric Biggers [this message]
2021-07-30 19:17   ` Eric Biggers
2021-07-30 22:12   ` [f2fs-dev] " Jaegeuk Kim
2021-07-30 22:12     ` Jaegeuk Kim
2021-07-30 22:19     ` Eric Biggers
2021-07-30 22:19       ` [f2fs-dev] " Eric Biggers
2021-07-31  1:05       ` Jaegeuk Kim
2021-07-31  1:05         ` [f2fs-dev] " Jaegeuk Kim
2021-07-31  1:18         ` Eric Biggers
2021-07-31  1:18           ` Eric Biggers
2021-07-31  2:46   ` Theodore Ts'o
2021-07-31  2:46     ` [f2fs-dev] " Theodore Ts'o
2021-08-02  4:39     ` Eric Biggers
2021-08-02  4:39       ` Eric Biggers
2021-08-02  9:00       ` Chao Yu
2021-08-02  9:00         ` [f2fs-dev] " Chao Yu
2021-08-02 18:23         ` Jaegeuk Kim
2021-08-02 18:23           ` Jaegeuk Kim
2021-08-03  1:19           ` [f2fs-dev] " Chao Yu
2021-08-03  1:19             ` Chao Yu
2021-08-03  1:34             ` [f2fs-dev] " Jaegeuk Kim
2021-08-03  1:34               ` Jaegeuk Kim
2021-08-17  2:03               ` [f2fs-dev] " Eric Biggers
2021-08-17  2:03                 ` Eric Biggers
2021-08-17  5:42                 ` [f2fs-dev] " Christoph Hellwig
2021-08-17  5:42                   ` Christoph Hellwig
2021-08-17 18:57                   ` Jaegeuk Kim
2021-08-17 18:57                     ` [f2fs-dev] " Jaegeuk Kim
2021-08-17 20:27                     ` Eric Biggers
2021-08-17 20:27                       ` [f2fs-dev] " Eric Biggers
2021-08-17 21:33                       ` Jaegeuk Kim
2021-08-17 21:33                         ` Jaegeuk Kim
2021-08-18  0:06                         ` [f2fs-dev] " Eric Biggers
2021-08-18  0:06                           ` Eric Biggers
2021-08-20  9:35                 ` Chao Yu
2021-08-20  9:35                   ` [f2fs-dev] " Chao Yu
2021-08-20 18:11                   ` Eric Biggers
2021-08-20 18:11                     ` Eric Biggers
2021-08-20 22:01                     ` [f2fs-dev] " Chao Yu
2021-08-20 22:01                       ` Chao Yu

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=YQRQRh1zUHSIzcC/@gmail.com \
    --to=ebiggers@kernel.org \
    --cc=chao@kernel.org \
    --cc=jaegeuk@kernel.org \
    --cc=linux-f2fs-devel@lists.sourceforge.net \
    --cc=linux-fsdevel@vger.kernel.org \
    --cc=stable@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: link
Be 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.