[PATCH] omfs: fix UAF and broken empty-check in omfs_dir_is_empty()

Farhad Alemi posted 1 patch 1 week, 6 days ago
fs/omfs/dir.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
[PATCH] omfs: fix UAF and broken empty-check in omfs_dir_is_empty()
Posted by Farhad Alemi 1 week, 6 days ago
omfs_dir_is_empty() releases the buffer head and then dereferences a
pointer that lives inside its b_data:

	brelse(bh);
	return *ptr != ~0;

Once the buffer is released the underlying page may be reclaimed, so
the post-brelse read is a slab-use-after-free. KASAN reports:

  BUG: KASAN: use-after-free in omfs_dir_is_empty fs/omfs/dir.c:235
  Read of size 8 at addr ffff8881...

The same statement is also semantically inverted. The loop above
breaks when a non-`~0` (occupied) bucket is found, so on the
break-out path the original `*ptr != ~0` would read as 1 whenever
the freed page still held its prior contents (the common case
immediately after brelse); combined with the caller in
omfs_remove():

	if (S_ISDIR(inode->i_mode) && !omfs_dir_is_empty(inode))
		return -ENOTEMPTY;

this means rmdir(2) silently succeeded on non-empty OMFS directories,
leaving their children as orphan inodes. On the loop-completion path
(no non-`~0` bucket found), ptr was advanced one element past the
bucket array, so the original return was also a latent
out-of-bounds read.

The loop's exit condition `i == nbuckets` is precisely "no occupied
bucket was found", i.e. "the directory is empty". Returning that
instead of dereferencing `*ptr` removes the UAF, removes the latent
OOB on the empty-directory path, and corrects the function's
semantics to match its name and the caller contract.

Reported-by: Farhad Alemi <farhad.alemi@berkeley.edu>
Signed-off-by: Farhad Alemi <farhad.alemi@berkeley.edu>
---
 fs/omfs/dir.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/fs/omfs/dir.c b/fs/omfs/dir.c
index 2ed541fccf33..c6ba390aabd3 100644
--- a/fs/omfs/dir.c
+++ b/fs/omfs/dir.c
@@ -232,7 +232,7 @@ static int omfs_dir_is_empty(struct inode *inode)
 			break;

 	brelse(bh);
-	return *ptr != ~0;
+	return i == nbuckets;
 }

 static int omfs_remove(struct inode *dir, struct dentry *dentry)
--
2.43.0