Skip to content

Implement Drop for Directory in tsk. - #244

Open
grrrrrrrrr wants to merge 2 commits into
google:masterfrom
grrrrrrrrr:fix-tsk-directory-leak
Open

Implement Drop for Directory in tsk.#244
grrrrrrrrr wants to merge 2 commits into
google:masterfrom
grrrrrrrrr:fix-tsk-directory-leak

Conversation

@grrrrrrrrr

@grrrrrrrrr grrrrrrrrr commented Aug 28, 2026

Copy link
Copy Markdown
Collaborator

Directory wraps a raw TSK_FS_DIR pointer obtained from
tsk_fs_dir_open or tsk_fs_dir_open_meta. In SleuthKit, directory
structures are dynamically allocated and must be freed with
tsk_fs_dir_close. Because Directory lacked a Drop implementation,
every opened directory leaked memory and underlying file handles.

This change implements Drop for Directory<'_> calling tsk_fs_dir_close
and relaxes iter_entries/file from &mut self to &self (the
underlying tsk_fs_dir_get takes const TSK_FS_DIR * and returns freshly
allocated files, so shared iteration is sound).

tsk_fs_dir_close also frees TSK_FS_DIR::fs_file, which is what
Directory::file hands back. Returning that File with the file system
lifetime 'a let it outlive the Directory, so once Directory has a
Drop a pattern like

let f = { let d = fs.open_dir(p)?; d.file() };

becomes a use-after-free. Directory::file now borrows self, tying the
returned File to the Directory it came from.

Note: File itself still has no Drop, so files from open_file and
iter_entries continue to leak TSK_FS_FILE. Fixing that needs an
owned/borrowed distinction (the walk_dir callback and Directory::file
must not close their File) and is left for a follow-up.

`Directory` wraps a raw `TSK_FS_DIR` pointer obtained from
`tsk_fs_dir_open` or `tsk_fs_dir_open_meta`. In SleuthKit, directory
structures are dynamically allocated and must be freed with
`tsk_fs_dir_close`. Because `Directory` lacked a `Drop` implementation,
every opened directory leaked memory and underlying file handles.

This change implements `Drop for Directory<'_>` calling `tsk_fs_dir_close`
and updates `iter_entries` and `DirectoryIterator` to borrow `&self`
with a decoupled iterator lifetime, allowing `Directory` instances to be
cleanly dropped once iteration completes.
@grrrrrrrrr
grrrrrrrrr force-pushed the fix-tsk-directory-leak branch from 899fdc0 to 03b9e31 Compare August 28, 2026 13:41
The File returned by Directory::file wraps TSK_FS_DIR::fs_file, which
tsk_fs_dir_close frees when the Directory is dropped. Returning it with
the file system lifetime 'a let it outlive the Directory, so with the
new Drop impl a pattern like

    let f = { let d = fs.open_dir(p)?; d.file() };

compiled into a use-after-free. Tie the returned File to the borrow of
self instead.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant