Re: [PATCH] elf-load: Respect PT_GNU_STACK
On Wed, Mar 27, 2024 at 9:37 PM Samuel Thibault wrote:
> But it's not getting used anywhere?
Indeed, I forgot to extract the kern/bootstrap.c part of the change.
Ooops :) Thanks for pointing it out.
Sergey
-- >8 --
If a bootstrap ELF contains a PT_GNU_STACK phdr, take stack protection
from there. Otherwise, default to VM_PROT_ALL.
---
include/mach/exec/elf.h | 1 +
include/mach/exec/exec.h | 2 ++
kern/bootstrap.c | 8
kern/elf-load.c | 7 +++
4 files changed, 14 insertions(+), 4 deletions(-)
diff --git a/include/mach/exec/elf.h b/include/mach/exec/elf.h
index 9e4f8f7e..55304496 100644
--- a/include/mach/exec/elf.h
+++ b/include/mach/exec/elf.h
@@ -300,6 +300,7 @@ typedef struct {
#define PT_NOTE4
#define PT_SHLIB 5
#define PT_PHDR6
+#define PT_GNU_STACK 0x6474e551
#define PT_LOPROC 0x7000
#define PT_HIPROC 0x7fff
diff --git a/include/mach/exec/exec.h b/include/mach/exec/exec.h
index 94b234b0..29fa897d 100644
--- a/include/mach/exec/exec.h
+++ b/include/mach/exec/exec.h
@@ -51,6 +51,8 @@ typedef struct exec_info
/* (ELF) Address of interpreter string for loading shared libraries,
null if none. */
vm_offset_t interp;
+ /* Required stack protection. */
+ vm_prot_t stack_prot;
} exec_info_t;
typedef int exec_sectype_t;
diff --git a/kern/bootstrap.c b/kern/bootstrap.c
index 49358ac6..0470e1b6 100644
--- a/kern/bootstrap.c
+++ b/kern/bootstrap.c
@@ -620,10 +620,10 @@ build_args_and_stack(struct exec_info *boot_exec_info,
stack_size = round_page(STACK_SIZE);
stack_base = user_stack_low(stack_size);
- (void) vm_allocate(current_task()->map,
- &stack_base,
- stack_size,
- FALSE);
+ (void) vm_map(current_map(), &stack_base, stack_size,
+ 0, FALSE, IP_NULL, 0, FALSE,
+ boot_exec_info->stack_prot, VM_PROT_ALL,
+ VM_INHERIT_DEFAULT);
arg_pos = (char *)
set_user_regs(stack_base, stack_size, boot_exec_info, arg_len);
diff --git a/kern/elf-load.c b/kern/elf-load.c
index ce86327c..596233a8 100644
--- a/kern/elf-load.c
+++ b/kern/elf-load.c
@@ -73,6 +73,8 @@ int exec_load(exec_read_func_t *read, exec_read_exec_func_t
*read_exec,
if (actual < phsize)
return EX_CORRUPT;
+ out_info->stack_prot = VM_PROT_ALL;
+
for (i = 0; i < x.e_phnum; i++)
{
ph = (Elf_Phdr *)((vm_offset_t)phdr + i * x.e_phentsize);
@@ -89,6 +91,11 @@ int exec_load(exec_read_func_t *read, exec_read_exec_func_t
*read_exec,
ph->p_vaddr + loadbase,
ph->p_memsz, type);
if (result)
return result;
+ } else if (ph->p_type == PT_GNU_STACK) {
+ out_info->stack_prot = 0;
+ if (ph->p_flags & PF_R) out_info->stack_prot |=
VM_PROT_READ;
+ if (ph->p_flags & PF_W) out_info->stack_prot |=
VM_PROT_WRITE;
+ if (ph->p_flags & PF_X) out_info->stack_prot |=
VM_PROT_EXECUTE;
}
}
--
2.44.0
Re: [PATCH 17/17] tests: Create tests/ in the build tree before trying to use it
Applied, thanks! Sergey Bugaev, le mer. 27 mars 2024 19:18:41 +0300, a ecrit: > --- > tests/user-qemu.mk | 1 + > 1 file changed, 1 insertion(+) > > diff --git a/tests/user-qemu.mk b/tests/user-qemu.mk > index fd5ae1ab..4f8390b6 100644 > --- a/tests/user-qemu.mk > +++ b/tests/user-qemu.mk > @@ -130,6 +130,7 @@ SRC_TESTLIB= \ > $(MIG_GEN_CC) > > tests/errlist.c: $(addprefix $(srcdir)/include/mach/,message.h kern_return.h > mig_errors.h) > + mkdir -p tests > echo "/* autogenerated file */" >$@ > echo "#include " >>$@ > echo "#include " >>$@ > -- > 2.44.0 > > -- Samuel --- Pour une évaluation indépendante, transparente et rigoureuse ! Je soutiens la Commission d'Évaluation de l'Inria.
Re: [PATCH 16/17] tests: Don't ask for executable stack
Applied, thanks! Sergey Bugaev, le mer. 27 mars 2024 19:18:40 +0300, a ecrit: > --- > tests/start.S| 2 ++ > tests/syscalls.S | 2 ++ > 2 files changed, 4 insertions(+) > > diff --git a/tests/start.S b/tests/start.S > index b795bfbd..15970fb9 100644 > --- a/tests/start.S > +++ b/tests/start.S > @@ -26,3 +26,5 @@ _start: > movq%rsp,%rdi > callq c_start > #endif /* __x86_64__ */ > + > + .section .note.GNU-stack,"",%progbits > diff --git a/tests/syscalls.S b/tests/syscalls.S > index df9c9bc0..b1e18aa8 100644 > --- a/tests/syscalls.S > +++ b/tests/syscalls.S > @@ -2,3 +2,5 @@ > #include > > kernel_trap(invalid_syscall,-31,0) > + > + .section .note.GNU-stack,"",%progbits > -- > 2.44.0 > > -- Samuel --- Pour une évaluation indépendante, transparente et rigoureuse ! Je soutiens la Commission d'Évaluation de l'Inria.
Re: [PATCH 15/17] tests: Make exception subcode a long
Applied, thansks!
Sergey Bugaev, le mer. 27 mars 2024 19:18:39 +0300, a ecrit:
> ---
> tests/test-syscalls.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/tests/test-syscalls.c b/tests/test-syscalls.c
> index 63c2690a..fbfecd9c 100644
> --- a/tests/test-syscalls.c
> +++ b/tests/test-syscalls.c
> @@ -34,7 +34,7 @@ static struct {
>mach_port_t task;
>integer_t exception;
>integer_t code;
> - integer_t subcode;
> + long_integer_t subcode;
> } last_exc;
> kern_return_t catch_exception_raise(mach_port_t exception_port,
> mach_port_t thread, mach_port_t task,
> --
> 2.44.0
>
>
--
Samuel
---
Pour une évaluation indépendante, transparente et rigoureuse !
Je soutiens la Commission d'Évaluation de l'Inria.
Re: [PATCH 14/17] tests: Use vm_page_size
Applied, thanks!
Sergey Bugaev, le mer. 27 mars 2024 19:18:38 +0300, a ecrit:
> ---
> tests/testlib_thread_start.c | 19 +++
> 1 file changed, 11 insertions(+), 8 deletions(-)
>
> diff --git a/tests/testlib_thread_start.c b/tests/testlib_thread_start.c
> index fa8af0ea..df4b19ab 100644
> --- a/tests/testlib_thread_start.c
> +++ b/tests/testlib_thread_start.c
> @@ -30,30 +30,33 @@
> #include
> #include
>
> -/* This is just a temporary mapping to set up the stack */
> -static long stack_top[PAGE_SIZE/sizeof(long)] __attribute__ ((aligned
> (PAGE_SIZE)));
> -
> thread_t test_thread_start(task_t task, void(*routine)(void*), void* arg) {
> - const vm_size_t stack_size = PAGE_SIZE * 16;
> + const vm_size_t stack_size = vm_page_size * 16;
>kern_return_t ret;
> - vm_address_t stack;
> + vm_address_t stack, local_stack;
> +
> + ret = vm_allocate(mach_task_self(), &local_stack, vm_page_size, TRUE);
> + ASSERT_RET(ret, "can't allocate local stack");
>
>ret = vm_allocate(task, &stack, stack_size, TRUE);
>ASSERT_RET(ret, "can't allocate the stack for a new thread");
>
> - ret = vm_protect(task, stack, PAGE_SIZE, FALSE, VM_PROT_NONE);
> + ret = vm_protect(task, stack, vm_page_size, FALSE, VM_PROT_NONE);
>ASSERT_RET(ret, "can't protect the stack from overflows");
>
> - long *top = (long*)((vm_offset_t)stack_top + PAGE_SIZE) - 1;
> + long *top = (long*)(local_stack + vm_page_size) - 1;
> #ifdef __i386__
>*top = (long)arg; /* The argument is passed on the stack on x86_32 */
>*(top - 1) = 0; /* The return address */
> #elif defined(__x86_64__)
>*top = 0; /* The return address */
> #endif
> - ret = vm_write(task, stack + stack_size - PAGE_SIZE,
> (vm_offset_t)stack_top, PAGE_SIZE);
> + ret = vm_write(task, stack + stack_size - vm_page_size, local_stack,
> vm_page_size);
>ASSERT_RET(ret, "can't initialize the stack for the new thread");
>
> + ret = vm_deallocate(mach_task_self(), local_stack, vm_page_size);
> + ASSERT_RET(ret, "can't deallocate local stack");
> +
>thread_t thread;
>ret = thread_create(task, &thread);
>ASSERT_RET(ret, "thread_create()");
> --
> 2.44.0
>
>
--
Samuel
---
Pour une évaluation indépendante, transparente et rigoureuse !
Je soutiens la Commission d'Évaluation de l'Inria.
Re: [PATCH 13/17] tests: Add vm_page_size
Applied, thanks!
Sergey Bugaev, le mer. 27 mars 2024 19:18:37 +0300, a ecrit:
> ---
> tests/include/testlib.h | 2 ++
> tests/testlib.c | 13 +
> 2 files changed, 15 insertions(+)
>
> diff --git a/tests/include/testlib.h b/tests/include/testlib.h
> index d2367124..035fdc28 100644
> --- a/tests/include/testlib.h
> +++ b/tests/include/testlib.h
> @@ -70,6 +70,8 @@ thread_t test_thread_start(task_t task,
> void(*routine)(void*), void* arg);
> mach_port_t host_priv(void);
> mach_port_t device_priv(void);
>
> +extern vm_size_t vm_page_size;
> +
> extern void mach_msg_destroy(mach_msg_header_t *msg);
>
> extern mach_msg_return_t mach_msg_server(
> diff --git a/tests/testlib.c b/tests/testlib.c
> index baf1ce5c..12c5e771 100644
> --- a/tests/testlib.c
> +++ b/tests/testlib.c
> @@ -29,6 +29,11 @@
> #include
> #include
>
> +#ifdef PAGE_SIZE
> +vm_size_t vm_page_size = PAGE_SIZE;
> +#else
> +vm_size_t vm_page_size;
> +#endif
>
> static int argc = 0;
> static char *argv_unknown[] = {"unknown", "m1", "123", "456"};
> @@ -212,6 +217,7 @@ mach_msg_return_t mach_msg_server_once(
> void __attribute__((used, retain))
> c_start(void **argptr)
> {
> + kern_return_t kr;
>intptr_t* argcptr = (intptr_t*)argptr;
>argc = argcptr[0];
>argv = (char **) &argcptr[1];
> @@ -224,6 +230,13 @@ c_start(void **argptr)
>mach_atoi(argv[1], &host_priv_port);
>mach_atoi(argv[2], &device_master_port);
>
> +#ifndef PAGE_SIZE
> + vm_statistics_data_t stats;
> + kr = vm_statistics (mach_task_self(), &stats);
> + ASSERT_RET(kr, "can't get page size");
> + vm_page_size = stats.pagesize;
> +#endif
> +
>printf("started %s", argv[0]);
>for (int i=1; i{
> --
> 2.44.0
>
>
--
Samuel
---
Pour une évaluation indépendante, transparente et rigoureuse !
Je soutiens la Commission d'Évaluation de l'Inria.
Re: [PATCH 12/17] tests: Add a more serious mach_msg_server() routine
Applied, thanks!
Sergey Bugaev, le mer. 27 mars 2024 19:18:36 +0300, a ecrit:
> ---
> tests/include/testlib.h | 16 ++
> tests/test-syscalls.c | 40 +
> tests/testlib.c | 123
> 3 files changed, 142 insertions(+), 37 deletions(-)
>
> diff --git a/tests/include/testlib.h b/tests/include/testlib.h
> index cdb2ce13..d2367124 100644
> --- a/tests/include/testlib.h
> +++ b/tests/include/testlib.h
> @@ -70,6 +70,22 @@ thread_t test_thread_start(task_t task,
> void(*routine)(void*), void* arg);
> mach_port_t host_priv(void);
> mach_port_t device_priv(void);
>
> +extern void mach_msg_destroy(mach_msg_header_t *msg);
> +
> +extern mach_msg_return_t mach_msg_server(
> + boolean_t (*demux) (mach_msg_header_t *request,
> + mach_msg_header_t *reply),
> + mach_msg_size_t max_size,
> + mach_port_t rcv_name,
> + mach_msg_option_t options);
> +
> +extern mach_msg_return_t mach_msg_server_once(
> + boolean_t (*demux) (mach_msg_header_t *request,
> + mach_msg_header_t *reply),
> + mach_msg_size_t max_size,
> + mach_port_t rcv_name,
> + mach_msg_option_t options);
> +
> int main(int argc, char *argv[], int envc, char *envp[]);
>
> #endif /* TESTLIB_H */
> diff --git a/tests/test-syscalls.c b/tests/test-syscalls.c
> index be4df8c3..63c2690a 100644
> --- a/tests/test-syscalls.c
> +++ b/tests/test-syscalls.c
> @@ -49,44 +49,10 @@ kern_return_t catch_exception_raise(mach_port_t
> exception_port,
>last_exc.exception = exception;
>last_exc.code = code;
>last_exc.subcode = subcode;
> + thread_terminate(thread);
>return KERN_SUCCESS;
> }
>
> -static char simple_request_data[PAGE_SIZE];
> -static char simple_reply_data[PAGE_SIZE];
> -int simple_msg_server(boolean_t (*demuxer) (mach_msg_header_t *request,
> - mach_msg_header_t *reply),
> - mach_port_t rcv_port_name,
> - int num_msgs)
> -{
> - int midx = 0, mok = 0;
> - int ret;
> - mig_reply_header_t *request = (mig_reply_header_t*)simple_request_data;
> - mig_reply_header_t *reply = (mig_reply_header_t*)simple_reply_data;
> - while ((midx - num_msgs) < 0)
> -{
> - ret = mach_msg(&request->Head, MACH_RCV_MSG, 0, PAGE_SIZE,
> - rcv_port_name, 0, MACH_PORT_NULL);
> - switch (ret)
> -{
> -case MACH_MSG_SUCCESS:
> - if ((*demuxer)(&request->Head, &reply->Head))
> -mok++; // TODO send reply
> - else
> -FAILURE("demuxer didn't handle the message");
> - break;
> -default:
> - ASSERT_RET(ret, "receiving in msg_server");
> - break;
> -}
> - midx++;
> -}
> - if (mok != midx)
> -FAILURE("wrong number of message received");
> - return mok != midx;
> -}
> -
> -
> void test_syscall_bad_arg_on_stack(void *arg)
> {
>/* mach_msg() has 7 arguments, so the last one should be always
> @@ -152,13 +118,13 @@ int main(int argc, char *argv[], int envc, char *envp[])
>
>memset(&last_exc, 0, sizeof(last_exc));
>test_thread_start(mach_task_self(), test_bad_syscall_num, NULL);
> - ASSERT_RET(simple_msg_server(exc_server, excp, 1), "error in exc server");
> + ASSERT_RET(mach_msg_server_once(exc_server, 4096, excp,
> MACH_MSG_OPTION_NONE), "error in exc server");
>ASSERT((last_exc.exception == EXC_BAD_INSTRUCTION) && (last_exc.code ==
> EXC_I386_INVOP),
> "bad exception for test_bad_syscall_num()");
>
>memset(&last_exc, 0, sizeof(last_exc));
>test_thread_start(mach_task_self(), test_syscall_bad_arg_on_stack, NULL);
> - ASSERT_RET(simple_msg_server(exc_server, excp, 1), "error in exc server");
> + ASSERT_RET(mach_msg_server_once(exc_server, 4096, excp,
> MACH_MSG_OPTION_NONE), "error in exc server");
>ASSERT((last_exc.exception == EXC_BAD_ACCESS) && (last_exc.code ==
> KERN_INVALID_ADDRESS),
> "bad exception for test_syscall_bad_arg_on_stack()");
>
> diff --git a/tests/testlib.c b/tests/testlib.c
> index d2198830..baf1ce5c 100644
> --- a/tests/testlib.c
> +++ b/tests/testlib.c
> @@ -26,6 +26,7 @@
> #include
>
> #include
> +#include
> #include
>
>
> @@ -81,6 +82,128 @@ const char* e2s(int err)
>}
> }
>
> +void mach_msg_destroy(mach_msg_header_t *msg)
> +{
> + mach_port_t tmp;
> +
> + tmp = mach_reply_port();
> +
> + msg->msgh_local_port = msg->msgh_remote_port;
> + msg->msgh_bits = MACH_MSGH_BITS(MACH_MSG_TYPE_MAKE_SEND,
> + MACH_MSGH_BITS_REMOTE(msg->msgh_bits))
> + | MACH_MSGH_BITS_OTHER(msg->msgh_bits);
> +
> + mach_msg(msg, MACH_SEND_MSG, msg->msgh_size, 0, MACH_PORT_NULL,
> + MACH_MSG_TIMEOUT_NONE, MACH_PORT_NUL
Re: [PATCH 11/17] tests: Fix halt()
Applied, thanks!
Sergey Bugaev, le mer. 27 mars 2024 19:18:35 +0300, a ecrit:
> Mark it as noreturn, and make sure to halt, not reboot.
> ---
> tests/include/testlib.h | 2 +-
> tests/testlib.c | 3 ++-
> 2 files changed, 3 insertions(+), 2 deletions(-)
>
> diff --git a/tests/include/testlib.h b/tests/include/testlib.h
> index a3f3a6a8..cdb2ce13 100644
> --- a/tests/include/testlib.h
> +++ b/tests/include/testlib.h
> @@ -63,7 +63,7 @@ extern const char* TEST_FAILURE_MARKER;
>
> const char* e2s(int err);
> const char* e2s_gnumach(int err);
> -void halt();
> +extern void __attribute__((noreturn)) halt();
> int msleep(uint32_t timeout);
> thread_t test_thread_start(task_t task, void(*routine)(void*), void* arg);
>
> diff --git a/tests/testlib.c b/tests/testlib.c
> index 2eaeb591..d2198830 100644
> --- a/tests/testlib.c
> +++ b/tests/testlib.c
> @@ -23,6 +23,7 @@
> #include
> #include
> #include
> +#include
>
> #include
> #include
> @@ -55,7 +56,7 @@ mach_port_t device_priv(void)
>
> void halt()
> {
> - int ret = host_reboot(host_priv_port, 0);
> + int ret = host_reboot(host_priv_port, RB_HALT);
>ASSERT_RET(ret, "host_reboot() failed!");
>while (1)
> ;
> --
> 2.44.0
>
>
--
Samuel
---
Pour une évaluation indépendante, transparente et rigoureuse !
Je soutiens la Commission d'Évaluation de l'Inria.
Re: [PATCH 10/17] Make -fno-PIE etc. architecture-dependent
Applied, thanks! Sergey Bugaev, le mer. 27 mars 2024 19:18:34 +0300, a ecrit: > There might be good reasons why Mach on x86 shouldn't be built as PIC/ > PIE, but there are also very good reasons to support PIE on other > architectures. Potentially implementing KASLR is one such reason; but > also the Linux AArch64 boot protocol (that the AArch64 port will use for > booting) lets the bootloader load the kernel image at any address, > which makes PIC pretty much required. > --- > Makefile.am | 4 > i386/Makefrag.am | 4 > 2 files changed, 4 insertions(+), 4 deletions(-) > > diff --git a/Makefile.am b/Makefile.am > index ad38249b..357e8470 100644 > --- a/Makefile.am > +++ b/Makefile.am > @@ -79,10 +79,6 @@ AM_CFLAGS += \ > -fno-stack-protector > endif > > -# We do not support or need position-independent > -AM_CFLAGS += \ > - -no-pie -fno-PIE -fno-pie -fno-pic > - > # This must be the same size as port names, see e.g. ipc/ipc_entry.c > AM_CFLAGS += -DRDXTREE_KEY_32 > > diff --git a/i386/Makefrag.am b/i386/Makefrag.am > index 5e7d4740..7a339417 100644 > --- a/i386/Makefrag.am > +++ b/i386/Makefrag.am > @@ -170,6 +170,10 @@ gnumach_LINKFLAGS += \ > -T '$(srcdir)'/i386/ldscript > endif > > +# We do not support or need position-independent > +AM_CFLAGS += \ > + -no-pie -fno-PIE -fno-pie -fno-pic > + > AM_CFLAGS += \ > -mno-3dnow \ > -mno-mmx \ > -- > 2.44.0 > > -- Samuel --- Pour une évaluation indépendante, transparente et rigoureuse ! Je soutiens la Commission d'Évaluation de l'Inria.
Re: [PATCH 09/17] Move copy{in,out}msg declarations to copy_user.h
Sergey Bugaev, le mer. 27 mars 2024 19:18:33 +0300, a ecrit:
> Since they are implemented in copy_user.c
> ---
> i386/i386/locore.h | 4
> ipc/copy_user.h| 15 +++
> kern/exception.c | 1 +
> 3 files changed, 16 insertions(+), 4 deletions(-)
>
> diff --git a/i386/i386/locore.h b/i386/i386/locore.h
> index 217e6dec..8ff587ed 100644
> --- a/i386/i386/locore.h
> +++ b/i386/i386/locore.h
> @@ -52,10 +52,6 @@ extern int copyinmsg (const void *userbuf, void
> *kernelbuf, size_t cn, size_t kn
> extern int copyout (const void *kernelbuf, void *userbuf, size_t cn);
> #ifdef USER32
> extern int copyoutmsg (const void *kernelbuf, void *userbuf, size_t cn);
Didn't you also want to move this one?
> -#else
> -static inline int copyoutmsg (const void *kernelbuf, void *userbuf, size_t
> cn) {
> - return copyout (kernelbuf, userbuf, cn);
> -}
> #endif
>
> extern int inst_fetch (int eip, int cs);
> diff --git a/ipc/copy_user.h b/ipc/copy_user.h
> index a57b3ee5..33beacd0 100644
> --- a/ipc/copy_user.h
> +++ b/ipc/copy_user.h
> @@ -25,6 +25,21 @@
> #include
> #include
>
> +int copyinmsg(
> + const void *userbuf,
> + void*kernelbuf,
> + size_t usize,
> + size_t ksize);
> +
> +#ifdef USER32
> +int copyoutmsg(
> + const void *kernelbuf,
> + void*userbuf,
> + size_t ksize);
> +#else
> +#define copyoutmsg copyout
Better define with parameters, so that it doesn't catch e.g. variables
called copyoutmsg.
> +#endif
> +
> /*
> * The copyin_32to64() and copyout_64to32() routines are meant for data types
> * that have different size in kernel and user space. They should be
> independent
> diff --git a/kern/exception.c b/kern/exception.c
> index 7139b466..cc023d45 100644
> --- a/kern/exception.c
> +++ b/kern/exception.c
> @@ -30,6 +30,7 @@
> #include
> #include
> #include
> +#include
> #include
> #include
> #include
> --
> 2.44.0
>
>
--
Samuel
---
Pour une évaluation indépendante, transparente et rigoureuse !
Je soutiens la Commission d'Évaluation de l'Inria.
Re: [PATCH 08/17] ipc: Turn ipc_entry_lookup_failed() into a macro
Applied, thanks!
Sergey Bugaev, le mer. 27 mars 2024 19:18:32 +0300, a ecrit:
> ipc_entry_lookup_failed() is used with both mach_msg_user_header_t and
> mach_msg_header_t arguments, which are different types. Make it into a
> macro, so it works with both.
> ---
> ipc/ipc_space.h | 22 +-
> 1 file changed, 13 insertions(+), 9 deletions(-)
>
> diff --git a/ipc/ipc_space.h b/ipc/ipc_space.h
> index 96d58942..9adbd3f7 100644
> --- a/ipc/ipc_space.h
> +++ b/ipc/ipc_space.h
> @@ -159,15 +159,19 @@ ipc_entry_lookup(
>
> extern volatile boolean_t mach_port_deallocate_debug;
>
> -static inline void
> -ipc_entry_lookup_failed(mach_msg_header_t *msg, mach_port_name_t name)
> -{
> - if (name == MACH_PORT_NAME_NULL || name == MACH_PORT_NAME_DEAD)
> - return;
> - printf("task %.*s looked up a bogus port %lu for %d, most probably a
> bug.\n", (int) sizeof current_task()->name, current_task()->name, (unsigned
> long) name, msg->msgh_id);
> - if (mach_port_deallocate_debug)
> - SoftDebugger("ipc_entry_lookup");
> -}
> +#define ipc_entry_lookup_failed(msg, port_name)
> \
> +MACRO_BEGIN \
> + if (MACH_PORT_NAME_VALID(port_name)) { \
> + printf("task %.*s looked up a bogus port %lu for %d, " \
> +"most probably a bug.\n",\
> + (int) sizeof current_task()->name, \
> + current_task()->name, \
> + (unsigned long) (port_name),\
> + (msg)->msgh_id);\
> + if (mach_port_deallocate_debug) \
> + SoftDebugger("ipc_entry_lookup"); \
> + } \
> +MACRO_END
>
> /*
> * Routine:ipc_entry_get
> --
> 2.44.0
>
--
Samuel
---
Pour une évaluation indépendante, transparente et rigoureuse !
Je soutiens la Commission d'Évaluation de l'Inria.
Re: [PATCH 07/17] kern/rdxtree: Fix undefined behavior
Applied, thanks!
Sergey Bugaev, le mer. 27 mars 2024 19:18:31 +0300, a ecrit:
> Initializing a variable with itself is undefined, and GCC 14 rightfully
> produces a warning about the variable being used (to initialize itself)
> prior to initialization. X15 sets the variables to 0 instead, so do the
> same in Mach.
> ---
> kern/rdxtree.c | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/kern/rdxtree.c b/kern/rdxtree.c
> index a23d6e7e..6d03710c 100644
> --- a/kern/rdxtree.c
> +++ b/kern/rdxtree.c
> @@ -437,7 +437,7 @@ rdxtree_insert_common(struct rdxtree *tree, rdxtree_key_t
> key,
>void *ptr, void ***slotp)
> {
> struct rdxtree_node *node, *prev;
> -unsigned int height, shift, index = index;
> +unsigned int height, shift, index = 0;
> int error;
>
> assert(ptr != NULL);
> @@ -513,7 +513,7 @@ rdxtree_insert_alloc_common(struct rdxtree *tree, void
> *ptr,
> rdxtree_key_t *keyp, void ***slotp)
> {
> struct rdxtree_node *node, *prev;
> -unsigned int height, shift, index = index;
> +unsigned int height, shift, index = 0;
> rdxtree_key_t key;
> int error;
>
> --
> 2.44.0
>
>
--
Samuel
---
Pour une évaluation indépendante, transparente et rigoureuse !
Je soutiens la Commission d'Évaluation de l'Inria.
Re: [PATCH 06/17] kern/syscall_subr.c: Use copyin()/copyout() to access user memory
Sergey Bugaev, le mer. 27 mars 2024 19:18:30 +0300, a ecrit:
> It's not always possible to directly access user memory from kernel
> mode. While it's in theory a lot more expensive to fetch each character
> to be printed separately, mach_print() is only a debugging facility, and
> it's not supposed to be used for printing large amounts of data.
Yes, but the atomicity of mach_print is really useful when debugging
issues with several translators etc.
Could you make it use a buffer so we get atomicity for e.g. a hundred
characters?
Samuel
> ---
> kern/syscall_subr.c | 11 ++-
> 1 file changed, 10 insertions(+), 1 deletion(-)
>
> diff --git a/kern/syscall_subr.c b/kern/syscall_subr.c
> index e0057d94..0027be29 100644
> --- a/kern/syscall_subr.c
> +++ b/kern/syscall_subr.c
> @@ -43,6 +43,7 @@
> #include
> #include
> #include /* for splsched */
> +#include /* for copyin */
>
> #if MACH_FIXPRI
> #include
> @@ -381,6 +382,14 @@ thread_depress_abort(thread_t thread)
> void
> mach_print(const char *s)
> {
> - printf("%s", s);
> + charc;
> + while (TRUE) {
> + if (copyin(s, &c, 1))
> + return;
> + if (c == 0)
> + break;
> + printf("%c", c);
> + s++;
> + }
> }
> #endif /* MACH_KDB */
> --
> 2.44.0
Re: [PATCH 05/17] gsync: Use copyin()/copyout() to access user memory
Applied, thanks!
Sergey Bugaev, le mer. 27 mars 2024 19:18:29 +0300, a ecrit:
> Depending on the architecture and setup, it may not be possible to
> access user memory directly, for example, due to user mode mappings not
> being accessible from kernel mode (x86 SMAP, AArch64 PAN). There are
> dedicated machine-specific copyin()/copyout() routines that know how to
> access user memory from the kernel; use them.
> ---
> kern/gsync.c | 38 +++---
> 1 file changed, 31 insertions(+), 7 deletions(-)
>
> diff --git a/kern/gsync.c b/kern/gsync.c
> index 31b564ca..656e47dd 100644
> --- a/kern/gsync.c
> +++ b/kern/gsync.c
> @@ -23,6 +23,7 @@
> #include
> #include
> #include
> +#include
>
> /* An entry in the global hash table. */
> struct gsync_hbucket
> @@ -254,9 +255,28 @@ kern_return_t gsync_wait (task_t task, vm_offset_t addr,
>
>boolean_t equal;
>if (! remote)
> -equal = ((unsigned int *)addr)[0] == lo &&
> - ((flags & GSYNC_QUAD) == 0 ||
> - ((unsigned int *)addr)[1] == hi);
> +{
> + unsigned int value;
> +
> + if (copyin ((const void *) addr, &value, 4))
> + {
> + vm_map_unlock_read (task->map);
> + kmutex_unlock (&hbp->lock);
> + return KERN_INVALID_ADDRESS;
> + }
> +
> + equal = (value == lo);
> + if (flags & GSYNC_QUAD)
> + {
> + if (copyin ((const void *) (addr + 4), &value, 4))
> + {
> + vm_map_unlock_read (task->map);
> + kmutex_unlock (&hbp->lock);
> + return KERN_INVALID_ADDRESS;
> + }
> + equal = equal && (value == hi);
> + }
> +}
>else
> {
>vm_offset_t paddr = temp_mapping (&va, addr, VM_PROT_READ);
> @@ -388,11 +408,15 @@ kern_return_t gsync_wake (task_t task,
> }
>
>addr = paddr + (addr & (PAGE_SIZE - 1));
> + *(unsigned int *)addr = val;
> + vm_map_remove (kernel_map, addr, addr + sizeof (int));
> +}
> + else if (copyout (&val, (void *) addr, 4))
> +{
> + kmutex_unlock (&hbp->lock);
> + vm_map_unlock_read (task->map);
> + return KERN_INVALID_ADDRESS;
> }
> -
> - *(unsigned int *)addr = val;
> - if (task != current_task ())
> -vm_map_remove (kernel_map, addr, addr + sizeof (int));
> }
>
>vm_map_unlock_read (task->map);
> --
> 2.44.0
>
>
--
Samuel
---
Pour une évaluation indépendante, transparente et rigoureuse !
Je soutiens la Commission d'Évaluation de l'Inria.
Re: [PATCH 04/17] Load 64-bit ELFs on all 64-bit ports
Applied, thanks!
Sergey Bugaev, le mer. 27 mars 2024 19:18:28 +0300, a ecrit:
> Not only on x86_64.
> ---
> include/mach/exec/elf.h | 4 ++--
> kern/exception.c| 2 +-
> 2 files changed, 3 insertions(+), 3 deletions(-)
>
> diff --git a/include/mach/exec/elf.h b/include/mach/exec/elf.h
> index 42920e25..55304496 100644
> --- a/include/mach/exec/elf.h
> +++ b/include/mach/exec/elf.h
> @@ -212,7 +212,7 @@ typedef struct elf64_sym {
> Elf64_Xword st_size;
> } Elf64_Sym;
>
> -#ifdef __x86_64__
> +#ifdef __LP64__
> #define Elf_Sym Elf64_Sym
> #define Elf_Shdr Elf64_Shdr
> #else
> @@ -350,7 +350,7 @@ typedef struct {
> #define DT_TEXTREL 22
> #define DT_JMPREL23
>
> -#if defined(__x86_64__) && ! defined(USER32)
> +#if defined(__LP64__) && ! defined(USER32)
> typedef Elf64_Ehdr Elf_Ehdr;
> typedef Elf64_Phdr Elf_Phdr;
> #else
> diff --git a/kern/exception.c b/kern/exception.c
> index 15f29705..7139b466 100644
> --- a/kern/exception.c
> +++ b/kern/exception.c
> @@ -283,7 +283,7 @@ struct mach_exception {
> #define INTEGER_T_SIZE_IN_BITS (8 * sizeof(integer_t))
> #define INTEGER_T_TYPE MACH_MSG_TYPE_INTEGER_T
> #define RPC_LONG_INTEGER_T_SIZE_IN_BITS (8 * sizeof(rpc_long_integer_t))
> -#if defined(__x86_64__) && !defined(USER32)
> +#if defined(__LP64__) && !defined(USER32)
> #define RPC_LONG_INTEGER_T_TYPE MACH_MSG_TYPE_INTEGER_64
> #else
> #define RPC_LONG_INTEGER_T_TYPE MACH_MSG_TYPE_INTEGER_32
> --
> 2.44.0
>
>
--
Samuel
---
Pour une évaluation indépendante, transparente et rigoureuse !
Je soutiens la Commission d'Évaluation de l'Inria.
Re: [PATCH 03/17] Use the x86_64 message ABI on all 64-bit ports
Applied, thanks!
Sergey Bugaev, le mer. 27 mars 2024 19:18:27 +0300, a ecrit:
> ---
> include/mach/message.h | 10 +-
> ipc/ipc_kmsg.c | 4 ++--
> 2 files changed, 7 insertions(+), 7 deletions(-)
>
> diff --git a/include/mach/message.h b/include/mach/message.h
> index 9790ef98..87b83951 100644
> --- a/include/mach/message.h
> +++ b/include/mach/message.h
> @@ -240,7 +240,7 @@ typedef struct {
> } mach_port_name_inlined_t;
>
> typedef struct {
> -#ifdef __x86_64__
> +#ifdef __LP64__
> /*
> * For 64 bits, this struct is 8 bytes long so we
> * can pack the same amount of information as mach_msg_type_long_t.
> @@ -275,9 +275,9 @@ typedef struct {
> } __attribute__ ((aligned (__alignof__ (uintptr_t mach_msg_type_t;
>
> typedef struct {
> -#ifdef __x86_64__
> +#ifdef __LP64__
> union {
> -/* On x86_64 this is equivalent to mach_msg_type_t so use
> +/* On 64-bit this is equivalent to mach_msg_type_t so use
> * union to overlay with the old field names. */
> mach_msg_type_t msgtl_header;
> struct {
> @@ -298,7 +298,7 @@ typedef struct {
> #endif
> } __attribute__ ((aligned (__alignof__ (uintptr_t mach_msg_type_long_t;
>
> -#ifdef __x86_64__
> +#ifdef __LP64__
> #ifdef __cplusplus
> #if __cplusplus >= 201103L
> static_assert (sizeof (mach_msg_type_t) == sizeof (mach_msg_type_long_t),
> @@ -401,7 +401,7 @@ typedef integer_t mach_msg_option_t;
>
> #define MACH_SEND_ALWAYS 0x0001 /* internal use only */
>
> -#ifdef __x86_64__
> +#ifdef __LP64__
> #if defined(KERNEL) && defined(USER32)
> #define MACH_MSG_USER_ALIGNMENT 4
> #else
> diff --git a/ipc/ipc_kmsg.c b/ipc/ipc_kmsg.c
> index 179c43fa..b23cae7c 100644
> --- a/ipc/ipc_kmsg.c
> +++ b/ipc/ipc_kmsg.c
> @@ -1357,7 +1357,7 @@ ipc_kmsg_copyin_body(
>
> if ((is_port && !is_inline && (size !=
> PORT_NAME_T_SIZE_IN_BITS)) ||
> (is_port && is_inline && (size != PORT_T_SIZE_IN_BITS)) ||
> -#ifndef __x86_64__
> +#ifndef __LP64__
> (longform && ((type->msgtl_header.msgt_name != 0) ||
> (type->msgtl_header.msgt_size != 0) ||
> (type->msgtl_header.msgt_number != 0))) ||
> @@ -2876,7 +2876,7 @@ ipc_msg_print(mach_msg_header_t *msgh)
> is_port = MACH_MSG_TYPE_PORT_ANY(name);
>
> if ((is_port && (size != PORT_T_SIZE_IN_BITS)) ||
> -#ifndef __x86_64__
> +#ifndef __LP64__
> (longform && ((type->msgtl_header.msgt_name != 0) ||
> (type->msgtl_header.msgt_size != 0) ||
> (type->msgtl_header.msgt_number != 0))) ||
> --
> 2.44.0
>
>
--
Samuel
---
Pour une évaluation indépendante, transparente et rigoureuse !
Je soutiens la Commission d'Évaluation de l'Inria.
Re: [PATCH 02/17] Disable host_kernel_version() everywhere but on i386
Applied, thanks! Sergey Bugaev, le mer. 27 mars 2024 19:18:26 +0300, a ecrit: > It's not only x86_64, none of new architectures are going to have it. > --- > include/mach/mach_host.defs | 6 +++--- > kern/host.c | 4 ++-- > 2 files changed, 5 insertions(+), 5 deletions(-) > > diff --git a/include/mach/mach_host.defs b/include/mach/mach_host.defs > index a8c40af6..8fd9d6b3 100644 > --- a/include/mach/mach_host.defs > +++ b/include/mach/mach_host.defs > @@ -161,9 +161,7 @@ routine task_get_assignment( > task: task_t; > out assigned_set: processor_set_name_t); > > -#if defined(__x86_64__) && !defined(USER32) > -skip; > -#else > +#if defined(__i386__) || (defined(__x86_64__) && defined(USER32)) > /* > * Get string describing current kernel version. > * Deprecated, use host_get_kernel_version. > @@ -171,6 +169,8 @@ skip; > routine host_kernel_version( > host: host_t; > out kernel_version : kernel_version_t); > +#else > +skip; > #endif > > /* > diff --git a/kern/host.c b/kern/host.c > index 69394374..53f8bdbd 100644 > --- a/kern/host.c > +++ b/kern/host.c > @@ -219,8 +219,8 @@ kern_return_t host_get_kernel_version( > return KERN_SUCCESS; > } > > -#if !defined(__x86_64__) || defined(USER32) > -/* Same as above, but does not exist for x86_64. */ > +#if defined(__i386__) || (defined(__x86_64__) && defined(USER32)) > +/* Same as above, but only exists on i386. */ > kern_return_t host_kernel_version( > const host_thost, > kernel_version_tout_version) > -- > 2.44.0 > > -- Samuel --- Pour une évaluation indépendante, transparente et rigoureuse ! Je soutiens la Commission d'Évaluation de l'Inria.
Re: [PATCH 01/17] elf-load: Respect PT_GNU_STACK
Hello, Sergey Bugaev, le mer. 27 mars 2024 19:18:25 +0300, a ecrit: > diff --git a/include/mach/exec/exec.h b/include/mach/exec/exec.h > index 94b234b0..29fa897d 100644 > --- a/include/mach/exec/exec.h > +++ b/include/mach/exec/exec.h > @@ -51,6 +51,8 @@ typedef struct exec_info > /* (ELF) Address of interpreter string for loading shared libraries, > null if none. */ > vm_offset_t interp; > > + /* Required stack protection. */ > + vm_prot_t stack_prot; But it's not getting used anywhere? Samuel
[PATCH 16/17] tests: Don't ask for executable stack
--- tests/start.S| 2 ++ tests/syscalls.S | 2 ++ 2 files changed, 4 insertions(+) diff --git a/tests/start.S b/tests/start.S index b795bfbd..15970fb9 100644 --- a/tests/start.S +++ b/tests/start.S @@ -26,3 +26,5 @@ _start: movq%rsp,%rdi callq c_start #endif /* __x86_64__ */ + + .section .note.GNU-stack,"",%progbits diff --git a/tests/syscalls.S b/tests/syscalls.S index df9c9bc0..b1e18aa8 100644 --- a/tests/syscalls.S +++ b/tests/syscalls.S @@ -2,3 +2,5 @@ #include kernel_trap(invalid_syscall,-31,0) + + .section .note.GNU-stack,"",%progbits -- 2.44.0
[PATCH 00/17] Preparatory patches
These are generic, relatively independent patches from the AArch64 branch. I've done a quick build for i386-gnu and things still seem to build, but please test! Sergey
[PATCH 04/17] Load 64-bit ELFs on all 64-bit ports
Not only on x86_64.
---
include/mach/exec/elf.h | 4 ++--
kern/exception.c| 2 +-
2 files changed, 3 insertions(+), 3 deletions(-)
diff --git a/include/mach/exec/elf.h b/include/mach/exec/elf.h
index 42920e25..55304496 100644
--- a/include/mach/exec/elf.h
+++ b/include/mach/exec/elf.h
@@ -212,7 +212,7 @@ typedef struct elf64_sym {
Elf64_Xwordst_size;
} Elf64_Sym;
-#ifdef __x86_64__
+#ifdef __LP64__
#define Elf_Sym Elf64_Sym
#define Elf_Shdr Elf64_Shdr
#else
@@ -350,7 +350,7 @@ typedef struct {
#define DT_TEXTREL 22
#define DT_JMPREL 23
-#if defined(__x86_64__) && ! defined(USER32)
+#if defined(__LP64__) && ! defined(USER32)
typedef Elf64_Ehdr Elf_Ehdr;
typedef Elf64_Phdr Elf_Phdr;
#else
diff --git a/kern/exception.c b/kern/exception.c
index 15f29705..7139b466 100644
--- a/kern/exception.c
+++ b/kern/exception.c
@@ -283,7 +283,7 @@ struct mach_exception {
#defineINTEGER_T_SIZE_IN_BITS (8 * sizeof(integer_t))
#defineINTEGER_T_TYPE MACH_MSG_TYPE_INTEGER_T
#define RPC_LONG_INTEGER_T_SIZE_IN_BITS(8 * sizeof(rpc_long_integer_t))
-#if defined(__x86_64__) && !defined(USER32)
+#if defined(__LP64__) && !defined(USER32)
#define RPC_LONG_INTEGER_T_TYPEMACH_MSG_TYPE_INTEGER_64
#else
#define RPC_LONG_INTEGER_T_TYPEMACH_MSG_TYPE_INTEGER_32
--
2.44.0
[PATCH 02/17] Disable host_kernel_version() everywhere but on i386
It's not only x86_64, none of new architectures are going to have it. --- include/mach/mach_host.defs | 6 +++--- kern/host.c | 4 ++-- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/include/mach/mach_host.defs b/include/mach/mach_host.defs index a8c40af6..8fd9d6b3 100644 --- a/include/mach/mach_host.defs +++ b/include/mach/mach_host.defs @@ -161,9 +161,7 @@ routine task_get_assignment( task: task_t; out assigned_set: processor_set_name_t); -#if defined(__x86_64__) && !defined(USER32) -skip; -#else +#if defined(__i386__) || (defined(__x86_64__) && defined(USER32)) /* * Get string describing current kernel version. * Deprecated, use host_get_kernel_version. @@ -171,6 +169,8 @@ skip; routinehost_kernel_version( host: host_t; out kernel_version : kernel_version_t); +#else +skip; #endif /* diff --git a/kern/host.c b/kern/host.c index 69394374..53f8bdbd 100644 --- a/kern/host.c +++ b/kern/host.c @@ -219,8 +219,8 @@ kern_return_t host_get_kernel_version( return KERN_SUCCESS; } -#if !defined(__x86_64__) || defined(USER32) -/* Same as above, but does not exist for x86_64. */ +#if defined(__i386__) || (defined(__x86_64__) && defined(USER32)) +/* Same as above, but only exists on i386. */ kern_return_t host_kernel_version( const host_thost, kernel_version_tout_version) -- 2.44.0
[PATCH 01/17] elf-load: Respect PT_GNU_STACK
If a bootstrap ELF contains a PT_GNU_STACK phdr, take stack protection
from there. Otherwise, default to VM_PROT_ALL.
---
include/mach/exec/elf.h | 1 +
include/mach/exec/exec.h | 2 ++
kern/elf-load.c | 7 +++
3 files changed, 10 insertions(+)
diff --git a/include/mach/exec/elf.h b/include/mach/exec/elf.h
index 409947c4..42920e25 100644
--- a/include/mach/exec/elf.h
+++ b/include/mach/exec/elf.h
@@ -300,6 +300,7 @@ typedef struct {
#define PT_NOTE4
#define PT_SHLIB 5
#define PT_PHDR6
+#define PT_GNU_STACK 0x6474e551
#define PT_LOPROC 0x7000
#define PT_HIPROC 0x7fff
diff --git a/include/mach/exec/exec.h b/include/mach/exec/exec.h
index 94b234b0..29fa897d 100644
--- a/include/mach/exec/exec.h
+++ b/include/mach/exec/exec.h
@@ -51,6 +51,8 @@ typedef struct exec_info
/* (ELF) Address of interpreter string for loading shared libraries,
null if none. */
vm_offset_t interp;
+ /* Required stack protection. */
+ vm_prot_t stack_prot;
} exec_info_t;
typedef int exec_sectype_t;
diff --git a/kern/elf-load.c b/kern/elf-load.c
index ce86327c..596233a8 100644
--- a/kern/elf-load.c
+++ b/kern/elf-load.c
@@ -73,6 +73,8 @@ int exec_load(exec_read_func_t *read, exec_read_exec_func_t
*read_exec,
if (actual < phsize)
return EX_CORRUPT;
+ out_info->stack_prot = VM_PROT_ALL;
+
for (i = 0; i < x.e_phnum; i++)
{
ph = (Elf_Phdr *)((vm_offset_t)phdr + i * x.e_phentsize);
@@ -89,6 +91,11 @@ int exec_load(exec_read_func_t *read, exec_read_exec_func_t
*read_exec,
ph->p_vaddr + loadbase,
ph->p_memsz, type);
if (result)
return result;
+ } else if (ph->p_type == PT_GNU_STACK) {
+ out_info->stack_prot = 0;
+ if (ph->p_flags & PF_R) out_info->stack_prot |=
VM_PROT_READ;
+ if (ph->p_flags & PF_W) out_info->stack_prot |=
VM_PROT_WRITE;
+ if (ph->p_flags & PF_X) out_info->stack_prot |=
VM_PROT_EXECUTE;
}
}
--
2.44.0
[PATCH 17/17] tests: Create tests/ in the build tree before trying to use it
--- tests/user-qemu.mk | 1 + 1 file changed, 1 insertion(+) diff --git a/tests/user-qemu.mk b/tests/user-qemu.mk index fd5ae1ab..4f8390b6 100644 --- a/tests/user-qemu.mk +++ b/tests/user-qemu.mk @@ -130,6 +130,7 @@ SRC_TESTLIB= \ $(MIG_GEN_CC) tests/errlist.c: $(addprefix $(srcdir)/include/mach/,message.h kern_return.h mig_errors.h) + mkdir -p tests echo "/* autogenerated file */" >$@ echo "#include " >>$@ echo "#include " >>$@ -- 2.44.0
[PATCH 05/17] gsync: Use copyin()/copyout() to access user memory
Depending on the architecture and setup, it may not be possible to
access user memory directly, for example, due to user mode mappings not
being accessible from kernel mode (x86 SMAP, AArch64 PAN). There are
dedicated machine-specific copyin()/copyout() routines that know how to
access user memory from the kernel; use them.
---
kern/gsync.c | 38 +++---
1 file changed, 31 insertions(+), 7 deletions(-)
diff --git a/kern/gsync.c b/kern/gsync.c
index 31b564ca..656e47dd 100644
--- a/kern/gsync.c
+++ b/kern/gsync.c
@@ -23,6 +23,7 @@
#include
#include
#include
+#include
/* An entry in the global hash table. */
struct gsync_hbucket
@@ -254,9 +255,28 @@ kern_return_t gsync_wait (task_t task, vm_offset_t addr,
boolean_t equal;
if (! remote)
-equal = ((unsigned int *)addr)[0] == lo &&
- ((flags & GSYNC_QUAD) == 0 ||
- ((unsigned int *)addr)[1] == hi);
+{
+ unsigned int value;
+
+ if (copyin ((const void *) addr, &value, 4))
+ {
+ vm_map_unlock_read (task->map);
+ kmutex_unlock (&hbp->lock);
+ return KERN_INVALID_ADDRESS;
+ }
+
+ equal = (value == lo);
+ if (flags & GSYNC_QUAD)
+ {
+ if (copyin ((const void *) (addr + 4), &value, 4))
+ {
+ vm_map_unlock_read (task->map);
+ kmutex_unlock (&hbp->lock);
+ return KERN_INVALID_ADDRESS;
+ }
+ equal = equal && (value == hi);
+ }
+}
else
{
vm_offset_t paddr = temp_mapping (&va, addr, VM_PROT_READ);
@@ -388,11 +408,15 @@ kern_return_t gsync_wake (task_t task,
}
addr = paddr + (addr & (PAGE_SIZE - 1));
+ *(unsigned int *)addr = val;
+ vm_map_remove (kernel_map, addr, addr + sizeof (int));
+}
+ else if (copyout (&val, (void *) addr, 4))
+{
+ kmutex_unlock (&hbp->lock);
+ vm_map_unlock_read (task->map);
+ return KERN_INVALID_ADDRESS;
}
-
- *(unsigned int *)addr = val;
- if (task != current_task ())
-vm_map_remove (kernel_map, addr, addr + sizeof (int));
}
vm_map_unlock_read (task->map);
--
2.44.0
[PATCH 07/17] kern/rdxtree: Fix undefined behavior
Initializing a variable with itself is undefined, and GCC 14 rightfully
produces a warning about the variable being used (to initialize itself)
prior to initialization. X15 sets the variables to 0 instead, so do the
same in Mach.
---
kern/rdxtree.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/kern/rdxtree.c b/kern/rdxtree.c
index a23d6e7e..6d03710c 100644
--- a/kern/rdxtree.c
+++ b/kern/rdxtree.c
@@ -437,7 +437,7 @@ rdxtree_insert_common(struct rdxtree *tree, rdxtree_key_t
key,
void *ptr, void ***slotp)
{
struct rdxtree_node *node, *prev;
-unsigned int height, shift, index = index;
+unsigned int height, shift, index = 0;
int error;
assert(ptr != NULL);
@@ -513,7 +513,7 @@ rdxtree_insert_alloc_common(struct rdxtree *tree, void *ptr,
rdxtree_key_t *keyp, void ***slotp)
{
struct rdxtree_node *node, *prev;
-unsigned int height, shift, index = index;
+unsigned int height, shift, index = 0;
rdxtree_key_t key;
int error;
--
2.44.0
[PATCH 12/17] tests: Add a more serious mach_msg_server() routine
---
tests/include/testlib.h | 16 ++
tests/test-syscalls.c | 40 +
tests/testlib.c | 123
3 files changed, 142 insertions(+), 37 deletions(-)
diff --git a/tests/include/testlib.h b/tests/include/testlib.h
index cdb2ce13..d2367124 100644
--- a/tests/include/testlib.h
+++ b/tests/include/testlib.h
@@ -70,6 +70,22 @@ thread_t test_thread_start(task_t task,
void(*routine)(void*), void* arg);
mach_port_t host_priv(void);
mach_port_t device_priv(void);
+extern void mach_msg_destroy(mach_msg_header_t *msg);
+
+extern mach_msg_return_t mach_msg_server(
+ boolean_t (*demux) (mach_msg_header_t *request,
+ mach_msg_header_t *reply),
+ mach_msg_size_t max_size,
+ mach_port_t rcv_name,
+ mach_msg_option_t options);
+
+extern mach_msg_return_t mach_msg_server_once(
+ boolean_t (*demux) (mach_msg_header_t *request,
+ mach_msg_header_t *reply),
+ mach_msg_size_t max_size,
+ mach_port_t rcv_name,
+ mach_msg_option_t options);
+
int main(int argc, char *argv[], int envc, char *envp[]);
#endif /* TESTLIB_H */
diff --git a/tests/test-syscalls.c b/tests/test-syscalls.c
index be4df8c3..63c2690a 100644
--- a/tests/test-syscalls.c
+++ b/tests/test-syscalls.c
@@ -49,44 +49,10 @@ kern_return_t catch_exception_raise(mach_port_t
exception_port,
last_exc.exception = exception;
last_exc.code = code;
last_exc.subcode = subcode;
+ thread_terminate(thread);
return KERN_SUCCESS;
}
-static char simple_request_data[PAGE_SIZE];
-static char simple_reply_data[PAGE_SIZE];
-int simple_msg_server(boolean_t (*demuxer) (mach_msg_header_t *request,
- mach_msg_header_t *reply),
- mach_port_t rcv_port_name,
- int num_msgs)
-{
- int midx = 0, mok = 0;
- int ret;
- mig_reply_header_t *request = (mig_reply_header_t*)simple_request_data;
- mig_reply_header_t *reply = (mig_reply_header_t*)simple_reply_data;
- while ((midx - num_msgs) < 0)
-{
- ret = mach_msg(&request->Head, MACH_RCV_MSG, 0, PAGE_SIZE,
- rcv_port_name, 0, MACH_PORT_NULL);
- switch (ret)
-{
-case MACH_MSG_SUCCESS:
- if ((*demuxer)(&request->Head, &reply->Head))
-mok++; // TODO send reply
- else
-FAILURE("demuxer didn't handle the message");
- break;
-default:
- ASSERT_RET(ret, "receiving in msg_server");
- break;
-}
- midx++;
-}
- if (mok != midx)
-FAILURE("wrong number of message received");
- return mok != midx;
-}
-
-
void test_syscall_bad_arg_on_stack(void *arg)
{
/* mach_msg() has 7 arguments, so the last one should be always
@@ -152,13 +118,13 @@ int main(int argc, char *argv[], int envc, char *envp[])
memset(&last_exc, 0, sizeof(last_exc));
test_thread_start(mach_task_self(), test_bad_syscall_num, NULL);
- ASSERT_RET(simple_msg_server(exc_server, excp, 1), "error in exc server");
+ ASSERT_RET(mach_msg_server_once(exc_server, 4096, excp,
MACH_MSG_OPTION_NONE), "error in exc server");
ASSERT((last_exc.exception == EXC_BAD_INSTRUCTION) && (last_exc.code ==
EXC_I386_INVOP),
"bad exception for test_bad_syscall_num()");
memset(&last_exc, 0, sizeof(last_exc));
test_thread_start(mach_task_self(), test_syscall_bad_arg_on_stack, NULL);
- ASSERT_RET(simple_msg_server(exc_server, excp, 1), "error in exc server");
+ ASSERT_RET(mach_msg_server_once(exc_server, 4096, excp,
MACH_MSG_OPTION_NONE), "error in exc server");
ASSERT((last_exc.exception == EXC_BAD_ACCESS) && (last_exc.code ==
KERN_INVALID_ADDRESS),
"bad exception for test_syscall_bad_arg_on_stack()");
diff --git a/tests/testlib.c b/tests/testlib.c
index d2198830..baf1ce5c 100644
--- a/tests/testlib.c
+++ b/tests/testlib.c
@@ -26,6 +26,7 @@
#include
#include
+#include
#include
@@ -81,6 +82,128 @@ const char* e2s(int err)
}
}
+void mach_msg_destroy(mach_msg_header_t *msg)
+{
+ mach_port_t tmp;
+
+ tmp = mach_reply_port();
+
+ msg->msgh_local_port = msg->msgh_remote_port;
+ msg->msgh_bits = MACH_MSGH_BITS(MACH_MSG_TYPE_MAKE_SEND,
+ MACH_MSGH_BITS_REMOTE(msg->msgh_bits))
+| MACH_MSGH_BITS_OTHER(msg->msgh_bits);
+
+ mach_msg(msg, MACH_SEND_MSG, msg->msgh_size, 0, MACH_PORT_NULL,
+MACH_MSG_TIMEOUT_NONE, MACH_PORT_NULL);
+
+ mach_port_mod_refs(mach_task_self(), tmp, MACH_PORT_RIGHT_RECEIVE, -1);
+}
+
+mach_msg_return_t mach_msg_server(
+ boolean_t (*demux) (mach_msg_header_t *request,
+ mach_msg_header_t *reply),
+ mach_msg_size_t max_size,
+
[PATCH 11/17] tests: Fix halt()
Mark it as noreturn, and make sure to halt, not reboot.
---
tests/include/testlib.h | 2 +-
tests/testlib.c | 3 ++-
2 files changed, 3 insertions(+), 2 deletions(-)
diff --git a/tests/include/testlib.h b/tests/include/testlib.h
index a3f3a6a8..cdb2ce13 100644
--- a/tests/include/testlib.h
+++ b/tests/include/testlib.h
@@ -63,7 +63,7 @@ extern const char* TEST_FAILURE_MARKER;
const char* e2s(int err);
const char* e2s_gnumach(int err);
-void halt();
+extern void __attribute__((noreturn)) halt();
int msleep(uint32_t timeout);
thread_t test_thread_start(task_t task, void(*routine)(void*), void* arg);
diff --git a/tests/testlib.c b/tests/testlib.c
index 2eaeb591..d2198830 100644
--- a/tests/testlib.c
+++ b/tests/testlib.c
@@ -23,6 +23,7 @@
#include
#include
#include
+#include
#include
#include
@@ -55,7 +56,7 @@ mach_port_t device_priv(void)
void halt()
{
- int ret = host_reboot(host_priv_port, 0);
+ int ret = host_reboot(host_priv_port, RB_HALT);
ASSERT_RET(ret, "host_reboot() failed!");
while (1)
;
--
2.44.0
[PATCH 15/17] tests: Make exception subcode a long
---
tests/test-syscalls.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/tests/test-syscalls.c b/tests/test-syscalls.c
index 63c2690a..fbfecd9c 100644
--- a/tests/test-syscalls.c
+++ b/tests/test-syscalls.c
@@ -34,7 +34,7 @@ static struct {
mach_port_t task;
integer_t exception;
integer_t code;
- integer_t subcode;
+ long_integer_t subcode;
} last_exc;
kern_return_t catch_exception_raise(mach_port_t exception_port,
mach_port_t thread, mach_port_t task,
--
2.44.0
[PATCH 06/17] kern/syscall_subr.c: Use copyin()/copyout() to access user memory
It's not always possible to directly access user memory from kernel
mode. While it's in theory a lot more expensive to fetch each character
to be printed separately, mach_print() is only a debugging facility, and
it's not supposed to be used for printing large amounts of data.
---
kern/syscall_subr.c | 11 ++-
1 file changed, 10 insertions(+), 1 deletion(-)
diff --git a/kern/syscall_subr.c b/kern/syscall_subr.c
index e0057d94..0027be29 100644
--- a/kern/syscall_subr.c
+++ b/kern/syscall_subr.c
@@ -43,6 +43,7 @@
#include
#include
#include/* for splsched */
+#include /* for copyin */
#ifMACH_FIXPRI
#include
@@ -381,6 +382,14 @@ thread_depress_abort(thread_t thread)
void
mach_print(const char *s)
{
- printf("%s", s);
+ charc;
+ while (TRUE) {
+ if (copyin(s, &c, 1))
+ return;
+ if (c == 0)
+ break;
+ printf("%c", c);
+ s++;
+ }
}
#endif /* MACH_KDB */
--
2.44.0
[PATCH 09/17] Move copy{in,out}msg declarations to copy_user.h
Since they are implemented in copy_user.c
---
i386/i386/locore.h | 4
ipc/copy_user.h| 15 +++
kern/exception.c | 1 +
3 files changed, 16 insertions(+), 4 deletions(-)
diff --git a/i386/i386/locore.h b/i386/i386/locore.h
index 217e6dec..8ff587ed 100644
--- a/i386/i386/locore.h
+++ b/i386/i386/locore.h
@@ -52,10 +52,6 @@ extern int copyinmsg (const void *userbuf, void *kernelbuf,
size_t cn, size_t kn
extern int copyout (const void *kernelbuf, void *userbuf, size_t cn);
#ifdef USER32
extern int copyoutmsg (const void *kernelbuf, void *userbuf, size_t cn);
-#else
-static inline int copyoutmsg (const void *kernelbuf, void *userbuf, size_t cn)
{
- return copyout (kernelbuf, userbuf, cn);
-}
#endif
extern int inst_fetch (int eip, int cs);
diff --git a/ipc/copy_user.h b/ipc/copy_user.h
index a57b3ee5..33beacd0 100644
--- a/ipc/copy_user.h
+++ b/ipc/copy_user.h
@@ -25,6 +25,21 @@
#include
#include
+int copyinmsg(
+ const void *userbuf,
+ void*kernelbuf,
+ size_t usize,
+ size_t ksize);
+
+#ifdef USER32
+int copyoutmsg(
+ const void *kernelbuf,
+ void*userbuf,
+ size_t ksize);
+#else
+#define copyoutmsg copyout
+#endif
+
/*
* The copyin_32to64() and copyout_64to32() routines are meant for data types
* that have different size in kernel and user space. They should be
independent
diff --git a/kern/exception.c b/kern/exception.c
index 7139b466..cc023d45 100644
--- a/kern/exception.c
+++ b/kern/exception.c
@@ -30,6 +30,7 @@
#include
#include
#include
+#include
#include
#include
#include
--
2.44.0
[PATCH 13/17] tests: Add vm_page_size
---
tests/include/testlib.h | 2 ++
tests/testlib.c | 13 +
2 files changed, 15 insertions(+)
diff --git a/tests/include/testlib.h b/tests/include/testlib.h
index d2367124..035fdc28 100644
--- a/tests/include/testlib.h
+++ b/tests/include/testlib.h
@@ -70,6 +70,8 @@ thread_t test_thread_start(task_t task,
void(*routine)(void*), void* arg);
mach_port_t host_priv(void);
mach_port_t device_priv(void);
+extern vm_size_t vm_page_size;
+
extern void mach_msg_destroy(mach_msg_header_t *msg);
extern mach_msg_return_t mach_msg_server(
diff --git a/tests/testlib.c b/tests/testlib.c
index baf1ce5c..12c5e771 100644
--- a/tests/testlib.c
+++ b/tests/testlib.c
@@ -29,6 +29,11 @@
#include
#include
+#ifdef PAGE_SIZE
+vm_size_t vm_page_size = PAGE_SIZE;
+#else
+vm_size_t vm_page_size;
+#endif
static int argc = 0;
static char *argv_unknown[] = {"unknown", "m1", "123", "456"};
@@ -212,6 +217,7 @@ mach_msg_return_t mach_msg_server_once(
void __attribute__((used, retain))
c_start(void **argptr)
{
+ kern_return_t kr;
intptr_t* argcptr = (intptr_t*)argptr;
argc = argcptr[0];
argv = (char **) &argcptr[1];
@@ -224,6 +230,13 @@ c_start(void **argptr)
mach_atoi(argv[1], &host_priv_port);
mach_atoi(argv[2], &device_master_port);
+#ifndef PAGE_SIZE
+ vm_statistics_data_t stats;
+ kr = vm_statistics (mach_task_self(), &stats);
+ ASSERT_RET(kr, "can't get page size");
+ vm_page_size = stats.pagesize;
+#endif
+
printf("started %s", argv[0]);
for (int i=1; i
[PATCH 14/17] tests: Use vm_page_size
---
tests/testlib_thread_start.c | 19 +++
1 file changed, 11 insertions(+), 8 deletions(-)
diff --git a/tests/testlib_thread_start.c b/tests/testlib_thread_start.c
index fa8af0ea..df4b19ab 100644
--- a/tests/testlib_thread_start.c
+++ b/tests/testlib_thread_start.c
@@ -30,30 +30,33 @@
#include
#include
-/* This is just a temporary mapping to set up the stack */
-static long stack_top[PAGE_SIZE/sizeof(long)] __attribute__ ((aligned
(PAGE_SIZE)));
-
thread_t test_thread_start(task_t task, void(*routine)(void*), void* arg) {
- const vm_size_t stack_size = PAGE_SIZE * 16;
+ const vm_size_t stack_size = vm_page_size * 16;
kern_return_t ret;
- vm_address_t stack;
+ vm_address_t stack, local_stack;
+
+ ret = vm_allocate(mach_task_self(), &local_stack, vm_page_size, TRUE);
+ ASSERT_RET(ret, "can't allocate local stack");
ret = vm_allocate(task, &stack, stack_size, TRUE);
ASSERT_RET(ret, "can't allocate the stack for a new thread");
- ret = vm_protect(task, stack, PAGE_SIZE, FALSE, VM_PROT_NONE);
+ ret = vm_protect(task, stack, vm_page_size, FALSE, VM_PROT_NONE);
ASSERT_RET(ret, "can't protect the stack from overflows");
- long *top = (long*)((vm_offset_t)stack_top + PAGE_SIZE) - 1;
+ long *top = (long*)(local_stack + vm_page_size) - 1;
#ifdef __i386__
*top = (long)arg; /* The argument is passed on the stack on x86_32 */
*(top - 1) = 0; /* The return address */
#elif defined(__x86_64__)
*top = 0; /* The return address */
#endif
- ret = vm_write(task, stack + stack_size - PAGE_SIZE, (vm_offset_t)stack_top,
PAGE_SIZE);
+ ret = vm_write(task, stack + stack_size - vm_page_size, local_stack,
vm_page_size);
ASSERT_RET(ret, "can't initialize the stack for the new thread");
+ ret = vm_deallocate(mach_task_self(), local_stack, vm_page_size);
+ ASSERT_RET(ret, "can't deallocate local stack");
+
thread_t thread;
ret = thread_create(task, &thread);
ASSERT_RET(ret, "thread_create()");
--
2.44.0
[PATCH 10/17] Make -fno-PIE etc. architecture-dependent
There might be good reasons why Mach on x86 shouldn't be built as PIC/ PIE, but there are also very good reasons to support PIE on other architectures. Potentially implementing KASLR is one such reason; but also the Linux AArch64 boot protocol (that the AArch64 port will use for booting) lets the bootloader load the kernel image at any address, which makes PIC pretty much required. --- Makefile.am | 4 i386/Makefrag.am | 4 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/Makefile.am b/Makefile.am index ad38249b..357e8470 100644 --- a/Makefile.am +++ b/Makefile.am @@ -79,10 +79,6 @@ AM_CFLAGS += \ -fno-stack-protector endif -# We do not support or need position-independent -AM_CFLAGS += \ - -no-pie -fno-PIE -fno-pie -fno-pic - # This must be the same size as port names, see e.g. ipc/ipc_entry.c AM_CFLAGS += -DRDXTREE_KEY_32 diff --git a/i386/Makefrag.am b/i386/Makefrag.am index 5e7d4740..7a339417 100644 --- a/i386/Makefrag.am +++ b/i386/Makefrag.am @@ -170,6 +170,10 @@ gnumach_LINKFLAGS += \ -T '$(srcdir)'/i386/ldscript endif +# We do not support or need position-independent +AM_CFLAGS += \ + -no-pie -fno-PIE -fno-pie -fno-pic + AM_CFLAGS += \ -mno-3dnow \ -mno-mmx \ -- 2.44.0
[PATCH 08/17] ipc: Turn ipc_entry_lookup_failed() into a macro
ipc_entry_lookup_failed() is used with both mach_msg_user_header_t and
mach_msg_header_t arguments, which are different types. Make it into a
macro, so it works with both.
---
ipc/ipc_space.h | 22 +-
1 file changed, 13 insertions(+), 9 deletions(-)
diff --git a/ipc/ipc_space.h b/ipc/ipc_space.h
index 96d58942..9adbd3f7 100644
--- a/ipc/ipc_space.h
+++ b/ipc/ipc_space.h
@@ -159,15 +159,19 @@ ipc_entry_lookup(
extern volatile boolean_t mach_port_deallocate_debug;
-static inline void
-ipc_entry_lookup_failed(mach_msg_header_t *msg, mach_port_name_t name)
-{
- if (name == MACH_PORT_NAME_NULL || name == MACH_PORT_NAME_DEAD)
- return;
- printf("task %.*s looked up a bogus port %lu for %d, most probably a
bug.\n", (int) sizeof current_task()->name, current_task()->name, (unsigned
long) name, msg->msgh_id);
- if (mach_port_deallocate_debug)
- SoftDebugger("ipc_entry_lookup");
-}
+#define ipc_entry_lookup_failed(msg, port_name)
\
+MACRO_BEGIN\
+ if (MACH_PORT_NAME_VALID(port_name)) { \
+ printf("task %.*s looked up a bogus port %lu for %d, " \
+ "most probably a bug.\n",\
+ (int) sizeof current_task()->name, \
+ current_task()->name, \
+ (unsigned long) (port_name),\
+ (msg)->msgh_id);\
+ if (mach_port_deallocate_debug) \
+ SoftDebugger("ipc_entry_lookup"); \
+ } \
+MACRO_END
/*
* Routine:ipc_entry_get
--
2.44.0
[PATCH 03/17] Use the x86_64 message ABI on all 64-bit ports
---
include/mach/message.h | 10 +-
ipc/ipc_kmsg.c | 4 ++--
2 files changed, 7 insertions(+), 7 deletions(-)
diff --git a/include/mach/message.h b/include/mach/message.h
index 9790ef98..87b83951 100644
--- a/include/mach/message.h
+++ b/include/mach/message.h
@@ -240,7 +240,7 @@ typedef struct {
} mach_port_name_inlined_t;
typedef struct {
-#ifdef __x86_64__
+#ifdef __LP64__
/*
* For 64 bits, this struct is 8 bytes long so we
* can pack the same amount of information as mach_msg_type_long_t.
@@ -275,9 +275,9 @@ typedef struct {
} __attribute__ ((aligned (__alignof__ (uintptr_t mach_msg_type_t;
typedef struct {
-#ifdef __x86_64__
+#ifdef __LP64__
union {
-/* On x86_64 this is equivalent to mach_msg_type_t so use
+/* On 64-bit this is equivalent to mach_msg_type_t so use
* union to overlay with the old field names. */
mach_msg_type_tmsgtl_header;
struct {
@@ -298,7 +298,7 @@ typedef struct {
#endif
} __attribute__ ((aligned (__alignof__ (uintptr_t mach_msg_type_long_t;
-#ifdef __x86_64__
+#ifdef __LP64__
#ifdef __cplusplus
#if __cplusplus >= 201103L
static_assert (sizeof (mach_msg_type_t) == sizeof (mach_msg_type_long_t),
@@ -401,7 +401,7 @@ typedef integer_t mach_msg_option_t;
#define MACH_SEND_ALWAYS 0x0001 /* internal use only */
-#ifdef __x86_64__
+#ifdef __LP64__
#if defined(KERNEL) && defined(USER32)
#define MACH_MSG_USER_ALIGNMENT 4
#else
diff --git a/ipc/ipc_kmsg.c b/ipc/ipc_kmsg.c
index 179c43fa..b23cae7c 100644
--- a/ipc/ipc_kmsg.c
+++ b/ipc/ipc_kmsg.c
@@ -1357,7 +1357,7 @@ ipc_kmsg_copyin_body(
if ((is_port && !is_inline && (size !=
PORT_NAME_T_SIZE_IN_BITS)) ||
(is_port && is_inline && (size != PORT_T_SIZE_IN_BITS)) ||
-#ifndef __x86_64__
+#ifndef __LP64__
(longform && ((type->msgtl_header.msgt_name != 0) ||
(type->msgtl_header.msgt_size != 0) ||
(type->msgtl_header.msgt_number != 0))) ||
@@ -2876,7 +2876,7 @@ ipc_msg_print(mach_msg_header_t *msgh)
is_port = MACH_MSG_TYPE_PORT_ANY(name);
if ((is_port && (size != PORT_T_SIZE_IN_BITS)) ||
-#ifndef __x86_64__
+#ifndef __LP64__
(longform && ((type->msgtl_header.msgt_name != 0) ||
(type->msgtl_header.msgt_size != 0) ||
(type->msgtl_header.msgt_number != 0))) ||
--
2.44.0
