From 3ba15b41a30e51aae595cbdf03b80bdf2e3794e3 Mon Sep 17 00:00:00 2001 From: Bananymous Date: Sat, 23 Sep 2023 02:43:02 +0300 Subject: [PATCH] Kernel/LibC: remove PATH resoltion from kernel I have no idea why I had made PATH environment variable parsing to be part of the kernel. Now the shell does the parsing and environment syscall is no longer needed. --- kernel/include/kernel/Process.h | 4 +-- kernel/kernel/Process.cpp | 61 +++------------------------------ kernel/kernel/Syscall.cpp | 3 -- libc/include/sys/syscall.h | 1 - libc/stdlib.cpp | 15 +++++--- libc/unistd.cpp | 17 ++++++++- userspace/Shell/main.cpp | 41 ++++++++++++++++++++-- userspace/init/main.cpp | 2 ++ 8 files changed, 73 insertions(+), 71 deletions(-) diff --git a/kernel/include/kernel/Process.h b/kernel/include/kernel/Process.h index 32bb2c183a..124c15ca0f 100644 --- a/kernel/include/kernel/Process.h +++ b/kernel/include/kernel/Process.h @@ -71,8 +71,6 @@ namespace Kernel BAN::ErrorOr sys_sleep(int seconds); BAN::ErrorOr sys_nanosleep(const timespec* rqtp, timespec* rmtp); - BAN::ErrorOr sys_setenvp(char** envp); - BAN::ErrorOr sys_setpwd(const char* path); BAN::ErrorOr sys_getpwd(char* buffer, size_t size); @@ -147,7 +145,7 @@ namespace Kernel static void register_process(Process*); // Load an elf file to virtual address space of the current page table - static BAN::ErrorOr> load_elf_for_exec(const Credentials&, BAN::StringView file_path, const BAN::String& cwd, const BAN::Vector& path_env); + static BAN::ErrorOr> load_elf_for_exec(const Credentials&, BAN::StringView file_path, const BAN::String& cwd); // Copy an elf file from the current page table to the processes own void load_elf_to_memory(LibELF::ELF&); diff --git a/kernel/kernel/Process.cpp b/kernel/kernel/Process.cpp index 13de5435eb..ad2c7c003d 100644 --- a/kernel/kernel/Process.cpp +++ b/kernel/kernel/Process.cpp @@ -99,7 +99,7 @@ namespace Kernel BAN::ErrorOr Process::create_userspace(const Credentials& credentials, BAN::StringView path) { - auto elf = TRY(load_elf_for_exec(credentials, path, "/"sv, {})); + auto elf = TRY(load_elf_for_exec(credentials, path, "/"sv)); auto* process = create_process(credentials, 0); MUST(process->m_working_directory.push_back('/')); @@ -114,7 +114,6 @@ namespace Kernel elf.clear(); char** argv = nullptr; - char** envp = nullptr; { PageTableScope _(process->page_table()); @@ -123,18 +122,11 @@ namespace Kernel memcpy(argv[0], path.data(), path.size()); argv[0][path.size()] = '\0'; argv[1] = nullptr; - - BAN::StringView env1 = "PATH=/bin:/usr/bin"sv; - envp = (char**)MUST(process->sys_alloc(sizeof(char**) * 2)); - envp[0] = (char*)MUST(process->sys_alloc(env1.size() + 1)); - memcpy(envp[0], env1.data(), env1.size()); - envp[0][env1.size()] = '\0'; - envp[1] = nullptr; } process->m_userspace_info.argc = 1; process->m_userspace_info.argv = argv; - process->m_userspace_info.envp = envp; + process->m_userspace_info.envp = nullptr; auto* thread = MUST(Thread::create_userspace(process)); process->add_thread(thread); @@ -275,7 +267,7 @@ namespace Kernel return 0; } - BAN::ErrorOr> Process::load_elf_for_exec(const Credentials& credentials, BAN::StringView file_path, const BAN::String& cwd, const BAN::Vector& path_env) + BAN::ErrorOr> Process::load_elf_for_exec(const Credentials& credentials, BAN::StringView file_path, const BAN::String& cwd) { if (file_path.empty()) return BAN::Error::from_errno(ENOENT); @@ -283,44 +275,13 @@ namespace Kernel BAN::String absolute_path; if (file_path.front() == '/') - { - // We have an absolute path TRY(absolute_path.append(file_path)); - } - else if (file_path.front() == '.' || file_path.contains('/')) + else { - // We have a relative path TRY(absolute_path.append(cwd)); TRY(absolute_path.push_back('/')); TRY(absolute_path.append(file_path)); } - else - { - // We have neither relative or absolute path, - // search from PATH environment - for (auto path_part : path_env) - { - if (path_part.empty()) - continue; - - if (path_part.front() != '/') - { - TRY(absolute_path.append(cwd)); - TRY(absolute_path.push_back('/')); - } - TRY(absolute_path.append(path_part)); - TRY(absolute_path.push_back('/')); - TRY(absolute_path.append(file_path)); - - if (!VirtualFileSystem::get().file_from_absolute_path(credentials, absolute_path, O_EXEC).is_error()) - break; - - absolute_path.clear(); - } - - if (absolute_path.empty()) - return BAN::Error::from_errno(ENOENT); - } auto file = TRY(VirtualFileSystem::get().file_from_absolute_path(credentials, absolute_path, O_EXEC)); @@ -412,14 +373,9 @@ namespace Kernel for (int i = 0; argv && argv[i]; i++) TRY(str_argv.emplace_back(argv[i])); - BAN::Vector path_env; BAN::Vector str_envp; for (int i = 0; envp && envp[i]; i++) - { TRY(str_envp.emplace_back(envp[i])); - if (strncmp(envp[i], "PATH=", 5) == 0) - path_env = TRY(BAN::StringView(envp[i]).substring(5).split(':')); - } BAN::String working_directory; @@ -428,7 +384,7 @@ namespace Kernel TRY(working_directory.append(m_working_directory)); } - auto elf = TRY(load_elf_for_exec(m_credentials, path, working_directory, path_env)); + auto elf = TRY(load_elf_for_exec(m_credentials, path, working_directory)); LockGuard lock_guard(m_lock); @@ -547,13 +503,6 @@ namespace Kernel return 0; } - BAN::ErrorOr Process::sys_setenvp(char** envp) - { - LockGuard _(m_lock); - m_userspace_info.envp = envp; - return 0; - } - void Process::load_elf_to_memory(LibELF::ELF& elf) { ASSERT(elf.is_native()); diff --git a/kernel/kernel/Syscall.cpp b/kernel/kernel/Syscall.cpp index d319e0dc2a..b8e9b37064 100644 --- a/kernel/kernel/Syscall.cpp +++ b/kernel/kernel/Syscall.cpp @@ -97,9 +97,6 @@ namespace Kernel case SYS_FSTAT: ret = Process::current().sys_fstat((int)arg1, (struct stat*)arg2); break; - case SYS_SETENVP: - ret = Process::current().sys_setenvp((char**)arg1); - break; case SYS_READ_DIR_ENTRIES: ret = Process::current().sys_read_dir_entries((int)arg1, (API::DirectoryEntryList*)arg2, (size_t)arg3); break; diff --git a/libc/include/sys/syscall.h b/libc/include/sys/syscall.h index 02ce94d331..dd8052e24d 100644 --- a/libc/include/sys/syscall.h +++ b/libc/include/sys/syscall.h @@ -21,7 +21,6 @@ __BEGIN_DECLS #define SYS_SLEEP 17 #define SYS_WAIT 18 #define SYS_FSTAT 19 -#define SYS_SETENVP 20 #define SYS_READ_DIR_ENTRIES 21 #define SYS_SET_UID 22 #define SYS_SET_GID 23 diff --git a/libc/stdlib.cpp b/libc/stdlib.cpp index 51c51efa64..4d5e95702a 100644 --- a/libc/stdlib.cpp +++ b/libc/stdlib.cpp @@ -124,6 +124,16 @@ int putenv(char* string) return -1; } + if (!environ) + { + environ = (char**)malloc(sizeof(char*) * 2); + if (!environ) + return -1; + environ[0] = string; + environ[1] = nullptr; + return 0; + } + int cnt = 0; for (int i = 0; string[i]; i++) if (string[i] == '=') @@ -151,10 +161,7 @@ int putenv(char* string) char** new_envp = (char**)malloc(sizeof(char*) * (env_count + 2)); if (new_envp == nullptr) - { - errno = ENOMEM; return -1; - } for (int i = 0; i < env_count; i++) new_envp[i] = environ[i]; @@ -164,8 +171,6 @@ int putenv(char* string) free(environ); environ = new_envp; - if (syscall(SYS_SETENVP, environ) == -1) - return -1; return 0; } diff --git a/libc/unistd.cpp b/libc/unistd.cpp index 964f517cba..c6eec3881c 100644 --- a/libc/unistd.cpp +++ b/libc/unistd.cpp @@ -14,8 +14,23 @@ char** environ; extern void init_malloc(); extern "C" void _init_libc(char** _environ) { - environ = _environ; init_malloc(); + + if (!_environ) + return; + + size_t env_count = 0; + while (_environ[env_count]) + env_count++; + + environ = (char**)malloc(sizeof(char*) * env_count + 1); + for (size_t i = 0; i < env_count; i++) + { + size_t bytes = strlen(_environ[i]) + 1; + environ[i] = (char*)malloc(bytes); + memcpy(environ[i], _environ[i], bytes); + } + environ[env_count] = nullptr; } void _exit(int status) diff --git a/userspace/Shell/main.cpp b/userspace/Shell/main.cpp index 2c7df0ba1f..3a59bcb818 100644 --- a/userspace/Shell/main.cpp +++ b/userspace/Shell/main.cpp @@ -437,6 +437,40 @@ pid_t execute_command_no_wait(BAN::Vector& args, int fd_in, int fd_ MUST(cmd_args.push_back((char*)arg.data())); MUST(cmd_args.push_back(nullptr)); + // do PATH resolution + BAN::String executable_file; + if (!args.front().empty() && args.front().front() != '.' && args.front().front() != '/') + { + char* path_env_cstr = getenv("PATH"); + if (path_env_cstr) + { + auto path_env_list = MUST(BAN::StringView(path_env_cstr).split(':')); + for (auto path_env : path_env_list) + { + BAN::String test_file = path_env; + MUST(test_file.push_back('/')); + MUST(test_file.append(args.front())); + + struct stat st; + if (stat(test_file.data(), &st) == 0) + { + executable_file = BAN::move(test_file); + break; + } + } + + if (executable_file.empty()) + { + fprintf(stderr, "command not found: %s\n", args.front().data()); + return -1; + } + } + } + else + { + executable_file = args.front(); + } + pid_t pid = fork(); if (pid == 0) { @@ -477,11 +511,14 @@ pid_t execute_command_no_wait(BAN::Vector& args, int fd_in, int fd_ setpgid(0, pgrp); } - execv(cmd_args.front(), cmd_args.data()); + execv(executable_file.data(), cmd_args.data()); perror("execv"); exit(1); } + if (pid == -1) + ERROR_RETURN("fork", -1); + return pid; } @@ -489,7 +526,7 @@ int execute_command(BAN::Vector& args, int fd_in, int fd_out) { pid_t pid = execute_command_no_wait(args, fd_in, fd_out, 0); if (pid == -1) - ERROR_RETURN("fork", 1); + return 1; int status; if (waitpid(pid, &status, 0) == -1) diff --git a/userspace/init/main.cpp b/userspace/init/main.cpp index 540d4d7da9..7252392827 100644 --- a/userspace/init/main.cpp +++ b/userspace/init/main.cpp @@ -74,6 +74,8 @@ int main() if (setuid(pwd->pw_uid) == -1) perror("setuid"); + setenv("PATH", "/bin:/usr/bin", 0); + setenv("HOME", pwd->pw_dir, 1); chdir(pwd->pw_dir);