Skip to content

Commit d0e64a9

Browse files
fdmananakdave
authored andcommitted
btrfs: always log symlinks in full mode
On Linux, empty symlinks are invalid, and attempting to create one with the system call symlink(2) results in an -ENOENT error and this is explicitly documented in the man page. If we rename a symlink that was created in the current transaction and its parent directory was logged before, we actually end up logging the symlink without logging its content, which is stored in an inline extent. That means that after a power failure we can end up with an empty symlink, having no content and an i_size of 0 bytes. It can be easily reproduced like this: $ mkfs.btrfs -f /dev/sdc $ mount /dev/sdc /mnt $ mkdir /mnt/testdir $ sync # Create a file inside the directory and fsync the directory. $ touch /mnt/testdir/foo $ xfs_io -c "fsync" /mnt/testdir # Create a symlink inside the directory and then rename the symlink. $ ln -s /mnt/testdir/foo /mnt/testdir/bar $ mv /mnt/testdir/bar /mnt/testdir/baz # Now fsync again the directory, this persist the log tree. $ xfs_io -c "fsync" /mnt/testdir <power failure> $ mount /dev/sdc /mnt $ stat -c %s /mnt/testdir/baz 0 $ readlink /mnt/testdir/baz $ Fix this by always logging symlinks in full mode (LOG_INODE_ALL), so that their content is also logged. A test case for fstests will follow. CC: [email protected] # 4.9+ Signed-off-by: Filipe Manana <[email protected]> Signed-off-by: David Sterba <[email protected]>
1 parent 0e852ab commit d0e64a9

File tree

1 file changed

+13
-1
lines changed

1 file changed

+13
-1
lines changed

fs/btrfs/tree-log.c

Lines changed: 13 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -5804,6 +5804,18 @@ static int btrfs_log_inode(struct btrfs_trans_handle *trans,
58045804
mutex_lock(&inode->log_mutex);
58055805
}
58065806

5807+
/*
5808+
* For symlinks, we must always log their content, which is stored in an
5809+
* inline extent, otherwise we could end up with an empty symlink after
5810+
* log replay, which is invalid on linux (symlink(2) returns -ENOENT if
5811+
* one attempts to create an empty symlink).
5812+
* We don't need to worry about flushing delalloc, because when we create
5813+
* the inline extent when the symlink is created (we never have delalloc
5814+
* for symlinks).
5815+
*/
5816+
if (S_ISLNK(inode->vfs_inode.i_mode))
5817+
inode_only = LOG_INODE_ALL;
5818+
58075819
/*
58085820
* Before logging the inode item, cache the value returned by
58095821
* inode_logged(), because after that we have the need to figure out if
@@ -6182,7 +6194,7 @@ static int log_new_dir_dentries(struct btrfs_trans_handle *trans,
61826194
}
61836195

61846196
ctx->log_new_dentries = false;
6185-
if (type == BTRFS_FT_DIR || type == BTRFS_FT_SYMLINK)
6197+
if (type == BTRFS_FT_DIR)
61866198
log_mode = LOG_INODE_ALL;
61876199
ret = btrfs_log_inode(trans, BTRFS_I(di_inode),
61886200
log_mode, ctx);

0 commit comments

Comments
 (0)