Fix locking and a double free in fsops rename and unlink#615
Open
matejk wants to merge 2 commits into
Open
Conversation
ltfs_fsops_rename ran the directory WORM checks right after lookup, before todir's meta_lock was acquired, then jumped to out_release, which releases todir->meta_lock via fs_release_dentry_unlocked() whenever todir != fromdir. Renaming into or out of a WORM directory therefore unlocked a lock that was never held, and the immutable/ appendonly fields were read without meta_lock. Move the check to after both directory meta_locks are held, mirroring the existing source/target entry WORM check. ltfs_fsops_unlink had the same defect: the shared out: path releases parent->meta_lock, but the lock was only acquired after the WORM and non-empty-directory checks, so those error paths unlocked a lock they never held. Acquire parent->meta_lock once before the checks; lock ordering is preserved (parent contents before parent meta before child meta). Releasing an unheld rwlock is undefined behaviour and can corrupt the lock state.
After the destination name buffers are assigned to fromdentry, a later failure (e.g. fs_add_key_to_hash_table) reached out_free, which freed those same buffers because ret < 0 — leaving fromdentry with dangling name/platform_safe_name pointers that are freed again when the dentry is disposed. Clear the locals once ownership moves to fromdentry.
matejk
force-pushed
the
fix/fsops-locking
branch
from
9372f75 to
bbdf041
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Two commits:
ltfs_fsops_rename, the directory WORM checks ran beforetodir'smeta_lockwas acquired but their error path released it (whenevertodir != fromdir);ltfs_fsops_unlinkhad the same defect forparent->meta_lockon its WORM and non-empty-directory error paths. Releasing an unheld rwlock is undefined behaviour and can corrupt the lock state. The checks now run with the locks held; lock ordering is preserved.fromdentry, a later failure freed them viaout_freeand left dangling pointers that were freed again when the dentry is disposed. Ownership is now cleared from the locals when it moves.