omfs: fix inverted check and use-after-brelse in omfs_dir_is_empty() - #2612
Open
vfsci-bot[bot] wants to merge 1 commit into
Open
vfsci-bot[bot] wants to merge 1 commit into
vfsci-bot[bot] wants to merge 1 commit into
Conversation
omfs_dir_is_empty() scans the directory's hash bucket table for an
occupied slot (~0 marks an empty bucket) and then returns:
for (i = 0; i < nbuckets; i++, ptr++)
if (*ptr != ~0)
break;
brelse(bh);
return *ptr != ~0;
That return statement has three bugs at once:
1. The sense is backwards. Its only caller is omfs_remove():
if (S_ISDIR(inode->i_mode) &&
!omfs_dir_is_empty(inode))
return -ENOTEMPTY;
When the directory is NOT empty the loop breaks on the first
occupied bucket, so *ptr != ~0 is true, omfs_dir_is_empty() returns
1 ("empty"), -ENOTEMPTY is skipped, and rmdir unlinks the directory
while leaving its children allocated and unreachable.
2. When the directory IS empty the loop runs to i == nbuckets, leaving
ptr one u64 past the end of the bucket array - which for a standard
block size is one u64 past the end of bh->b_data itself. Both the
out-of-bounds read and the emptiness verdict then depend on whatever
sits in memory after the buffer.
3. Either way, ptr points into bh->b_data and is dereferenced after
brelse(bh) has dropped the buffer reference.
With KASAN enabled, rmdir of an empty directory reports:
==================================================================
BUG: KASAN: use-after-free in omfs_remove+0x265/0x270
Read of size 8 at addr ffff88811a145000 by task init/1
CPU: 1 UID: 0 PID: 1 Comm: init Tainted: G B D 7.3.0-rc3 #1
Call Trace:
<TASK>
dump_stack_lvl+0x70/0xa0
print_report+0x153/0x4c6
kasan_report+0xf1/0x120
omfs_remove+0x265/0x270
vfs_rmdir+0x2e6/0x810
filename_rmdir+0x3bf/0x530
__x64_sys_rmdir+0x4b/0x70
do_syscall_64+0xda/0x4b0
entry_SYSCALL_64_after_hwframe+0x77/0x7f
</TASK>
==================================================================
All the information needed is already in the loop counter: the directory
is empty iff the scan reached nbuckets without breaking. Return
`i == nbuckets`, which neither touches the buffer after brelse() nor
reads past the end of the array, and gives omfs_remove() the polarity it
expects.
Fixes: a3ab715 ("omfs: add directory routines")
Assisted-by: LLM
Signed-off-by: Hui Peng <benquike@gmail.com>
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.
Series: https://patchwork.kernel.org/project/linux-fsdevel/list/?series=1169387
Submitter: Hui Peng
Version: 1
Patches: 1/1
Message-ID:
<20260919202053.2435593-1-benquike@gmail.com>Base: vfs.base.ci
Lore: https://lore.kernel.org/linux-fsdevel/20260919202053.2435593-1-benquike@gmail.com
Automated by ml2pr