From: Tulio A M Mendes Date: Wed, 20 May 2026 15:52:55 +0000 (-0300) Subject: mount/VFS: make diskfs non-destructive; fix error propagation and syscall safety X-Git-Url: https://projects.tadryanom.me/?a=commitdiff_plain;h=be11f06217a68dc8d4005f4b1a4a99b90dadd3cb;p=AdrOS.git mount/VFS: make diskfs non-destructive; fix error propagation and syscall safety Patch 1 (CRITICAL): diskfs_super_load now returns -ENODEV on bad magic and -EINVAL on unknown version instead of auto-formatting. Added diskfs_format() as explicit separate function. diskfs_create_root preserves auto-format for blank disks (boot/battery test), but autodetect probing uses new diskfs_probe() which is non-destructive. Patch 2: Autodetect order changed from {diskfs,fat,ext2} to {ext2,fat,diskfs}. diskfs_probe() checked before diskfs_create_root in autodetect path, preventing corruption of FAT/ext2 disks. Patch 3: init_mount_fs returns negative errno (-EINVAL, -ENODEV) instead of -1. sys_mount passes through rc directly instead of converting all errors to -EIO. Patch 4: fulltest sys_mount wrapper now passes flags=0 as 4th argument via esi register, preventing garbage MS_REMOUNT bits. Patch 5: vfs_mount_nolock_full reordered to check existing entry (remount) before table-full check, preventing -ENOSPC on remount. All strncpy calls now have explicit NUL-termination. Patch 6: mount userland requires -t fstype (no more diskfs default), warns on unknown -o options. Patch 7: Added copy_user_cstr() helper for byte-by-byte C-string copy from userspace with -ENAMETOOLONG on overflow. Replaces fixed-size copy_from_user for mount syscall strings. --- diff --git a/include/diskfs.h b/include/diskfs.h index 70aab738..fadc5105 100644 --- a/include/diskfs.h +++ b/include/diskfs.h @@ -15,6 +15,10 @@ fs_node_t* diskfs_create_root(int drive); +// Non-destructive probe: returns 0 if drive has valid diskfs superblock, +// -ENODEV if not diskfs, or other negative errno on I/O error. +int diskfs_probe(int drive); + // Open (and optionally create) a diskfs file at the root (flat namespace). // rel_path must not contain '/'. // flags: supports O_CREAT (0x40) and O_TRUNC (0x200) semantics (minimal). diff --git a/src/kernel/diskfs.c b/src/kernel/diskfs.c index 82b0f3e5..c22f8e64 100644 --- a/src/kernel/diskfs.c +++ b/src/kernel/diskfs.c @@ -111,6 +111,24 @@ static void diskfs_close_impl(fs_node_t* node) { kfree(dn); } +static int diskfs_format(struct diskfs_super* sb) { + /* Initialize a fresh diskfs superblock and write it to disk. + * This is the ONLY function that writes a new superblock. + * Callers must explicitly request formatting — probe/mount must NOT + * call this automatically on unknown disks. */ + memset(sb, 0, sizeof(*sb)); + sb->magic = DISKFS_MAGIC; + sb->version = DISKFS_VERSION; + sb->next_free_lba = DISKFS_LBA_DATA_START; + + // Root inode + sb->inodes[0].type = DISKFS_INODE_DIR; + sb->inodes[0].parent = 0; + sb->inodes[0].name[0] = 0; + + return diskfs_super_store(sb); +} + static int diskfs_super_load(struct diskfs_super* sb) { if (!sb) return -EINVAL; uint8_t sec0[DISKFS_SECTOR]; @@ -125,17 +143,8 @@ static int diskfs_super_load(struct diskfs_super* sb) { } if (sb->magic != DISKFS_MAGIC) { - memset(sb, 0, sizeof(*sb)); - sb->magic = DISKFS_MAGIC; - sb->version = DISKFS_VERSION; - sb->next_free_lba = DISKFS_LBA_DATA_START; - - // Root inode - sb->inodes[0].type = DISKFS_INODE_DIR; - sb->inodes[0].parent = 0; - sb->inodes[0].name[0] = 0; - - return diskfs_super_store(sb); + /* Not a diskfs filesystem — do NOT auto-format */ + return -ENODEV; } if (sb->version == DISKFS_VERSION) { @@ -188,15 +197,8 @@ static int diskfs_super_load(struct diskfs_super* sb) { return diskfs_super_store(sb); } - // Unknown version -> re-init (best-effort) - memset(sb, 0, sizeof(*sb)); - sb->magic = DISKFS_MAGIC; - sb->version = DISKFS_VERSION; - sb->next_free_lba = DISKFS_LBA_DATA_START; - sb->inodes[0].type = DISKFS_INODE_DIR; - sb->inodes[0].parent = 0; - sb->inodes[0].name[0] = 0; - return diskfs_super_store(sb); + /* Unknown version — do NOT reformat */ + return -EINVAL; } static int diskfs_super_store(const struct diskfs_super* sb) { @@ -1183,9 +1185,32 @@ fs_node_t* diskfs_create_root(int drive) { if (g_ready) { struct diskfs_super sb; - (void)diskfs_super_load(&sb); + int rc = diskfs_super_load(&sb); + if (rc == -ENODEV) { + /* No diskfs superblock found — format a fresh one. + * This preserves the auto-format behavior for boot-time + * mounting of blank disks (e.g. battery test root_hda.img). + * Autodetect probing will check diskfs_probe() first and + * skip diskfs if it returns -ENODEV, so this path is only + * reached when the caller explicitly requests diskfs. */ + (void)diskfs_format(&sb); + } } } return g_ready ? &g_root.vfs : NULL; } + +int diskfs_probe(int drive) { + /* Non-destructive probe: check if the drive contains a valid diskfs + * superblock without formatting. Returns 0 if valid, -ENODEV if + * not a diskfs disk, or other negative errno on I/O error. + * Does NOT modify global state. */ + if (!ata_pio_drive_present(drive)) return -ENODEV; + uint8_t sec0[DISKFS_SECTOR]; + if (ata_pio_read28(drive, DISKFS_LBA_SUPER, sec0) < 0) return -EIO; + uint32_t magic; + memcpy(&magic, sec0, sizeof(magic)); + if (magic != DISKFS_MAGIC) return -ENODEV; + return 0; +} diff --git a/src/kernel/fs.c b/src/kernel/fs.c index 7286ad7e..e80ab076 100644 --- a/src/kernel/fs.c +++ b/src/kernel/fs.c @@ -74,8 +74,6 @@ static void normalize_mountpoint(const char* in, char* out, size_t out_sz) { int vfs_mount_nolock_full(const char* mountpoint, fs_node_t* root, const char* fstype, const char* source, unsigned long flags) { - if (g_mount_count >= (int)(sizeof(g_mounts) / sizeof(g_mounts[0]))) return -ENOSPC; - char mp[128]; normalize_mountpoint(mountpoint, mp, sizeof(mp)); @@ -83,23 +81,38 @@ int vfs_mount_nolock_full(const char* mountpoint, fs_node_t* root, if (strcmp(g_mounts[i].mountpoint, mp) == 0) { /* Remount: update provided fields only */ if (root) g_mounts[i].root = root; - if (fstype) strncpy(g_mounts[i].fstype, fstype, sizeof(g_mounts[i].fstype) - 1); - if (source) strncpy(g_mounts[i].source, source, sizeof(g_mounts[i].source) - 1); + if (fstype) { + strncpy(g_mounts[i].fstype, fstype, sizeof(g_mounts[i].fstype) - 1); + g_mounts[i].fstype[sizeof(g_mounts[i].fstype) - 1] = '\0'; + } + if (source) { + strncpy(g_mounts[i].source, source, sizeof(g_mounts[i].source) - 1); + g_mounts[i].source[sizeof(g_mounts[i].source) - 1] = '\0'; + } /* Always update flags on remount (even if 0, caller explicitly set them) */ g_mounts[i].flags = flags; return 0; } } - /* New mount entry — root is required */ + /* New mount entry — check table space first, then require root */ + if (g_mount_count >= (int)(sizeof(g_mounts) / sizeof(g_mounts[0]))) return -ENOSPC; if (!root) return -EINVAL; strcpy(g_mounts[g_mount_count].mountpoint, mp); g_mounts[g_mount_count].root = root; - if (fstype) strncpy(g_mounts[g_mount_count].fstype, fstype, sizeof(g_mounts[g_mount_count].fstype) - 1); - else g_mounts[g_mount_count].fstype[0] = '\0'; - if (source) strncpy(g_mounts[g_mount_count].source, source, sizeof(g_mounts[g_mount_count].source) - 1); - else g_mounts[g_mount_count].source[0] = '\0'; + if (fstype) { + strncpy(g_mounts[g_mount_count].fstype, fstype, sizeof(g_mounts[g_mount_count].fstype) - 1); + g_mounts[g_mount_count].fstype[sizeof(g_mounts[g_mount_count].fstype) - 1] = '\0'; + } else { + g_mounts[g_mount_count].fstype[0] = '\0'; + } + if (source) { + strncpy(g_mounts[g_mount_count].source, source, sizeof(g_mounts[g_mount_count].source) - 1); + g_mounts[g_mount_count].source[sizeof(g_mounts[g_mount_count].source) - 1] = '\0'; + } else { + g_mounts[g_mount_count].source[0] = '\0'; + } g_mounts[g_mount_count].flags = flags; g_mount_count++; return 0; diff --git a/src/kernel/init.c b/src/kernel/init.c index 7725d59e..3faf40bd 100644 --- a/src/kernel/init.c +++ b/src/kernel/init.c @@ -57,14 +57,14 @@ int init_mount_fs(const char* fstype, int drive, uint32_t lba, const char* mount root = persistfs_create_root(drive); } else { kprintf("[MOUNT] Unknown filesystem type: %s\n", fstype); - return -1; + return -EINVAL; } if (!root) { kprintf("[MOUNT] Failed to mount %s on /dev/%s at %s\n", fstype, ata_drive_to_name(drive) ? ata_drive_to_name(drive) : "?", mountpoint); - return -1; + return -ENODEV; } /* Build device name for mount table metadata */ @@ -79,9 +79,10 @@ int init_mount_fs(const char* fstype, int drive, uint32_t lba, const char* mount *dp = '\0'; } - if (vfs_mount_full(mountpoint, root, fstype, devname, 0) < 0) { - kprintf("[MOUNT] Failed to register mount at %s\n", mountpoint); - return -1; + int rc = vfs_mount_full(mountpoint, root, fstype, devname, 0); + if (rc < 0) { + kprintf("[MOUNT] Failed to register mount at %s (err=%d)\n", mountpoint, rc); + return rc; } kprintf("[MOUNT] %s on /dev/%s -> %s\n", @@ -206,10 +207,18 @@ int init_start(const struct boot_info* bi) { if (strncmp(root_dev, "/dev/", 5) == 0) drive = ata_name_to_drive(root_dev + 5); if (drive >= 0 && ata_pio_drive_present(drive)) { - /* Try auto-detect: diskfs, fat, ext2 */ - static const char* fstypes[] = { "diskfs", "fat", "ext2", NULL }; + /* Auto-detect: try ext2, fat first (non-destructive), then diskfs. + * diskfs_probe() is non-destructive — it only checks the magic + * without formatting. diskfs_create_root() will auto-format + * blank disks, so it is only called when probe confirms diskfs. */ + static const char* fstypes[] = { "ext2", "fat", "diskfs", NULL }; int mounted = 0; for (int i = 0; fstypes[i]; i++) { + if (strcmp(fstypes[i], "diskfs") == 0) { + /* Non-destructive probe first — skip if not diskfs */ + extern int diskfs_probe(int drive); + if (diskfs_probe(drive) != 0) continue; + } if (init_mount_fs(fstypes[i], drive, 0, "/disk") == 0) { kprintf("[INIT] root=%s mounted as %s on /disk\n", root_dev, fstypes[i]); @@ -224,8 +233,12 @@ int init_start(const struct boot_info* bi) { } } else if (ata_pio_drive_present(0)) { /* No root= on cmdline, but primary master is present — auto-mount */ - static const char* fstypes[] = { "diskfs", "fat", "ext2", NULL }; + static const char* fstypes[] = { "ext2", "fat", "diskfs", NULL }; for (int i = 0; fstypes[i]; i++) { + if (strcmp(fstypes[i], "diskfs") == 0) { + extern int diskfs_probe(int drive); + if (diskfs_probe(0) != 0) continue; + } if (init_mount_fs(fstypes[i], 0, 0, "/disk") == 0) { kprintf("[INIT] /dev/hda auto-mounted as %s on /disk\n", fstypes[i]); break; diff --git a/src/kernel/syscall.c b/src/kernel/syscall.c index 1571e089..428ff52e 100644 --- a/src/kernel/syscall.c +++ b/src/kernel/syscall.c @@ -934,6 +934,23 @@ static int syscall_select_impl(uint32_t nfds, return ready; } +static int copy_user_cstr(char* dst, const char* src, size_t dst_sz) { + /* Copy a user-space C string byte-by-byte until NUL terminator. + * Returns 0 on success, -EFAULT on bad address, -ENAMETOOLONG if + * the string (including NUL) exceeds dst_sz. */ + if (!dst || dst_sz == 0) return -EINVAL; + if (!src) { dst[0] = '\0'; return 0; } + for (size_t i = 0; i < dst_sz; i++) { + uint8_t byte; + if (copy_from_user(&byte, src + i, 1) < 0) return -EFAULT; + dst[i] = (char)byte; + if (byte == 0) return 0; + } + /* String too long — NUL-terminate and report error */ + dst[dst_sz - 1] = '\0'; + return -ENAMETOOLONG; +} + static int execve_copy_user_str(char* out, size_t out_sz, const char* user_s) { if (!out || out_sz == 0 || !user_s) return -EFAULT; /* Check full range upfront to avoid 128 individual copy_from_user calls */ @@ -4913,14 +4930,13 @@ static void socket_syscall_dispatch(struct registers* regs, uint32_t syscall_no) const char* user_type = (const char*)sc_arg2(regs); unsigned long mount_flags = (unsigned long)sc_arg3(regs); char kdev[64], kmp[128], ktype[32]; - if (copy_from_user(kdev, user_dev, sizeof(kdev)) < 0 || - path_resolve_user(user_mp, kmp, sizeof(kmp)) < 0 || - copy_from_user(ktype, user_type, sizeof(ktype)) < 0) { - sc_ret(regs) = (uint32_t)-EFAULT; - return; - } - kdev[sizeof(kdev)-1] = '\0'; - ktype[sizeof(ktype)-1] = '\0'; + + int crc; + crc = copy_user_cstr(kdev, user_dev, sizeof(kdev)); + if (crc < 0) { sc_ret(regs) = (uint32_t)crc; return; } + if (path_resolve_user(user_mp, kmp, sizeof(kmp)) < 0) { sc_ret(regs) = (uint32_t)-EFAULT; return; } + crc = copy_user_cstr(ktype, user_type, sizeof(ktype)); + if (crc < 0) { sc_ret(regs) = (uint32_t)crc; return; } /* MS_REMOUNT (0x20): update flags on existing mount */ if (mount_flags & 0x20 /* MS_REMOUNT */) { @@ -4963,7 +4979,7 @@ static void socket_syscall_dispatch(struct registers* regs, uint32_t syscall_no) extern int init_mount_fs(const char* fstype, int drive, uint32_t lba, const char* mountpoint); (void)vfs_mkdirp(kmp); /* auto-create mountpoint (recursive) */ int rc = init_mount_fs(ktype, drive, 0, kmp); - sc_ret(regs) = (uint32_t)(rc < 0 ? -EIO : 0); + sc_ret(regs) = (uint32_t)(rc < 0 ? rc : 0); return; } diff --git a/user/cmds/fulltest/fulltest.c b/user/cmds/fulltest/fulltest.c index 0aa5c20f..d2b8d576 100644 --- a/user/cmds/fulltest/fulltest.c +++ b/user/cmds/fulltest/fulltest.c @@ -1610,9 +1610,9 @@ static int sys_sigqueue(int pid, int sig, int value) { return __syscall_fix(ret); } -static int sys_mount(const char* dev, const char* dir, const char* type) { +static int sys_mount(const char* dev, const char* dir, const char* type, unsigned long flags) { int ret; - __asm__ volatile("int $0x80" : "=a"(ret) : "a"(SYSCALL_MOUNT), "b"(dev), "c"(dir), "d"(type) : "memory"); + __asm__ volatile("int $0x80" : "=a"(ret) : "a"(SYSCALL_MOUNT), "b"(dev), "c"(dir), "d"(type), "S"(flags) : "memory"); return __syscall_fix(ret); } @@ -5184,7 +5184,7 @@ void _start(void) { { /* Create mount point directory first */ (void)sys_mkdir("/tmp/mnt_test"); - if (sys_mount("none", "/tmp/mnt_test", "tmpfs") < 0) { + if (sys_mount("none", "/tmp/mnt_test", "tmpfs", 0) < 0) { sys_write(1, "[test] mount tmpfs failed\n", (uint32_t)(sizeof("[test] mount tmpfs failed\n") - 1)); sys_exit(1); @@ -5357,7 +5357,7 @@ void _start(void) { (void)sys_mkdir("/tmp/pivot_new"); (void)sys_mkdir("/tmp/pivot_old"); - if (sys_mount("none", "/tmp/pivot_new", "tmpfs") < 0) { + if (sys_mount("none", "/tmp/pivot_new", "tmpfs", 0) < 0) { sys_write(1, "[test] pivot_root mount tmpfs failed\n", (uint32_t)(sizeof("[test] pivot_root mount tmpfs failed\n") - 1)); sys_exit(1); diff --git a/user/cmds/mount/mount.c b/user/cmds/mount/mount.c index 5b0d4d0a..c64a6761 100644 --- a/user/cmds/mount/mount.c +++ b/user/cmds/mount/mount.c @@ -34,7 +34,7 @@ int main(int argc, char** argv) { return 0; } - const char* fstype = "diskfs"; + const char* fstype = NULL; const char* device = NULL; const char* mountpoint = NULL; unsigned long mountflags = 0; @@ -58,8 +58,10 @@ int main(int argc, char** argv) { } else if (strncmp(o, "rw", 2) == 0 && (o[2] == ',' || o[2] == '\0')) { o += 2; } else { - /* Skip unknown option */ + /* Warn on unknown option */ + const char* start = o; while (*o && *o != ',') o++; + fprintf(stderr, "mount: unknown option: %.*s\n", (int)(o - start), start); } if (*o == ',') o++; } @@ -75,6 +77,11 @@ int main(int argc, char** argv) { return 1; } + if (!fstype) { + fprintf(stderr, "mount: filesystem type required (-t fstype)\n"); + return 1; + } + if (!device) device = "none"; int rc = mount(device, mountpoint, fstype, mountflags, NULL);