From e379b4ec8bdb0898bd9da4b26088d97e7e9352c2 Mon Sep 17 00:00:00 2001 From: Matej Kenda Date: Fri, 12 Jun 2026 12:51:57 +0200 Subject: [PATCH 1/2] fix: releases of unheld meta_lock in rename and unlink error paths 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. --- src/libltfs/ltfs_fsops.c | 37 ++++++++++++++++++++++++------------- 1 file changed, 24 insertions(+), 13 deletions(-) diff --git a/src/libltfs/ltfs_fsops.c b/src/libltfs/ltfs_fsops.c index 3b7f57d6..c90edf22 100644 --- a/src/libltfs/ltfs_fsops.c +++ b/src/libltfs/ltfs_fsops.c @@ -460,6 +460,13 @@ int ltfs_fsops_unlink(const char *path, ltfs_file_id *id, struct ltfs_volume *vo } parent = d->parent; + /* Acquire parent->meta_lock up front: the shared out: path releases it + * via fs_release_dentry_unlocked(), so every goto out (including the WORM + * and non-empty-directory error paths below) must hold it. Ordering is + * preserved: parent->contents_lock (held by fs_path_lookup) before + * parent->meta_lock before the child d->meta_lock. */ + acquirewrite_mrsw(&parent->meta_lock); + if (parent->is_immutable || parent->is_appendonly) { ltfsmsg(LTFS_ERR, 17237E, "unlink: parent is WORM"); ret = -LTFS_WORM_ENABLED; @@ -482,7 +489,6 @@ int ltfs_fsops_unlink(const char *path, ltfs_file_id *id, struct ltfs_volume *vo goto out; } - acquirewrite_mrsw(&parent->meta_lock); acquirewrite_mrsw(&d->meta_lock); if (dcache_initialized(vol)) { @@ -640,18 +646,9 @@ int ltfs_fsops_rename(const char *from, const char *to, ltfs_file_id *id, struct goto out_release; } - if (fromdir->is_appendonly || fromdir->is_immutable ) { - ltfsmsg(LTFS_ERR, 17237E, "rename: parent is WORM"); - ret = -LTFS_WORM_ENABLED; - acquirewrite_mrsw(&fromdir->meta_lock); - goto out_release; - } - if (todir->is_immutable || todir->is_appendonly) { - ltfsmsg(LTFS_ERR, 17237E, "rename: target dir is WORM"); - ret = -LTFS_WORM_ENABLED; - acquirewrite_mrsw(&fromdir->meta_lock); - goto out_release; - } + /* The WORM state of the source and target directories is checked below, + * after both directory meta_locks are held (the fields are meta_lock + * protected, and the out_release path expects both meta_locks held). */ /* Take locks in the appropriate order and look up the source and destination dentries */ if (todir == fromdir || fs_is_predecessor(todir, fromdir)) { @@ -760,6 +757,20 @@ int ltfs_fsops_rename(const char *from, const char *to, ltfs_file_id *id, struct } #endif + /* Reject renames into or out of a WORM directory. Checked here, with both + * directory meta_locks held, rather than right after lookup: the fields are + * meta_lock protected and the out_unlock/out_release path releases both + * directory meta_locks via fs_release_dentry_unlocked. */ + if (fromdir->is_immutable || fromdir->is_appendonly || + todir->is_immutable || todir->is_appendonly) { + ltfsmsg(LTFS_ERR, 17237E, "rename: source or target dir is WORM"); + ret = -LTFS_WORM_ENABLED; + fs_release_dentry(fromdentry); + if (todentry && todentry != fromdentry) + fs_release_dentry(todentry); + goto out_unlock; + } + if (fromdentry->is_immutable || fromdentry->is_appendonly) { ltfsmsg(LTFS_ERR, 17237E, "rename: src entry is WORM"); ret = -LTFS_WORM_ENABLED; From bbdf0419bd0f8f41ab4b68541713400dc609cc26 Mon Sep 17 00:00:00 2001 From: Matej Kenda Date: Wed, 10 Jun 2026 23:35:53 +0200 Subject: [PATCH 2/2] fix: double free of the new name on a late rename failure MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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. --- src/libltfs/ltfs_fsops.c | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/src/libltfs/ltfs_fsops.c b/src/libltfs/ltfs_fsops.c index c90edf22..84440f04 100644 --- a/src/libltfs/ltfs_fsops.c +++ b/src/libltfs/ltfs_fsops.c @@ -868,6 +868,10 @@ int ltfs_fsops_rename(const char *from, const char *to, ltfs_file_id *id, struct fromdentry->name.percent_encode = fs_is_percent_encode_required(fromdentry->name.name); fromdentry->platform_safe_name = to_filename_copy2; fromdentry->matches_name_criteria = index_criteria_match(fromdentry, vol); + /* Ownership of both buffers has moved to fromdentry; clear the locals so + * the out_free path does not free them again if a later step fails. */ + to_filename_copy = NULL; + to_filename_copy2 = NULL; /* Add fromdentry to new directory */ todir->child_list = fs_add_key_to_hash_table(todir->child_list, fromdentry, &ret);