fix(starry-kernel): validate sync_file_range flags and offsets

os/StarryOS/kernel/src/syscall/fs/io.rs

@@ -317,19 +317,30 @@ pub fn sys_fdatasync(fd: c_int) -> AxResult<isize> {
     Err(AxError::from(LinuxError::EINVAL))
 }
 
-pub fn sys_sync_file_range(fd: c_int, _offset: i64, _nbytes: i64, flags: u32) -> AxResult<isize> {
+pub fn sys_sync_file_range(fd: c_int, offset: i64, nbytes: i64, flags: u32) -> AxResult<isize> {
     debug!("sys_sync_file_range <= fd: {fd}, flags: {flags:#x}");
+    const SYNC_FILE_RANGE_WAIT_BEFORE: u32 = 1;
+    const SYNC_FILE_RANGE_WRITE: u32 = 2;
+    const SYNC_FILE_RANGE_WAIT_AFTER: u32 = 4;
+    const SYNC_FILE_RANGE_ALL: u32 =
+        SYNC_FILE_RANGE_WAIT_BEFORE | SYNC_FILE_RANGE_WRITE | SYNC_FILE_RANGE_WAIT_AFTER;
+    if offset < 0 || nbytes < 0 {
+        return Err(AxError::from(LinuxError::EINVAL));
+    }
+    if (flags & !SYNC_FILE_RANGE_ALL) != 0 {
+        return Err(AxError::from(LinuxError::EINVAL));
+    }
     // sync_file_range(2) is an advisory hint to initiate writeback for a
     // byte range. Until range-based writeback is implemented, keep this as
     // a no-op after basic fd validation rather than turning it into a
     // stronger whole-file fdatasync-style flush (matches the advisory
     // nature documented in the man page). Invalid fds still surface the
     // underlying error (EBADF). Directory fds are accepted to match fsync.
     let any = get_file_like(fd)?;
-    if flags & !0x7 != 0 {
-        return Err(AxError::from(LinuxError::EINVAL));
-    }
-    if any.downcast_ref::<File>().is_none() && any.downcast_ref::<Directory>().is_none() {
+    if any.downcast_ref::<File>().is_none()
+        && any.downcast_ref::<Directory>().is_none()
+        && any.downcast_ref::<Memfd>().is_none()
+    {
         return Err(AxError::from(LinuxError::ESPIPE));
     }
     Ok(0)

os/StarryOS/kernel/src/syscall/ipc/shm.rs

@@ -497,15 +497,20 @@ pub fn sys_shmat(shmid: i32, addr: usize, shmflg: u32) -> AxResult<isize> {
     let proc_data = &curr.as_thread().proc_data;
     let pid = proc_data.proc.pid();
 
-    // Lock SHM_MANAGER first, then shm_inner, then aspace. This matches
-    // the ordering used by sys_shmget and avoids the AB/BA deadlock that
-    // the old code caused by locking shm_inner before SHM_MANAGER.
-    let mut shm_manager = SHM_MANAGER.lock();
-    let shm_inner_arc = shm_manager
-        .get_inner_by_shmid(shmid)
-        .ok_or(AxError::InvalidInput)?;
+    info!("shmat pid={pid} shmid={shmid} enter");
+
+    // Grab the shm_inner Arc under SHM_MANAGER, then drop it before
+    // mapping work to avoid holding the global lock across aspace ops.
+    let shm_inner_arc = {
+        let shm_manager = SHM_MANAGER.lock();
+        shm_manager
+            .get_inner_by_shmid(shmid)
+            .ok_or(AxError::InvalidInput)?
+    };
+    info!("shmat pid={pid} shmid={shmid} lock shm_inner");
     let mut shm_inner = shm_inner_arc.lock();
     let aspace_arc = proc_data.aspace();
+    info!("shmat pid={pid} shmid={shmid} lock aspace");
     let mut aspace = aspace_arc.lock();
 
     let mut mapping_flags = shm_inner.mapping_flags;
@@ -541,7 +546,6 @@ pub fn sys_shmat(shmid: i32, addr: usize, shmflg: u32) -> AxResult<isize> {
     let end_addr = VirtAddr::from(start_addr.as_usize() + length);
     let va_range = VirtAddrRange::new(start_addr, end_addr);
 
-    shm_manager.insert_shmid_vaddr(pid, shm_inner.shmid, start_addr);
     info!(
         "Process {} alloc shm virt addr start: {:#x}, size: {}, mapping_flags: {:#x?}",
         pid,
@@ -565,7 +569,15 @@ pub fn sys_shmat(shmid: i32, addr: usize, shmflg: u32) -> AxResult<isize> {
         shm_inner.map_to_phys(pages);
     }
 
+    info!("shmat pid={pid} shmid={shmid} mapped; attach_process");
     shm_inner.attach_process(pid, va_range)?;
+    drop(aspace);
+    drop(shm_inner);
+
+    info!("shmat pid={pid} shmid={shmid} lock shm_manager for vaddr");
+    let mut shm_manager = SHM_MANAGER.lock();
+    shm_manager.insert_shmid_vaddr(pid, shmid, start_addr);
+    info!("shmat pid={pid} shmid={shmid} done");
     Ok(start_addr.as_usize() as isize)
 }
 
@@ -641,6 +653,8 @@ pub fn sys_shmdt(shmaddr: usize) -> AxResult<isize> {
     let proc_data = &curr.as_thread().proc_data;
     let pid = proc_data.proc.pid();
 
+    info!("shmdt pid={pid} addr={shmaddr:?} enter");
+
     // Look up shmid and grab the inner Arc while holding SHM_MANAGER.
     let (shmid, shm_inner_arc) = {
         let shm_manager = SHM_MANAGER.lock();
@@ -655,19 +669,22 @@ pub fn sys_shmdt(shmaddr: usize) -> AxResult<isize> {
 
     // Snapshot the mapped range for this process.
     let va_range = {
+        info!("shmdt pid={pid} lock shm_inner for range");
         let shm_inner = shm_inner_arc.lock();
         shm_inner.get_addr_range(pid).ok_or(AxError::InvalidInput)?
     };
 
     // Unmap while only holding the aspace lock.
     {
+        info!("shmdt pid={pid} lock aspace for unmap");
         let aspace_arc = proc_data.aspace();
         let mut aspace = aspace_arc.lock();
         aspace.unmap(va_range.start, va_range.size())?;
     }
 
     // Reacquire SHM_MANAGER then shm_inner for bookkeeping, matching
     // the global lock ordering.
+    info!("shmdt pid={pid} lock shm_manager for bookkeeping");
     let mut shm_manager = SHM_MANAGER.lock();
     shm_manager.remove_shmaddr(pid, shmaddr);
     let mut shm_inner = shm_inner_arc.lock();

test-suit/starryos/normal/qemu-smp1/syscall/test-fsync-dir/c/src/main.c

@@ -4,14 +4,19 @@
  * 测试文件系统修复:
  * 1. fsync 对目录 fd 应返回成功 (Linux 允许)
  * 2. fdatasync 对目录 fd 应返回成功
- * 3. sync_file_range 应返回成功 (建议性优化)
+ * 3. 非法 fd 应返回 EBADF
+ * 4. pipe fd 应返回 EINVAL
+ * 5. socket fd 应返回 EINVAL
+ * 6. sync_file_range 应返回成功 (建议性优化)
  */
 
 #include "test_framework.h"
 #include <fcntl.h>
 #include <unistd.h>
-#include <sys/syscall.h>
+#include <sys/socket.h>
 #include <sys/stat.h>
+#include <sys/syscall.h>
+#include <sys/types.h>
 
 int main(void)
 {
@@ -40,7 +45,41 @@ int main(void)
         unlink("/tmp/starry_fsync_test/file");
     }
 
-    /* Test 3: sync_file_range */
+    /* Test 3: invalid fd handling */
+    {
+        CHECK_ERR(fsync(-1), EBADF, "fsync on invalid fd -> EBADF");
+        CHECK_ERR(fdatasync(-1), EBADF, "fdatasync on invalid fd -> EBADF");
+    }
+
+    /* Test 4: fsync on pipe -> EINVAL (Linux behavior) */
+    {
+        int pfd[2];
+        CHECK(pipe(pfd) == 0, "create pipe for fsync EINVAL");
+        if (pfd[0] >= 0) {
+            CHECK_ERR(fsync(pfd[0]), EINVAL, "fsync on pipe -> EINVAL");
+            CHECK_ERR(fdatasync(pfd[0]), EINVAL, "fdatasync on pipe -> EINVAL");
+            close(pfd[0]);
+        }
+        if (pfd[1] >= 0) {
+            close(pfd[1]);
+        }
+    }
+
+    /* Test 5: fsync on socket -> EINVAL (Linux behavior) */
+    {
+        int sfd[2];
+        CHECK(socketpair(AF_UNIX, SOCK_STREAM, 0, sfd) == 0, "create socketpair for fsync EINVAL");
+        if (sfd[0] >= 0) {
+            CHECK_ERR(fsync(sfd[0]), EINVAL, "fsync on socket -> EINVAL");
+            CHECK_ERR(fdatasync(sfd[0]), EINVAL, "fdatasync on socket -> EINVAL");
+            close(sfd[0]);
+        }
+        if (sfd[1] >= 0) {
+            close(sfd[1]);
+        }
+    }
+
+    /* Test 6: sync_file_range */
     {
         int fd = open("/tmp/starry_fsync_test/sfrfile", O_RDWR | O_CREAT | O_TRUNC, 0644);
         CHECK(fd >= 0, "create file for sync_file_range");
@@ -50,6 +89,14 @@ int main(void)
          * SYNC_FILE_RANGE_WRITE = 2 */
         long rc = syscall(SYS_sync_file_range, fd, 0, 29, 2);
         CHECK(rc == 0, "sync_file_range returns 0");
+        CHECK_RET(syscall(SYS_sync_file_range, fd, 0, 29, 0), 0,
+              "sync_file_range flags==0 -> success");
+        CHECK_ERR(syscall(SYS_sync_file_range, fd, 0, 29, 0x8000), EINVAL,
+                  "sync_file_range invalid flags -> EINVAL");
+        CHECK_ERR(syscall(SYS_sync_file_range, fd, (off_t)-1, 29, 2), EINVAL,
+                  "sync_file_range negative offset -> EINVAL");
+        CHECK_ERR(syscall(SYS_sync_file_range, fd, 0, (off_t)-1, 2), EINVAL,
+              "sync_file_range negative nbytes -> EINVAL");
         close(fd);
         unlink("/tmp/starry_fsync_test/sfrfile");
     }

test-suit/starryos/normal/qemu-smp4/test-shm-deadlock/c/src/main.c

@@ -20,6 +20,7 @@
 #include <sys/shm.h>
 #include <sys/mman.h>
 #include <sys/wait.h>
+#include <signal.h>
 #include <sched.h>
 #include <unistd.h>
 
@@ -28,6 +29,7 @@ static volatile int g_running = 1;
 static volatile int g_shmid = -1;
 static volatile int g_deadlock_detected = 0;
 static volatile int g_shmat_started = 0;
+static volatile int g_threads_done = 0;
 
 /* Thread stack size */
 #define STACK_SIZE (64 * 1024)
@@ -90,7 +92,7 @@ static int watchdog_thread(void *arg) {
 
     for (int i = 0; i < SHM_ALARM_SEC * 10; i++) {
         usleep(100000);
-        if (!g_running) {
+        if (g_threads_done) {
             return 0;
         }
     }
@@ -159,6 +161,12 @@ int main(void)
                     int status;
                     waitpid(tid1, &status, __WALL);
                     waitpid(tid2, &status, __WALL);
+                    g_threads_done = 1;
+                    /*
+                     * Ensure watchdog exits even if CLONE_VM isn't honored
+                     * on this platform; avoids false timeouts.
+                     */
+                    kill(tid3, SIGKILL);
                     waitpid(tid3, &status, __WALL);
 
                     CHECK(!g_deadlock_detected, "no deadlock detected");