stuartl/linux-stable
1
0
Fork 0
mirror of https://kernel.googlesource.com/pub/scm/linux/kernel/git/stable/linux-stable.git synced 2025-09-28 17:40:21 +10:00
Code Issues Releases Wiki Activity

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

Browse Source 
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>
This commit is contained in:
Filipe Manana 2025-08-06 12:11:32 +01:00 committed by David Sterba
parent 59a0dd4ab9
commit 986bf6ed44
1 changed files with 21 additions and 4 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

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

@ -3382,15 +3382,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