From bc4c6e1218d6903b07c32b11ea7d3c82c463686e Mon Sep 17 00:00:00 2001 From: Minep Date: Fri, 19 Aug 2022 20:14:02 +0100 Subject: [PATCH 1/1] fix: symlink resolve. refactor: add some invariants checks. chore: minor refactors and clean up. --- lunaix-os/hal/acpi/parser/madt_parser.c | 1 - lunaix-os/hal/ahci/ahci.c | 5 +- lunaix-os/kernel/fs/vfs.c | 100 ++++++++++++++---------- lunaix-os/libs/klibc/stdio/sprintf.c | 2 - 4 files changed, 61 insertions(+), 47 deletions(-) diff --git a/lunaix-os/hal/acpi/parser/madt_parser.c b/lunaix-os/hal/acpi/parser/madt_parser.c index 5e9aeb4..7510d40 100644 --- a/lunaix-os/hal/acpi/parser/madt_parser.c +++ b/lunaix-os/hal/acpi/parser/madt_parser.c @@ -11,7 +11,6 @@ madt_parse(acpi_madt_t* madt, acpi_context* toc) uintptr_t ics_end = (uintptr_t)madt + madt->header.length; // Cosidering only one IOAPIC present (max 24 pins) - // FIXME: use hash table instead toc->madt.irq_exception = (acpi_intso_t**)vcalloc(24, sizeof(acpi_intso_t*)); diff --git a/lunaix-os/hal/ahci/ahci.c b/lunaix-os/hal/ahci/ahci.c index 41ebc86..9a7073a 100644 --- a/lunaix-os/hal/ahci/ahci.c +++ b/lunaix-os/hal/ahci/ahci.c @@ -174,10 +174,11 @@ ahci_init() continue; } - kprintf(KINFO "sata%d: %s (%s)\n", + kprintf(KINFO "sata%d: %s, sector_size=%dB, sector=%d\n", i, port->device->model, - port->device->serial_num); + port->device->block_size, + (uint32_t)port->device->max_lba); block_mount_disk(port->device); } diff --git a/lunaix-os/kernel/fs/vfs.c b/lunaix-os/kernel/fs/vfs.c index b879185..c13b8d3 100644 --- a/lunaix-os/kernel/fs/vfs.c +++ b/lunaix-os/kernel/fs/vfs.c @@ -161,6 +161,8 @@ vfs_dcache_lookup(struct v_dnode* parent, struct hstr* str) void vfs_dcache_add(struct v_dnode* parent, struct v_dnode* dnode) { + assert(parent); + atomic_fetch_add(&dnode->ref_count, 1); dnode->parent = parent; llist_append(&parent->children, &dnode->siblings); @@ -172,6 +174,7 @@ vfs_dcache_add(struct v_dnode* parent, struct v_dnode* dnode) void vfs_dcache_remove(struct v_dnode* dnode) { + assert(dnode); assert(dnode->ref_count == 1); llist_delete(&dnode->siblings); @@ -184,21 +187,31 @@ vfs_dcache_remove(struct v_dnode* dnode) void vfs_dcache_rehash(struct v_dnode* new_parent, struct v_dnode* dnode) { + assert(new_parent); + hstr_rehash(&dnode->name, HSTR_FULL_HASH); vfs_dcache_remove(dnode); vfs_dcache_add(new_parent, dnode); } +#define VFS_SYMLINK_DEPTH 16 + int __vfs_walk(struct v_dnode* start, const char* path, struct v_dnode** dentry, struct hstr* component, - int walk_options) + int walk_options, + size_t depth, + char* fname_buffer) { int errno = 0; int i = 0, j = 0; + if (depth >= VFS_SYMLINK_DEPTH) { + return ENAMETOOLONG; + } + if (path[0] == PATH_DELIM || !start) { if ((walk_options & VFS_WALK_FSRELATIVE) && start) { start = start->super_block->root; @@ -212,10 +225,10 @@ __vfs_walk(struct v_dnode* start, } struct v_dnode* dnode; + struct v_inode* current_inode; struct v_dnode* current_level = start; - char name_content[VFS_NAME_MAXLEN]; - struct hstr name = HSTR(name_content, 0); + struct hstr name = HSTR(fname_buffer, 0); char current = path[i++], lookahead; while (current && current_level) { @@ -227,7 +240,7 @@ __vfs_walk(struct v_dnode* start, if (!VFS_VALID_CHAR(current)) { return EINVAL; } - name_content[j++] = current; + fname_buffer[j++] = current; if (lookahead) { goto cont; } @@ -238,9 +251,7 @@ __vfs_walk(struct v_dnode* start, goto cont; } - lock_dnode(current_level); - - name_content[j] = 0; + fname_buffer[j] = 0; name.len = j; hstr_rehash(&name, HSTR_FULL_HASH); @@ -248,12 +259,44 @@ __vfs_walk(struct v_dnode* start, if (component) { component->hash = name.hash; component->len = j; - strcpy(component->value, name_content); + strcpy(component->value, fname_buffer); } - unlock_dnode(current_level); break; } + current_inode = current_level->inode; + + if ((current_inode->itype & VFS_IFSYMLINK)) { + const char* link; + + lock_inode(current_inode); + if ((errno = + current_inode->ops->read_symlink(current_inode, &link))) { + unlock_inode(current_inode); + goto error; + } + unlock_inode(current_inode); + + errno = __vfs_walk(current_level->parent, + link, + &dnode, + NULL, + 0, + depth + 1, + fname_buffer + name.len + 1); + + if (errno) { + goto error; + } + + // reposition the resolved subtree pointed by symlink + vfs_dcache_rehash(current_level->parent, dnode); + current_level = dnode; + current_inode = dnode->inode; + } + + lock_dnode(current_level); + dnode = vfs_dcache_lookup(current_level, &name); if (!dnode) { @@ -264,8 +307,6 @@ __vfs_walk(struct v_dnode* start, goto error; } - struct v_inode* current_inode = current_level->inode; - lock_inode(current_inode); errno = current_inode->ops->dir_lookup(current_inode, dnode); @@ -305,8 +346,6 @@ error: return errno; } -#define VFS_MAX_SYMLINK 16 - int vfs_walk(struct v_dnode* start, const char* path, @@ -314,37 +353,14 @@ vfs_walk(struct v_dnode* start, struct hstr* component, int options) { - struct v_dnode* interim; - const char* pathname = path; - int errno = __vfs_walk(start, path, &interim, component, options); - int counter = 0; - - // FIXME This is NOT a correct way to resolve symlink! - while (!errno && interim->inode && (options & VFS_WALK_NOFOLLOW)) { - if (counter >= VFS_MAX_SYMLINK) { - errno = ELOOP; - continue; - } - if ((interim->inode->itype & VFS_IFSYMLINK) && - interim->inode->ops->read_symlink) { - - lock_inode(interim->inode); - errno = - interim->inode->ops->read_symlink(interim->inode, &pathname); - unlock_inode(interim->inode); - - if (errno) { - break; - } - } else { - break; - } - errno = __vfs_walk(start, pathname, &interim, component, options); - counter++; - } + // allocate a file name stack for path walking and recursion to resolve + // symlink + char* name_buffer = valloc(2048); - *dentry = errno ? 0 : interim; + int errno = + __vfs_walk(start, path, dentry, component, options, 0, name_buffer); + vfree(name_buffer); return errno; } diff --git a/lunaix-os/libs/klibc/stdio/sprintf.c b/lunaix-os/libs/klibc/stdio/sprintf.c index d357068..ade37bb 100644 --- a/lunaix-os/libs/klibc/stdio/sprintf.c +++ b/lunaix-os/libs/klibc/stdio/sprintf.c @@ -19,8 +19,6 @@ static const char flag_chars[] = "#0- +"; #define FLAG_ALT2 (1 << 8) #define FLAG_CAPS (1 << 9) -// FIXME: use something like IO_FILE to abstract this into a more flexible, -// stream based, vprintf size_t __sprintf_internal(char* buffer, char* fmt, size_t max_len, va_list vargs) { -- 2.27.0