From 20054ae93f6e47654fed4974be64f313fdd85de6 Mon Sep 17 00:00:00 2001 From: David van Moolenbroek Date: Mon, 21 Sep 2015 14:52:29 +0000 Subject: [PATCH] Kernel: separate userland ABI on kernel page Currently, the userland ABI uses a single field ('user_sp') far into the very large 'kinfo' structure on the shared kernel information page. This precludes us from modifying or getting rid of 'kinfo' in the future without breaking userland. This patch adds a separate 'kuserinfo' structure to the kernel information page, with only information that is part of the userland ABI, in an extensible manner. Userland now uses this field if it is present, and falls back to the old field if not. Change-Id: Ib7b24b53a440f40a2edc28cdfa48447ac2179288 --- minix/include/lib.h | 2 ++ minix/include/minix/type.h | 41 ++++++++++++++++++++++++------- minix/kernel/arch/earm/memory.c | 4 +++ minix/kernel/arch/i386/memory.c | 3 +++ minix/kernel/glo.h | 5 ++-- minix/kernel/main.c | 4 +++ minix/kernel/usermapped_data.c | 6 +++-- minix/lib/libc/sys/kernel_utils.c | 39 ++++++++++++++++++++++++++++- minix/lib/libc/sys/stack_utils.c | 3 +-- minix/servers/rs/exec.c | 3 +-- minix/servers/vfs/exec.c | 2 +- minix/usr.bin/trace/kernel.c | 3 +-- 12 files changed, 94 insertions(+), 21 deletions(-) diff --git a/minix/include/lib.h b/minix/include/lib.h index 5caa03ecb..00a8eb586 100644 --- a/minix/include/lib.h +++ b/minix/include/lib.h @@ -23,6 +23,8 @@ struct minix_kerninfo *get_minix_kerninfo(void); +vir_bytes minix_get_user_sp(void); + struct ps_strings; /* forward declaration for minix_stack_fill. */ void minix_stack_params(const char *path, char * const *argv, diff --git a/minix/include/minix/type.h b/minix/include/minix/type.h index 29a2838ae..f1dba2a69 100644 --- a/minix/include/minix/type.h +++ b/minix/include/minix/type.h @@ -13,6 +13,7 @@ #include #include +#include /* Type definitions. */ typedef unsigned int vir_clicks; /* virtual addr/length in clicks */ @@ -190,10 +191,32 @@ struct arm_frclock { u32_t tcrr; /* tcrr address */ }; +/* The userland ABI portion of general information exposed by the kernel. + * This structure may only ever be extended with new fields! + */ +struct kuserinfo { + size_t kui_size; /* size of this structure, for ABI testing */ + vir_bytes kui_user_sp; /* initial stack pointer for exec'd process */ +}; + +/* If MINIX_KIF_USERINFO is set, use this to check for a particular field. */ +#define KUSERINFO_HAS_FIELD(kui,f) \ + (kui->kui_size >= offsetof(struct kuserinfo, f) + sizeof(kui->f)) + struct minix_kerninfo { - /* Binaries will depend on the offsets etc. in this - * structure, so it can't be changed willy-nilly. In - * other words, it is ABI-restricted. + /* Binaries will depend on the offsets etc. in this structure, so it + * can't be changed willy-nilly. In other words, it is ABI-restricted. + * However, various fields are to be used by services only, and are not + * to be used by userland directly. For pointers to non-userland-ABI + * structures, these structures themselves may be changed without + * breaking the userland ABI. + * + * There is currently one important legacy exception: the 'kinfo' + * structure should not be part of the userland ABI, but one of its + * fields, "user_sp" at offset 2440, is used by legacy user binaries. + * This field has since been moved into the 'kuserinfo' structure, but + * it will take another major release before we can start changing the + * layout of the 'kinfo' structure. */ #define KERNINFO_MAGIC 0xfc3b84bf u32_t kerninfo_magic; @@ -202,17 +225,17 @@ struct minix_kerninfo { u32_t flags_unused2; u32_t flags_unused3; u32_t flags_unused4; - struct kinfo *kinfo; + struct kinfo *kinfo; /* see note above! */ struct machine *machine; /* NOT userland ABI */ struct kmessages *kmessages; /* NOT userland ABI */ struct loadinfo *loadinfo; /* NOT userland ABI */ - struct minix_ipcvecs *minix_ipcvecs; - u32_t reserved; + struct minix_ipcvecs *minix_ipcvecs; /* userland ABI */ + struct kuserinfo *kuserinfo; /* userland ABI */ struct arm_frclock *arm_frclock; /* NOT userland ABI */ volatile struct kclockinfo *kclockinfo; /* NOT userland ABI */ -} __packed; +}; -#define MINIX_KIF_IPCVECS (1L << 0) +#define MINIX_KIF_IPCVECS (1L << 0) /* minix_ipcvecs is valid */ +#define MINIX_KIF_USERINFO (1L << 1) /* kuserinfo is valid */ #endif /* _TYPE_H */ - diff --git a/minix/kernel/arch/earm/memory.c b/minix/kernel/arch/earm/memory.c index 26a8eae21..7f29af43b 100644 --- a/minix/kernel/arch/earm/memory.c +++ b/minix/kernel/arch/earm/memory.c @@ -714,6 +714,7 @@ int arch_phys_map_reply(const int index, const vir_bytes addr) ASSIGN(machine); ASSIGN(kmessages); ASSIGN(loadinfo); + ASSIGN(kuserinfo); ASSIGN(arm_frclock); ASSIGN(kclockinfo); @@ -723,6 +724,9 @@ int arch_phys_map_reply(const int index, const vir_bytes addr) minix_kerninfo.kerninfo_magic = KERNINFO_MAGIC; minix_kerninfo.minix_feature_flags = minix_feature_flags; minix_kerninfo_user = (vir_bytes) FIXEDPTR(&minix_kerninfo); + + minix_kerninfo.ki_flags |= MINIX_KIF_USERINFO; + return OK; } diff --git a/minix/kernel/arch/i386/memory.c b/minix/kernel/arch/i386/memory.c index a29e06826..eac6c34ab 100644 --- a/minix/kernel/arch/i386/memory.c +++ b/minix/kernel/arch/i386/memory.c @@ -879,6 +879,7 @@ int arch_phys_map_reply(const int index, const vir_bytes addr) ASSIGN(machine); ASSIGN(kmessages); ASSIGN(loadinfo); + ASSIGN(kuserinfo); ASSIGN(arm_frclock); /* eh, why not. */ ASSIGN(kclockinfo); @@ -920,6 +921,8 @@ int arch_phys_map_reply(const int index, const vir_bytes addr) minix_kerninfo.ki_flags |= MINIX_KIF_IPCVECS; } + minix_kerninfo.ki_flags |= MINIX_KIF_USERINFO; + return OK; } diff --git a/minix/kernel/glo.h b/minix/kernel/glo.h index 2bab873e3..938105d79 100644 --- a/minix/kernel/glo.h +++ b/minix/kernel/glo.h @@ -19,10 +19,11 @@ #include "debug.h" /* Kernel information structures. This groups vital kernel information. */ -extern struct kinfo kinfo; /* kernel information for users */ -extern struct machine machine; /* machine information for users */ +extern struct kinfo kinfo; /* kernel information for services */ +extern struct machine machine; /* machine info for services */ extern struct kmessages kmessages; /* diagnostic messages in kernel */ extern struct loadinfo loadinfo; /* status of load average */ +extern struct kuserinfo kuserinfo; /* kernel information for users */ extern struct arm_frclock arm_frclock; /* ARM free-running timer info */ extern struct kclockinfo kclockinfo; /* clock information */ extern struct minix_kerninfo minix_kerninfo; diff --git a/minix/kernel/main.c b/minix/kernel/main.c index 1c1e52be8..691b2a6f5 100644 --- a/minix/kernel/main.c +++ b/minix/kernel/main.c @@ -439,6 +439,10 @@ void cstart() /* Initialize various user-mapped structures. */ memset(&arm_frclock, 0, sizeof(arm_frclock)); + memset(&kuserinfo, 0, sizeof(kuserinfo)); + kuserinfo.kui_size = sizeof(kuserinfo); + kuserinfo.kui_user_sp = kinfo.user_sp; + #ifdef USE_APIC value = env_get("no_apic"); if(value) diff --git a/minix/kernel/usermapped_data.c b/minix/kernel/usermapped_data.c index cad2511a8..42cfbedad 100644 --- a/minix/kernel/usermapped_data.c +++ b/minix/kernel/usermapped_data.c @@ -4,10 +4,12 @@ struct minix_kerninfo minix_kerninfo __section(".usermapped"); /* Kernel information structures. */ -struct kinfo kinfo __section(".usermapped"); /* kernel information for users */ -struct machine machine __section(".usermapped"); /* machine information for users */ +struct kinfo kinfo __section(".usermapped"); /* kernel information for services */ +struct machine machine __section(".usermapped"); /* machine information for services */ struct kmessages kmessages __section(".usermapped"); /* diagnostic messages in kernel */ struct loadinfo loadinfo __section(".usermapped"); /* status of load average */ +struct kuserinfo kuserinfo __section(".usermapped"); + /* kernel information for users */ struct arm_frclock arm_frclock __section(".usermapped"); /* ARM free running timer information */ struct kclockinfo kclockinfo __section(".usermapped"); /* clock information */ diff --git a/minix/lib/libc/sys/kernel_utils.c b/minix/lib/libc/sys/kernel_utils.c index 38b3f58ae..38681ab86 100644 --- a/minix/lib/libc/sys/kernel_utils.c +++ b/minix/lib/libc/sys/kernel_utils.c @@ -1,6 +1,12 @@ /* * This file contains the main routine for retrieval of the kernel information - * page. + * page, as well as abstraction routines for retrieval of specific values from + * this kernel-mapped user information structure. These routines may be used + * from both userland and system services, and their accesses are considered to + * establish part of the userland ABI. Do not add routines here that are not + * for retrieval of userland ABI fields (e.g., clock information)! Also, since + * these functions are MINIX3 specific, their names should contain - preferably + * be prefixed with - "minix_". */ #define _MINIX_SYSTEM @@ -8,6 +14,7 @@ #include #include "namespace.h" #include +#include #include extern struct minix_kerninfo *_minix_kerninfo; @@ -23,3 +30,33 @@ get_minix_kerninfo(void) return _minix_kerninfo; } + +/* + * Obtain the initial stack pointer for a new userland process. This value + * is used by routines that set up the stack when executing a new program. + * It is used for userland exec(2) and in various system services. + */ +vir_bytes +minix_get_user_sp(void) +{ + struct minix_kerninfo *ki; + + /* All information is obtained from the kernel information page. */ + ki = get_minix_kerninfo(); + + /* + * Check whether we can retrieve the user stack pointer value from the + * kuserinfo structure. In general, this test is the correct one to + * see whether the kuserinfo structure has a certain field. + */ + if ((ki->ki_flags & MINIX_KIF_USERINFO) && + KUSERINFO_HAS_FIELD(ki->kuserinfo, kui_user_sp)) { + return ki->kuserinfo->kui_user_sp; + } + + /* + * Otherwise, fall back to legacy support: retrieve the value from the + * kinfo structure. This field will eventually be removed. + */ + return ki->kinfo->user_sp; +} diff --git a/minix/lib/libc/sys/stack_utils.c b/minix/lib/libc/sys/stack_utils.c index 133881acd..0c8ad9e01 100644 --- a/minix/lib/libc/sys/stack_utils.c +++ b/minix/lib/libc/sys/stack_utils.c @@ -12,7 +12,6 @@ #include #include #include -#include #include #include @@ -131,7 +130,7 @@ void minix_stack_fill(const char *path, int argc, char * const *argv, size_t const min_size = STACK_MIN_SZ; /* Virtual address of the stack pointer, in new memory space. */ - *vsp = get_minix_kerninfo()->kinfo->user_sp - stack_size; + *vsp = minix_get_user_sp() - stack_size; /* Fill in the frame now. */ fpw = (char **) frame; diff --git a/minix/servers/rs/exec.c b/minix/servers/rs/exec.c index ea332948e..97442e4bc 100644 --- a/minix/servers/rs/exec.c +++ b/minix/servers/rs/exec.c @@ -2,7 +2,6 @@ #include #include #include -#include #include static int do_exec(int proc_e, char *exec, size_t exec_len, char *progname, @@ -73,7 +72,7 @@ static int do_exec(int proc_e, char *exec, size_t exec_len, char *progname, memset(&execi, 0, sizeof(execi)); - execi.stack_high = get_minix_kerninfo()->kinfo->user_sp; + execi.stack_high = minix_get_user_sp(); execi.stack_size = DEFAULT_STACK_LIMIT; execi.proc_e = proc_e; execi.hdr = exec; diff --git a/minix/servers/vfs/exec.c b/minix/servers/vfs/exec.c index af65b2bc5..b1049fa3e 100644 --- a/minix/servers/vfs/exec.c +++ b/minix/servers/vfs/exec.c @@ -214,7 +214,7 @@ int pm_exec(vir_bytes path, size_t path_len, vir_bytes frame, size_t frame_len, /* passed from exec() libc code */ execi.userflags = 0; - execi.args.stack_high = get_minix_kerninfo()->kinfo->user_sp; + execi.args.stack_high = minix_get_user_sp(); execi.args.stack_size = DEFAULT_STACK_LIMIT; fp->text_size = 0; diff --git a/minix/usr.bin/trace/kernel.c b/minix/usr.bin/trace/kernel.c index 4931d7ba9..bb84f2b7a 100644 --- a/minix/usr.bin/trace/kernel.c +++ b/minix/usr.bin/trace/kernel.c @@ -24,7 +24,6 @@ #include "kernel/arch/i386/include/archconst.h" /* for the KTS_ constants */ #endif #include -#include /* * Working area. By obtaining values from the kernel into these local process @@ -154,7 +153,7 @@ vir_bytes kernel_get_stacktop(void) { - return get_minix_kerninfo()->kinfo->user_sp; + return minix_get_user_sp(); } /* -- 2.44.0