[PATCH v2] f2fs: return symlink writeback errors

[PATCH v2] f2fs: return symlink writeback errors

[Wenjie Qi]

F2FS writes long symlink data with page_symlink() and then flushes the
symlink mapping to reduce the chance of exposing a broken symlink.

That flush result is currently ignored. If the writeback fails, symlink()
still returns success even though the symlink is not durable and the same
operation can already surface -EIO through syncfs().

Return the writeback error to userspace and skip the dirsync flush once the
symlink data flush has failed.

Fixes: d0cae97cb600 ("f2fs: flush symlink path to avoid broken symlink after POR")
Cc: stable@kernel.org
Signed-off-by: Wenjie Qi <qiwenjie@xiaomi.com>
---
Changes since v1:
- fix the Fixes tag to point to the real introducing commit

Reproducer:
- enable FAULT_WRITE_IO on the mounted F2FS filesystem
- create a long symlink so the target is written through page_symlink()
- old kernel: symlink() returns 0, syncfs() returns -EIO, and the symlink is
  absent after remount
- new kernel: symlink() returns -EIO

 fs/f2fs/namei.c | 11 ++++++-----
 1 file changed, 6 insertions(+), 5 deletions(-)

diff --git a/fs/f2fs/namei.c b/fs/f2fs/namei.c
index 7ffdf23cea5e..a8db3928c8b5 100644
--- a/fs/f2fs/namei.c
+++ b/fs/f2fs/namei.c
@@ -707,15 +707,16 @@ static int f2fs_symlink(struct mnt_idmap *idmap, struct inode *dir,
 	 * performance regression.
 	 */
 	if (!err) {
-		filemap_write_and_wait_range(inode->i_mapping, 0,
-							disk_link.len - 1);
+		err = filemap_write_and_wait_range(inode->i_mapping, 0,
+						   disk_link.len - 1);
 
-		if (IS_DIRSYNC(dir))
+		if (!err && IS_DIRSYNC(dir))
 			f2fs_sync_fs(sbi->sb, 1);
-	} else {
-		f2fs_unlink(dir, dentry);
 	}
 
+	if (err)
+		f2fs_unlink(dir, dentry);
+
 	f2fs_balance_fs(sbi, true);
 	goto out_free_encrypted_link;
 
-- 
2.43.0

Re: [PATCH v2] f2fs: return symlink writeback errors

[Chao Yu]

On 8/10/26 21:38, Wenjie Qi wrote:
> F2FS writes long symlink data with page_symlink() and then flushes the
> symlink mapping to reduce the chance of exposing a broken symlink.
> 
> That flush result is currently ignored. If the writeback fails, symlink()
> still returns success even though the symlink is not durable and the same
> operation can already surface -EIO through syncfs().
> 
> Return the writeback error to userspace and skip the dirsync flush once the
> symlink data flush has failed.
> 
> Fixes: d0cae97cb600 ("f2fs: flush symlink path to avoid broken symlink after POR")
> Cc: stable@kernel.org
> Signed-off-by: Wenjie Qi <qiwenjie@xiaomi.com>
Reviewed-by: Chao Yu <chao@kernel.org>

Thanks,

Re: [f2fs-dev] [PATCH v2] f2fs: return symlink writeback errors

[patchwork-bot+f2fs]

Hello:

This patch was applied to jaegeuk/f2fs.git (dev)
by Jaegeuk Kim <jaegeuk@kernel.org>:

On Mon, 10 Aug 2026 21:38:32 +0800 you wrote:
> F2FS writes long symlink data with page_symlink() and then flushes the
> symlink mapping to reduce the chance of exposing a broken symlink.
> 
> That flush result is currently ignored. If the writeback fails, symlink()
> still returns success even though the symlink is not durable and the same
> operation can already surface -EIO through syncfs().
> 
> [...]

Here is the summary with links:
  - [f2fs-dev,v2] f2fs: return symlink writeback errors
    https://git.kernel.org/jaegeuk/f2fs/c/5d6f08574082

You are awesome, thank you!
-- 
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html

Re: [PATCH v2] f2fs: return symlink writeback errors

[Al Viro]

On Mon, Aug 10, 2026 at 09:38:32PM +0800, Wenjie Qi wrote:
> F2FS writes long symlink data with page_symlink() and then flushes the
> symlink mapping to reduce the chance of exposing a broken symlink.
> 
> That flush result is currently ignored. If the writeback fails, symlink()
> still returns success even though the symlink is not durable and the same
> operation can already surface -EIO through syncfs().
> 
> Return the writeback error to userspace and skip the dirsync flush once the
> symlink data flush has failed.

>  	if (!err) {
> -		filemap_write_and_wait_range(inode->i_mapping, 0,
> -							disk_link.len - 1);
> +		err = filemap_write_and_wait_range(inode->i_mapping, 0,
> +						   disk_link.len - 1);
>  
> -		if (IS_DIRSYNC(dir))
> +		if (!err && IS_DIRSYNC(dir))
>  			f2fs_sync_fs(sbi->sb, 1);
> -	} else {
> -		f2fs_unlink(dir, dentry);
>  	}
>  
> +	if (err)
> +		f2fs_unlink(dir, dentry);

That looks fishy.  At that point you already have dentry hashed and
AFAICS f2fs_unlink() will leave it hashed and attached to the same
inode; sure, memory pressure will eventually evict the sucker, but
until that point any lookups will simply pick it from dcache.

It's not introduced by this patch; the same issue, AFAICS, already exists
in mainline.  Why do we even bother with d_instantiate_new() before we
know that everything's fine, nevermind doing that when we already know
the operation has failed?

Incidentally, is there any reason to add a directory entry before the
inode is set up?  Usually that's the last step, and cleanup tends to
be simpler that way; are there f2fs-specific reasons to do it in the
unusual order?