shinys000114/linux
1
0
Fork 0
mirror of https://github.com/hardkernel/linux.git synced 2026-06-06 10:58:48 +09:00
Code Issues Packages Projects Releases Wiki Activity

btrfs: avoid load/store tearing races when checking if an inode was logged

Browse Source 
[ Upstream commit 986bf6ed44dff7fbae7b43a0882757ee7f5ba21b ]

At inode_logged() we do a couple lockless checks for ->logged_trans, and
these are generally safe except the second one in case we get a load or
store tearing due to a concurrent call updating ->logged_trans (either at
btrfs_log_inode() or later at inode_logged()).

In the first case it's safe to compare to the current transaction ID since
once ->logged_trans is set the current transaction, we never set it to a
lower value.

In the second case, where we check if it's greater than zero, we are prone
to load/store tearing races, since we can have a concurrent task updating
to the current transaction ID with store tearing for example, instead of
updating with a single 64 bits write, to update with two 32 bits writes or
four 16 bits writes. In that case the reading side at inode_logged() could
see a positive value that does not match the current transaction and then
return a false negative.

Fix this by doing the second check while holding the inode's spinlock, add
some comments about it too. Also add the data_race() annotation to the
first check to avoid any reports from KCSAN (or similar tools) and comment
about it.

Fixes: 0f8ce49821 ("btrfs: avoid inode logging during rename and link when possible")
Reviewed-by: Boris Burkov <boris@bur.io>
Signed-off-by: Filipe Manana <fdmanana@suse.com>
Signed-off-by: David Sterba <dsterba@suse.com>
Signed-off-by: Sasha Levin <sashal@kernel.org>
This commit is contained in:
Filipe Manana
2025-08-06 12:11:32 +01:00
committed by Greg Kroah-Hartman
parent 48545620ca
commit dc4a024b19
1 changed files with 21 additions and 4 deletions
Download Patch File Download Diff File Expand all files Collapse all files

25
fs/btrfs/tree-log.c
View File

@@ -3398,15 +3398,32 @@ static int inode_logged(const struct btrfs_trans_handle *trans,
struct btrfs_key key;
int ret;
if (inode->logged_trans == trans->transid)
/*
* Quick lockless call, since once ->logged_trans is set to the current
* transaction, we never set it to a lower value anywhere else.
*/
if (data_race(inode->logged_trans) == trans->transid)
return 1;
/*
* If logged_trans is not 0, then we know the inode logged was not logged
* in this transaction, so we can return false right away.
* If logged_trans is not 0 and not trans->transid, then we know the
* inode was not logged in this transaction, so we can return false
* right away. We take the lock to avoid a race caused by load/store
* tearing with a concurrent btrfs_log_inode() call or a concurrent task
* in this function further below - an update to trans->transid can be
* teared into two 32 bits updates for example, in which case we could
* see a positive value that is not trans->transid and assume the inode
* was not logged when it was.
*/
if (inode->logged_trans > 0)
spin_lock(&inode->lock);
if (inode->logged_trans == trans->transid) {
spin_unlock(&inode->lock);
return 1;
} else if (inode->logged_trans > 0) {
spin_unlock(&inode->lock);
return 0;
}
spin_unlock(&inode->lock);
/*
* If no log tree was created for this root in this transaction, then