From 5ef51edc5c4b06154b25a7979f5c8b91a3d27cc1 Mon Sep 17 00:00:00 2001 From: zyc107109102 Date: Wed, 27 May 2026 17:21:04 +0000 Subject: [PATCH 1/4] fix(rsext4): use physical byte offset in readdir to fix rm -rf skipping entries --- components/rsext4/src/file/delete.rs | 33 +--- .../axfs-ng/src/fs/ext4/rsext4/inode.rs | 43 +++-- .../bugfix/bug-ext4-dir-ops/c/CMakeLists.txt | 2 +- .../bugfix/bug-ext4-dir-ops/c/src/main.c | 164 ++++++++++++++++++ 4 files changed, 206 insertions(+), 36 deletions(-) diff --git a/components/rsext4/src/file/delete.rs b/components/rsext4/src/file/delete.rs index b0e57be666..f2d9396403 100644 --- a/components/rsext4/src/file/delete.rs +++ b/components/rsext4/src/file/delete.rs @@ -154,8 +154,6 @@ fn remove_dentry_in_dir_block( ) -> bool { let block_bytes = BLOCK_SIZE; let mut offset: usize = 0; - let mut prev_off: Option = None; - let mut prev_rec_len: u16 = 0; while offset + 8 <= block_bytes { let inode = u32::from_le_bytes([ data[offset], @@ -178,27 +176,14 @@ fn remove_dentry_in_dir_block( if name_len > 0 && offset + 8 + name_len <= entry_end { let name = &data[offset + 8..offset + 8 + name_len]; if inode != 0 && name == name_bytes { - if let Some(poff) = prev_off { - // Merge current entry's space into previous entry. - let new_len = prev_rec_len.saturating_add(rec_len); - let bytes = new_len.to_le_bytes(); - data[poff + 4] = bytes[0]; - data[poff + 5] = bytes[1]; - - // Clear current entry inode so it will be treated as free. - let zero = 0u32.to_le_bytes(); - data[offset] = zero[0]; - data[offset + 1] = zero[1]; - data[offset + 2] = zero[2]; - data[offset + 3] = zero[3]; - } else { - // No previous entry in this block: mark this entry free. - let zero = 0u32.to_le_bytes(); - data[offset] = zero[0]; - data[offset + 1] = zero[1]; - data[offset + 2] = zero[2]; - data[offset + 3] = zero[3]; - } + // Mark entry as deleted by zeroing inode. Do NOT merge rec_len + // into the previous entry — keeping rec_len unchanged preserves + // stable byte offsets for readdir (getdents64) across deletions. + let zero = 0u32.to_le_bytes(); + data[offset] = zero[0]; + data[offset + 1] = zero[1]; + data[offset + 2] = zero[2]; + data[offset + 3] = zero[3]; update_ext4_dirblock_csum32( superblock, parent_ino_num.raw(), @@ -211,8 +196,6 @@ fn remove_dentry_in_dir_block( if entry_end >= block_bytes { break; } - prev_off = Some(offset); - prev_rec_len = rec_len; offset = entry_end; } false diff --git a/os/arceos/modules/axfs-ng/src/fs/ext4/rsext4/inode.rs b/os/arceos/modules/axfs-ng/src/fs/ext4/rsext4/inode.rs index 0ab1565a79..27861ac2a1 100644 --- a/os/arceos/modules/axfs-ng/src/fs/ext4/rsext4/inode.rs +++ b/os/arceos/modules/axfs-ng/src/fs/ext4/rsext4/inode.rs @@ -422,7 +422,7 @@ impl DirNodeOps for Inode { let blocks = rsext4::loopfile::resolve_inode_block_allextend(fs, dev, &mut inode) .map_err(into_vfs_err)?; - let mut idx = 0u64; + let mut byte_offset: u64 = 0; let mut count = 0usize; for &phys in blocks.values() { let cached = fs @@ -430,21 +430,44 @@ impl DirNodeOps for Inode { .get_or_load(dev, phys) .map_err(into_vfs_err)?; let data = &cached.data[..BLOCK_SIZE]; - let iter = rsext4::entries::DirEntryIterator::new(data); - for (entry, _) in iter { - if entry.inode == 0 { + + // Manually iterate entries, tracking byte_offset for ALL entries + // (including inode==0 deleted ones) so offset stays physical. + let mut pos = 0usize; + while pos + 8 <= data.len() { + let entry_inode = + u32::from_le_bytes([data[pos], data[pos + 1], data[pos + 2], data[pos + 3]]); + let rec_len = u16::from_le_bytes([data[pos + 4], data[pos + 5]]); + if rec_len < 8 { + break; + } + let rec_usize = rec_len as usize; + if pos + rec_usize > data.len() { + break; + } + + let entry_offset = byte_offset; + byte_offset += rec_len as u64; + pos += rec_usize; + + if entry_inode == 0 { + continue; + } + if entry_offset < offset { continue; } - if idx < offset { - idx += 1; + + let name_len = data[pos - rec_usize + 6] as usize; + let file_type = data[pos - rec_usize + 7]; + let name_start = pos - rec_usize + 8; + if name_len > rec_usize - 8 { continue; } - let name = core::str::from_utf8(entry.name) + let name = core::str::from_utf8(&data[name_start..name_start + name_len]) .map_err(|_| VfsError::InvalidData)? .to_owned(); - let node_type = dir_entry_type_to_vfs(entry.file_type); - idx += 1; - if !sink.accept(&name, entry.inode as u64, node_type, idx) { + let node_type = dir_entry_type_to_vfs(file_type); + if !sink.accept(&name, entry_inode as u64, node_type, byte_offset) { return Ok(count); } count += 1; diff --git a/test-suit/starryos/normal/qemu-smp1/bugfix/bug-ext4-dir-ops/c/CMakeLists.txt b/test-suit/starryos/normal/qemu-smp1/bugfix/bug-ext4-dir-ops/c/CMakeLists.txt index 09c8e28dec..4e5049e140 100644 --- a/test-suit/starryos/normal/qemu-smp1/bugfix/bug-ext4-dir-ops/c/CMakeLists.txt +++ b/test-suit/starryos/normal/qemu-smp1/bugfix/bug-ext4-dir-ops/c/CMakeLists.txt @@ -4,5 +4,5 @@ set(CMAKE_C_STANDARD 11) set(CMAKE_C_STANDARD_REQUIRED ON) set(CMAKE_C_EXTENSIONS OFF) add_executable(bug-ext4-dir-ops src/main.c) -target_compile_options(bug-ext4-dir-ops PRIVATE -Wall -Wextra -Werror) +target_compile_options(bug-ext4-dir-ops PRIVATE -Wall -Wextra -Werror -Wno-format-truncation) install(TARGETS bug-ext4-dir-ops RUNTIME DESTINATION usr/bin) diff --git a/test-suit/starryos/normal/qemu-smp1/bugfix/bug-ext4-dir-ops/c/src/main.c b/test-suit/starryos/normal/qemu-smp1/bugfix/bug-ext4-dir-ops/c/src/main.c index 20f11a88f1..480a3a505f 100644 --- a/test-suit/starryos/normal/qemu-smp1/bugfix/bug-ext4-dir-ops/c/src/main.c +++ b/test-suit/starryos/normal/qemu-smp1/bugfix/bug-ext4-dir-ops/c/src/main.c @@ -13,8 +13,18 @@ #include #include #include +#include #include +/* linux_dirent64 for getdents64 syscall */ +struct linux_dirent64 { + unsigned long long d_ino; + long long d_off; + unsigned short d_reclen; + unsigned char d_type; + char d_name[]; +}; + #define BASE "/root/bug-ext4-dir-ops-test" /* ========== 辅助函数 ========== */ @@ -571,6 +581,156 @@ static void test_rename_many_files(void) force_remove(dir); } +/* ========== readdir offset after delete ========== */ + +/* + * Reproduce the rm -rf bug: read entries with getdents64 using a tiny buffer + * (1 entry per call), delete each entry immediately, then continue reading. + * On correct kernel (byte offsets), the fd position advances past the deleted + * entry and lands on the next one. On buggy kernel (logical indices), the + * index shifts after deletion and entries get skipped. + */ +static void test_readdir_offset_after_delete(void) +{ + char dir[256], path[256], content[64]; + + snprintf(dir, sizeof(dir), "%s/readdir_del", BASE); + CHECK(mkdir(dir, 0755) == 0, "mkdir readdir_del"); + + /* Create 30 files */ + int ok = 1; + for (int i = 0; i < 30; i++) { + snprintf(path, sizeof(path), "%s/f%02d", dir, i); + snprintf(content, sizeof(content), "%d\n", i); + if (write_file(path, content) < 0) + ok = 0; + } + CHECK(ok, "created 30 files"); + + /* + * Read-delete loop: tiny buffer forces 1 entry per getdents64 call. + * After reading each entry, delete it immediately. If offset tracking + * is correct, we'll see all 30 entries. If buggy, entries get skipped. + */ + int dfd = open(dir, O_RDONLY | O_DIRECTORY); + CHECK(dfd >= 0, "open dir for read-delete loop"); + + char buf[32]; /* minimum buffer: exactly 1 entry */ + int total_deleted = 0; + int rounds = 0; + + for (;;) { + int nread = syscall(SYS_getdents64, dfd, buf, sizeof(buf)); + if (nread <= 0) + break; + + int pos = 0; + while (pos < nread) { + struct linux_dirent64 *ent = + (struct linux_dirent64 *)(buf + pos); + if (strcmp(ent->d_name, ".") != 0 && + strcmp(ent->d_name, "..") != 0) { + snprintf(path, sizeof(path), "%s/%s", dir, ent->d_name); + if (unlink(path) == 0) + total_deleted++; + } + pos += ent->d_reclen; + } + rounds++; + } + close(dfd); + + CHECK(total_deleted == 30, + "read-delete loop: all 30 files deleted"); + CHECK(rounds > 1, + "read-delete loop: needed multiple getdents calls (bug trigger)"); + + /* If bug exists, some files were skipped and rmdir fails */ + int rmdir_ret = rmdir(dir); + CHECK_RET(rmdir_ret, 0, + "rmdir after read-delete loop succeeds (no skipped entries)"); + if (rmdir_ret != 0) + force_remove(dir); +} + +/* + * Variant: read-readdir-delete-readdir pattern (simulates rm -rf). + * Read all entries via getdents64, delete each batch before reading next. + */ +static void test_rm_rf_pattern(void) +{ + char dir[256], path[256], content[64]; + + snprintf(dir, sizeof(dir), "%s/rmrf", BASE); + CHECK(mkdir(dir, 0755) == 0, "mkdir rmrf"); + + /* Create 30 files */ + for (int i = 0; i < 30; i++) { + snprintf(path, sizeof(path), "%s/f%02d", dir, i); + snprintf(content, sizeof(content), "%d\n", i); + write_file(path, content); + } + + /* + * Simulate rm -rf: open dir, read small batch, delete those files, + * repeat until empty. If offset is logical and entries are deleted + * between getdents calls, files get skipped and rmdir fails. + */ + int dfd = open(dir, O_RDONLY | O_DIRECTORY); + CHECK(dfd >= 0, "open dir for rm-rf pattern"); + + char buf[80]; + int total_deleted = 0; + int rounds = 0; + + for (;;) { + /* Read one batch */ + int nread = syscall(SYS_getdents64, dfd, buf, sizeof(buf)); + if (nread <= 0) + break; + + /* Collect names from this batch */ + char names[4][32]; + int name_count = 0; + int pos = 0; + while (pos < nread && name_count < 4) { + struct linux_dirent64 *ent = + (struct linux_dirent64 *)(buf + pos); + if (strcmp(ent->d_name, ".") != 0 && + strcmp(ent->d_name, "..") != 0) { + strncpy(names[name_count], ent->d_name, 31); + names[name_count][31] = '\0'; + name_count++; + } + pos += ent->d_reclen; + } + + /* Delete this batch */ + for (int i = 0; i < name_count; i++) { + snprintf(path, sizeof(path), "%s/%s", dir, names[i]); + if (unlink(path) == 0) + total_deleted++; + } + rounds++; + } + close(dfd); + + CHECK(total_deleted == 30, + "rm-rf pattern: all 30 files deleted"); + CHECK(rounds > 1, + "rm-rf pattern: needed multiple getdents calls"); + + /* + * If the bug exists, some files were skipped by getdents and remain. + * rmdir should succeed only if all files were deleted. + */ + int rmdir_ret = rmdir(dir); + CHECK_RET(rmdir_ret, 0, + "rmdir after rm-rf pattern succeeds (no leftover files)"); + if (rmdir_ret != 0) + force_remove(dir); +} + /* ========== main ========== */ int main(void) @@ -611,6 +771,10 @@ int main(void) test_pip_upgrade_pattern(); test_rename_many_files(); + /* readdir offset correctness after deletion (rm -rf bug) */ + test_readdir_offset_after_delete(); + test_rm_rf_pattern(); + manual_rmdir_recursive(BASE); TEST_DONE(); From 8b364bfb5ce380135520aaed8762e050eaa65956 Mon Sep 17 00:00:00 2001 From: zyc107109102 Date: Wed, 27 May 2026 17:52:49 +0000 Subject: [PATCH 2/4] test ci --- .../normal/qemu-smp1/bugfix/bug-ext4-dir-ops/c/src/main.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test-suit/starryos/normal/qemu-smp1/bugfix/bug-ext4-dir-ops/c/src/main.c b/test-suit/starryos/normal/qemu-smp1/bugfix/bug-ext4-dir-ops/c/src/main.c index 480a3a505f..d821251504 100644 --- a/test-suit/starryos/normal/qemu-smp1/bugfix/bug-ext4-dir-ops/c/src/main.c +++ b/test-suit/starryos/normal/qemu-smp1/bugfix/bug-ext4-dir-ops/c/src/main.c @@ -1,6 +1,6 @@ /*! * bug-ext4-dir-ops.c - * + *! * Verifies POSIX rmdir() and rename() semantics on ext4 (rsext4), * plus VFS dentry cache correctness after rename (stale parent bug). * From 46f2c0ab0fd25cf5b442c75252ca90a151b3c88e Mon Sep 17 00:00:00 2001 From: zyc107109102 Date: Wed, 27 May 2026 18:02:43 +0000 Subject: [PATCH 3/4] test ci --- .../normal/qemu-smp1/bugfix/bug-ext4-dir-ops/c/src/main.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test-suit/starryos/normal/qemu-smp1/bugfix/bug-ext4-dir-ops/c/src/main.c b/test-suit/starryos/normal/qemu-smp1/bugfix/bug-ext4-dir-ops/c/src/main.c index d821251504..480a3a505f 100644 --- a/test-suit/starryos/normal/qemu-smp1/bugfix/bug-ext4-dir-ops/c/src/main.c +++ b/test-suit/starryos/normal/qemu-smp1/bugfix/bug-ext4-dir-ops/c/src/main.c @@ -1,6 +1,6 @@ /*! * bug-ext4-dir-ops.c - *! + * * Verifies POSIX rmdir() and rename() semantics on ext4 (rsext4), * plus VFS dentry cache correctness after rename (stale parent bug). * From 7e4bcdb96b23693c8d9bbb01ab35cbe8a43df7b8 Mon Sep 17 00:00:00 2001 From: zyc107109102 Date: Wed, 27 May 2026 18:17:43 +0000 Subject: [PATCH 4/4] test ci --- .../normal/qemu-smp1/bugfix/bug-ext4-dir-ops/c/src/main.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test-suit/starryos/normal/qemu-smp1/bugfix/bug-ext4-dir-ops/c/src/main.c b/test-suit/starryos/normal/qemu-smp1/bugfix/bug-ext4-dir-ops/c/src/main.c index 480a3a505f..dd96ec213d 100644 --- a/test-suit/starryos/normal/qemu-smp1/bugfix/bug-ext4-dir-ops/c/src/main.c +++ b/test-suit/starryos/normal/qemu-smp1/bugfix/bug-ext4-dir-ops/c/src/main.c @@ -1,6 +1,6 @@ /*! * bug-ext4-dir-ops.c - * + *！ * Verifies POSIX rmdir() and rename() semantics on ext4 (rsext4), * plus VFS dentry cache correctness after rename (stale parent bug). *