[PATCH 1/5] htl: Let Mach place thread stacks

[PATCH 1/5] htl: Let Mach place thread stacks

[Sergey Bugaev]

Instead of trying to allocate a thread stack at a specific address,
looping over the address space, just set the ANYWHERE flag in
vm_allocate (). The previous behavior:

- defeats ASLR (for Mach versions that support ASLR),
- is particularly slow if the lower 4 GB of the address space are mapped
  inaccessible, as we're planning to do on 64-bit Hurd,
- is just silly.

Signed-off-by: Sergey Bugaev <bugaevc@gmail.com>
---
 sysdeps/mach/htl/pt-stack-alloc.c | 35 ++++++-------------------------
 1 file changed, 6 insertions(+), 29 deletions(-)

diff --git a/sysdeps/mach/htl/pt-stack-alloc.c b/sysdeps/mach/htl/pt-stack-alloc.c
index 429ac2d9..97e6b445 100644
--- a/sysdeps/mach/htl/pt-stack-alloc.c
+++ b/sysdeps/mach/htl/pt-stack-alloc.c
@@ -19,14 +19,9 @@
 #include <errno.h>
 
 #include <mach.h>
-#include <mach/machine/vm_param.h>
 
 #include <pt-internal.h>
 
-/* The next address to use for stack allocation.  */
-static vm_address_t next_stack_base = VM_MIN_ADDRESS;
-
-
 /* Allocate a new stack of size STACKSIZE.  If successful, store the
    address of the newly allocated stack in *STACKADDR and return 0.
    Otherwise return an error code (EINVAL for an invalid stack size,
@@ -35,30 +30,12 @@ static vm_address_t next_stack_base = VM_MIN_ADDRESS;
 int
 __pthread_stack_alloc (void **stackaddr, size_t stacksize)
 {
-  vm_offset_t base;
-  int i = 0;
-
-get_stack:
-  i++;
-  for (base = next_stack_base;
-       base < VM_MAX_ADDRESS
-       && __vm_allocate (__mach_task_self (), &base,
-			 stacksize, FALSE) != KERN_SUCCESS; base += stacksize)
-    ;
-
-  if (base >= VM_MAX_ADDRESS)
-    {
-      if (i == 1)
-	{
-	  next_stack_base = VM_MIN_ADDRESS;
-	  goto get_stack;
-	}
-      else
-	return EAGAIN;
-    }
+  error_t err;
 
-  next_stack_base = base + stacksize;
+  err = __vm_allocate (__mach_task_self (), (vm_offset_t *) stackaddr,
+		       stacksize, TRUE);
 
-  (*stackaddr) = (void *) base;
-  return 0;
+  if (err == KERN_NO_SPACE)
+    err = EAGAIN;
+  return err;
 }
-- 
2.41.0

[PATCH 2/5] hurd: Map brk non-executable

[Sergey Bugaev]

The rest of the heap (backed by individual pages) is already mapped RW.
Mapping these pages RWX presents a security hazard.

Also, in another branch memory gets allocated using vm_allocate, which
sets memory protection to VM_PROT_DEFAULT (which is RW). The mismatch
between protections prevents Mach from coalescing the VM map entries.

Signed-off-by: Sergey Bugaev <bugaevc@gmail.com>
---
 sysdeps/mach/hurd/brk.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/sysdeps/mach/hurd/brk.c b/sysdeps/mach/hurd/brk.c
index f1349495..3a335194 100644
--- a/sysdeps/mach/hurd/brk.c
+++ b/sysdeps/mach/hurd/brk.c
@@ -106,7 +106,7 @@ _hurd_set_brk (vm_address_t addr)
 	/* First finish allocation.  */
 	err = __vm_protect (__mach_task_self (), pagebrk,
 			    alloc_start - pagebrk, 0,
-			    VM_PROT_READ|VM_PROT_WRITE|VM_PROT_EXECUTE);
+			    VM_PROT_READ|VM_PROT_WRITE);
       if (! err)
 	_hurd_brk = alloc_start;
 
@@ -120,7 +120,7 @@ _hurd_set_brk (vm_address_t addr)
   else
     /* Make the memory accessible.  */
     err = __vm_protect (__mach_task_self (), pagebrk, pagend - pagebrk,
-			0, VM_PROT_READ|VM_PROT_WRITE|VM_PROT_EXECUTE);
+			0, VM_PROT_READ|VM_PROT_WRITE);
 
   if (err)
     return __hurd_fail (err);
-- 
2.41.0

Re: [PATCH 2/5] hurd: Map brk non-executable

[Samuel Thibault]

Applied, thanks!

Sergey Bugaev via Libc-alpha, le lun. 26 juin 2023 02:17:48 +0300, a ecrit:
> The rest of the heap (backed by individual pages) is already mapped RW.
> Mapping these pages RWX presents a security hazard.
> 
> Also, in another branch memory gets allocated using vm_allocate, which
> sets memory protection to VM_PROT_DEFAULT (which is RW). The mismatch
> between protections prevents Mach from coalescing the VM map entries.
> 
> Signed-off-by: Sergey Bugaev <bugaevc@gmail.com>
> ---
>  sysdeps/mach/hurd/brk.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/sysdeps/mach/hurd/brk.c b/sysdeps/mach/hurd/brk.c
> index f1349495..3a335194 100644
> --- a/sysdeps/mach/hurd/brk.c
> +++ b/sysdeps/mach/hurd/brk.c
> @@ -106,7 +106,7 @@ _hurd_set_brk (vm_address_t addr)
>  	/* First finish allocation.  */
>  	err = __vm_protect (__mach_task_self (), pagebrk,
>  			    alloc_start - pagebrk, 0,
> -			    VM_PROT_READ|VM_PROT_WRITE|VM_PROT_EXECUTE);
> +			    VM_PROT_READ|VM_PROT_WRITE);
>        if (! err)
>  	_hurd_brk = alloc_start;
>  
> @@ -120,7 +120,7 @@ _hurd_set_brk (vm_address_t addr)
>    else
>      /* Make the memory accessible.  */
>      err = __vm_protect (__mach_task_self (), pagebrk, pagend - pagebrk,
> -			0, VM_PROT_READ|VM_PROT_WRITE|VM_PROT_EXECUTE);
> +			0, VM_PROT_READ|VM_PROT_WRITE);
>  
>    if (err)
>      return __hurd_fail (err);
> -- 
> 2.41.0
> 

-- 
Samuel
---
Pour une évaluation indépendante, transparente et rigoureuse !
Je soutiens la Commission d'Évaluation de l'Inria.

[PATCH 3/5] hurd: Fix calling vm_deallocate (NULL)

[Sergey Bugaev]

Only call vm_deallocate when we do have the old buffer, and check for
unexpected errors.

Spotted while debugging a msgids/readdir issue on x86_64-gnu.

Signed-off-by: Sergey Bugaev <bugaevc@gmail.com>
---
 sysdeps/mach/hurd/readdir64.c | 10 +++++++---
 1 file changed, 7 insertions(+), 3 deletions(-)

diff --git a/sysdeps/mach/hurd/readdir64.c b/sysdeps/mach/hurd/readdir64.c
index 2c01ca22..2d946793 100644
--- a/sysdeps/mach/hurd/readdir64.c
+++ b/sysdeps/mach/hurd/readdir64.c
@@ -64,9 +64,13 @@ __readdir64 (DIR *dirp)
 	      /* The data was passed out of line, so our old buffer is no
 		 longer useful.  Deallocate the old buffer and reset our
 		 information for the new buffer.  */
-	      __vm_deallocate (__mach_task_self (),
-			       (vm_address_t) dirp->__data,
-			       dirp->__allocation);
+	      if (dirp->__data != NULL)
+		{
+		  err = __vm_deallocate (__mach_task_self (),
+					 (vm_address_t) dirp->__data,
+					 dirp->__allocation);
+		  assert_perror (err);
+		}
 	      dirp->__data = data;
 	      dirp->__allocation = round_page (dirp->__size);
 	    }
-- 
2.41.0

Re: [PATCH 3/5] hurd: Fix calling vm_deallocate (NULL)

[Samuel Thibault]

Applied, thanks!

Sergey Bugaev, le lun. 26 juin 2023 02:17:49 +0300, a ecrit:
> Only call vm_deallocate when we do have the old buffer, and check for
> unexpected errors.
> 
> Spotted while debugging a msgids/readdir issue on x86_64-gnu.
> 
> Signed-off-by: Sergey Bugaev <bugaevc@gmail.com>
> ---
>  sysdeps/mach/hurd/readdir64.c | 10 +++++++---
>  1 file changed, 7 insertions(+), 3 deletions(-)
> 
> diff --git a/sysdeps/mach/hurd/readdir64.c b/sysdeps/mach/hurd/readdir64.c
> index 2c01ca22..2d946793 100644
> --- a/sysdeps/mach/hurd/readdir64.c
> +++ b/sysdeps/mach/hurd/readdir64.c
> @@ -64,9 +64,13 @@ __readdir64 (DIR *dirp)
>  	      /* The data was passed out of line, so our old buffer is no
>  		 longer useful.  Deallocate the old buffer and reset our
>  		 information for the new buffer.  */
> -	      __vm_deallocate (__mach_task_self (),
> -			       (vm_address_t) dirp->__data,
> -			       dirp->__allocation);
> +	      if (dirp->__data != NULL)
> +		{
> +		  err = __vm_deallocate (__mach_task_self (),
> +					 (vm_address_t) dirp->__data,
> +					 dirp->__allocation);
> +		  assert_perror (err);
> +		}
>  	      dirp->__data = data;
>  	      dirp->__allocation = round_page (dirp->__size);
>  	    }
> -- 
> 2.41.0
> 
> 

-- 
Samuel
---
Pour une évaluation indépendante, transparente et rigoureuse !
Je soutiens la Commission d'Évaluation de l'Inria.

[PATCH 4/5] hurd: Fix mapping at address 0 with MAP_FIXED

[Sergey Bugaev]

Zero address passed to mmap () typically means the caller doesn't have
any specific preferred address. Not so if MAP_FIXED is passed: in this
case 0 means literal 0. Fix this case to pass anywhere = 0 into vm_map.

Also add some documentation.

Signed-off-by: Sergey Bugaev <bugaevc@gmail.com>
---
 sysdeps/mach/hurd/mmap.c | 11 +++++++++--
 1 file changed, 9 insertions(+), 2 deletions(-)

diff --git a/sysdeps/mach/hurd/mmap.c b/sysdeps/mach/hurd/mmap.c
index 5aa70083..33672cf6 100644
--- a/sysdeps/mach/hurd/mmap.c
+++ b/sysdeps/mach/hurd/mmap.c
@@ -38,7 +38,7 @@ __mmap (void *addr, size_t len, int prot, int flags, int fd, off_t offset)
   vm_prot_t vmprot, max_vmprot;
   memory_object_t memobj;
   vm_address_t mapaddr, mask;
-  boolean_t copy;
+  boolean_t copy, anywhere;
 
   mapaddr = (vm_address_t) addr;
 
@@ -55,6 +55,7 @@ __mmap (void *addr, size_t len, int prot, int flags, int fd, off_t offset)
     vmprot |= VM_PROT_EXECUTE;
 
   copy = ! (flags & MAP_SHARED);
+  anywhere = ! (flags & MAP_FIXED);
 
 #ifdef __LP64__
   if ((addr == NULL) && (prot & PROT_EXEC)
@@ -141,9 +142,12 @@ __mmap (void *addr, size_t len, int prot, int flags, int fd, off_t offset)
   if (copy)
     max_vmprot = VM_PROT_ALL;
 
+  /* When ANYWHERE is true but the caller has provided a preferred address,
+     try mapping at that address with anywhere = 0 first.  If this fails,
+     we'll retry with anywhere = 1 below.  */
   err = __vm_map (__mach_task_self (),
 		  &mapaddr, (vm_size_t) len, mask,
-		  mapaddr == 0,
+		  anywhere && (mapaddr == 0),
 		  memobj, (vm_offset_t) offset,
 		  copy, vmprot, max_vmprot,
 		  copy ? VM_INHERIT_COPY : VM_INHERIT_SHARE);
@@ -165,7 +169,10 @@ __mmap (void *addr, size_t len, int prot, int flags, int fd, off_t offset)
     }
   else
     {
+      /* This mmap call is allowed to allocate anywhere,  */
       if (mapaddr != 0 && (err == KERN_NO_SPACE || err == KERN_INVALID_ADDRESS))
+        /* ...but above, we tried allocating at the specific address,
+           and failed to.  Now try again, with anywhere = 1 this time.  */
 	err = __vm_map (__mach_task_self (),
 			&mapaddr, (vm_size_t) len, mask,
 			1, memobj, (vm_offset_t) offset,
-- 
2.41.0

Re: [PATCH 4/5] hurd: Fix mapping at address 0 with MAP_FIXED

[Samuel Thibault]

Applied, thanks!

Sergey Bugaev, le lun. 26 juin 2023 02:17:50 +0300, a ecrit:
> Zero address passed to mmap () typically means the caller doesn't have
> any specific preferred address. Not so if MAP_FIXED is passed: in this
> case 0 means literal 0. Fix this case to pass anywhere = 0 into vm_map.
> 
> Also add some documentation.
> 
> Signed-off-by: Sergey Bugaev <bugaevc@gmail.com>
> ---
>  sysdeps/mach/hurd/mmap.c | 11 +++++++++--
>  1 file changed, 9 insertions(+), 2 deletions(-)
> 
> diff --git a/sysdeps/mach/hurd/mmap.c b/sysdeps/mach/hurd/mmap.c
> index 5aa70083..33672cf6 100644
> --- a/sysdeps/mach/hurd/mmap.c
> +++ b/sysdeps/mach/hurd/mmap.c
> @@ -38,7 +38,7 @@ __mmap (void *addr, size_t len, int prot, int flags, int fd, off_t offset)
>    vm_prot_t vmprot, max_vmprot;
>    memory_object_t memobj;
>    vm_address_t mapaddr, mask;
> -  boolean_t copy;
> +  boolean_t copy, anywhere;
>  
>    mapaddr = (vm_address_t) addr;
>  
> @@ -55,6 +55,7 @@ __mmap (void *addr, size_t len, int prot, int flags, int fd, off_t offset)
>      vmprot |= VM_PROT_EXECUTE;
>  
>    copy = ! (flags & MAP_SHARED);
> +  anywhere = ! (flags & MAP_FIXED);
>  
>  #ifdef __LP64__
>    if ((addr == NULL) && (prot & PROT_EXEC)
> @@ -141,9 +142,12 @@ __mmap (void *addr, size_t len, int prot, int flags, int fd, off_t offset)
>    if (copy)
>      max_vmprot = VM_PROT_ALL;
>  
> +  /* When ANYWHERE is true but the caller has provided a preferred address,
> +     try mapping at that address with anywhere = 0 first.  If this fails,
> +     we'll retry with anywhere = 1 below.  */
>    err = __vm_map (__mach_task_self (),
>  		  &mapaddr, (vm_size_t) len, mask,
> -		  mapaddr == 0,
> +		  anywhere && (mapaddr == 0),
>  		  memobj, (vm_offset_t) offset,
>  		  copy, vmprot, max_vmprot,
>  		  copy ? VM_INHERIT_COPY : VM_INHERIT_SHARE);
> @@ -165,7 +169,10 @@ __mmap (void *addr, size_t len, int prot, int flags, int fd, off_t offset)
>      }
>    else
>      {
> +      /* This mmap call is allowed to allocate anywhere,  */
>        if (mapaddr != 0 && (err == KERN_NO_SPACE || err == KERN_INVALID_ADDRESS))
> +        /* ...but above, we tried allocating at the specific address,
> +           and failed to.  Now try again, with anywhere = 1 this time.  */
>  	err = __vm_map (__mach_task_self (),
>  			&mapaddr, (vm_size_t) len, mask,
>  			1, memobj, (vm_offset_t) offset,
> -- 
> 2.41.0
> 
> 

-- 
Samuel
---
Pour une évaluation indépendante, transparente et rigoureuse !
Je soutiens la Commission d'Évaluation de l'Inria.

[PATCH 5/5] hurd: Implement MAP_EXCL

[Sergey Bugaev]

MAP_FIXED is defined to silently replace any existing mappings at the
address range being mapped over. This, however, is a dangerous, and only
rarely desired behavior.

Various Unix systems provide replacements or additions to MAP_FIXED:

* SerenityOS and Linux provide MAP_FIXED_NOREPLACE. If the address space
  already contains a mapping in the requested range, Linux returns
  EEXIST. SerenityOS returns ENOMEM, however that is a bug, as the
  MAP_FIXED_NOREPLACE implementation is intended to be compatible with
  Linux.

* FreeBSD provides the MAP_EXCL flag that has to be used in combination
  with MAP_FIXED. It returns EINVAL if the requested range already
  contains existing mappings. This is directly analogous to the O_EXCL
  flag in the open () call.

* DragonFly BSD, NetBSD, and OpenBSD provide MAP_TRYFIXED, but with
  different semantics. DragonFly BSD returns ENOMEM if the requested
  range already contains existing mappings. NetBSD does not return an
  error, but instead creates the mapping at a different address if the
  requested range contains mappings. OpenBSD behaves the same, but also
  notes that this is the default behavior even without MAP_TRYFIXED
  (which is the case on the Hurd too).

Since the Hurd leans closer to the BSD side, add MAP_EXCL as the primary
API to request the behavior of not replacing existing mappings. Declare
MAP_FIXED_NOREPLACE and MAP_TRYFIXED as aliases of (MAP_FIXED|MAP_EXCL),
so any existing software that checks for either of those macros will
pick them up automatically. For compatibility with Linux, return EEXIST
if a mapping already exists.

Finally, a fun bit of horrifying trivia: until very recently, there has
been a function named try_mmap_fixed () in the Wine project. On Darwin,
it used mach_vm_map () to map the (anonymous) pages without replacing
any existing mappings. On Solaris and NetBSD, it instead paused the
other threads in the process using vfork (), then used mincore () to
probe the pages in the desired address range for being already mapped --
an error return from mincore () indicates that the page is not mapped at
all. Finally, if no conflicting mappings were found, still in the
vforked child process it performed the mapping with MAP_FIXED, and then
returned from the child back into the parent, resuming the other
threads.

This relied on:

* being able to do calls other than execve and _exit from the vforked
  child, which is undefined behavior according to POSIX;

* the parent and the child sharing the address space, and changes made
  in the child being visible in the parent;

* vfork suspending all the threads of the calling process, not just the
  calling thread.

All of this is undefined according to POSIX, but was apparently true on
Solaris, which is the system this function was originally implemented
for. But on NetBSD, where this shim was later ported to, the last bullet
point does not hold: vfork only suspends the calling thread; while the
other threads continue to run.

Signed-off-by: Sergey Bugaev <bugaevc@gmail.com>
---
 sysdeps/mach/hurd/bits/mman_ext.h |  7 ++++++-
 sysdeps/mach/hurd/mmap.c          | 26 +++++++++++++++++---------
 2 files changed, 23 insertions(+), 10 deletions(-)

diff --git a/sysdeps/mach/hurd/bits/mman_ext.h b/sysdeps/mach/hurd/bits/mman_ext.h
index bbb94743..9658cdd6 100644
--- a/sysdeps/mach/hurd/bits/mman_ext.h
+++ b/sysdeps/mach/hurd/bits/mman_ext.h
@@ -22,5 +22,10 @@
 
 #ifdef __USE_GNU
 # define SHM_ANON	((const char *) 1)
-# define MAP_32BIT	0x1000
+
+# define MAP_32BIT	0x1000	/* Map in the lower 2 GB.  */
+# define MAP_EXCL	0x4000	/* With MAP_FIXED, don't replace existing mappings.  */
+
+# define MAP_TRYFIXED		(MAP_FIXED | MAP_EXCL)	/* BSD name.  */
+# define MAP_FIXED_NOREPLACE	(MAP_FIXED | MAP_EXCL)	/* Linux name.  */
 #endif /* __USE_GNU  */
diff --git a/sysdeps/mach/hurd/mmap.c b/sysdeps/mach/hurd/mmap.c
index 33672cf6..20264a77 100644
--- a/sysdeps/mach/hurd/mmap.c
+++ b/sysdeps/mach/hurd/mmap.c
@@ -46,6 +46,9 @@ __mmap (void *addr, size_t len, int prot, int flags, int fd, off_t offset)
   if ((mapaddr & (__vm_page_size - 1)) || (offset & (__vm_page_size - 1)))
     return (void *) (long int) __hurd_fail (EINVAL);
 
+  if ((flags & MAP_EXCL) && ! (flags & MAP_FIXED))
+    return (void *) (long int) __hurd_fail (EINVAL);
+
   vmprot = VM_PROT_NONE;
   if (prot & PROT_READ)
     vmprot |= VM_PROT_READ;
@@ -156,15 +159,20 @@ __mmap (void *addr, size_t len, int prot, int flags, int fd, off_t offset)
     {
       if (err == KERN_NO_SPACE)
 	{
-	  /* XXX this is not atomic as it is in unix! */
-	  /* The region is already allocated; deallocate it first.  */
-	  err = __vm_deallocate (__mach_task_self (), mapaddr, len);
-	  if (! err)
-	    err = __vm_map (__mach_task_self (),
-			    &mapaddr, (vm_size_t) len, mask,
-			    0, memobj, (vm_offset_t) offset,
-			    copy, vmprot, max_vmprot,
-			    copy ? VM_INHERIT_COPY : VM_INHERIT_SHARE);
+	  if (flags & MAP_EXCL)
+	    err = EEXIST;
+	  else
+	    {
+	      /* The region is already allocated; deallocate it first.  */
+	      /* XXX this is not atomic as it is in unix! */
+	      err = __vm_deallocate (__mach_task_self (), mapaddr, len);
+	      if (! err)
+		err = __vm_map (__mach_task_self (),
+				&mapaddr, (vm_size_t) len, mask,
+				0, memobj, (vm_offset_t) offset,
+				copy, vmprot, max_vmprot,
+				copy ? VM_INHERIT_COPY : VM_INHERIT_SHARE);
+	    }
 	}
     }
   else
-- 
2.41.0

Re: [PATCH 5/5] hurd: Implement MAP_EXCL

[Samuel Thibault]

Applied, thanks!

Sergey Bugaev via Libc-alpha, le lun. 26 juin 2023 02:17:51 +0300, a ecrit:
> MAP_FIXED is defined to silently replace any existing mappings at the
> address range being mapped over. This, however, is a dangerous, and only
> rarely desired behavior.
> 
> Various Unix systems provide replacements or additions to MAP_FIXED:
> 
> * SerenityOS and Linux provide MAP_FIXED_NOREPLACE. If the address space
>   already contains a mapping in the requested range, Linux returns
>   EEXIST. SerenityOS returns ENOMEM, however that is a bug, as the
>   MAP_FIXED_NOREPLACE implementation is intended to be compatible with
>   Linux.
> 
> * FreeBSD provides the MAP_EXCL flag that has to be used in combination
>   with MAP_FIXED. It returns EINVAL if the requested range already
>   contains existing mappings. This is directly analogous to the O_EXCL
>   flag in the open () call.
> 
> * DragonFly BSD, NetBSD, and OpenBSD provide MAP_TRYFIXED, but with
>   different semantics. DragonFly BSD returns ENOMEM if the requested
>   range already contains existing mappings. NetBSD does not return an
>   error, but instead creates the mapping at a different address if the
>   requested range contains mappings. OpenBSD behaves the same, but also
>   notes that this is the default behavior even without MAP_TRYFIXED
>   (which is the case on the Hurd too).
> 
> Since the Hurd leans closer to the BSD side, add MAP_EXCL as the primary
> API to request the behavior of not replacing existing mappings. Declare
> MAP_FIXED_NOREPLACE and MAP_TRYFIXED as aliases of (MAP_FIXED|MAP_EXCL),
> so any existing software that checks for either of those macros will
> pick them up automatically. For compatibility with Linux, return EEXIST
> if a mapping already exists.
> 
> Finally, a fun bit of horrifying trivia: until very recently, there has
> been a function named try_mmap_fixed () in the Wine project. On Darwin,
> it used mach_vm_map () to map the (anonymous) pages without replacing
> any existing mappings. On Solaris and NetBSD, it instead paused the
> other threads in the process using vfork (), then used mincore () to
> probe the pages in the desired address range for being already mapped --
> an error return from mincore () indicates that the page is not mapped at
> all. Finally, if no conflicting mappings were found, still in the
> vforked child process it performed the mapping with MAP_FIXED, and then
> returned from the child back into the parent, resuming the other
> threads.
> 
> This relied on:
> 
> * being able to do calls other than execve and _exit from the vforked
>   child, which is undefined behavior according to POSIX;
> 
> * the parent and the child sharing the address space, and changes made
>   in the child being visible in the parent;
> 
> * vfork suspending all the threads of the calling process, not just the
>   calling thread.
> 
> All of this is undefined according to POSIX, but was apparently true on
> Solaris, which is the system this function was originally implemented
> for. But on NetBSD, where this shim was later ported to, the last bullet
> point does not hold: vfork only suspends the calling thread; while the
> other threads continue to run.
> 
> Signed-off-by: Sergey Bugaev <bugaevc@gmail.com>
> ---
>  sysdeps/mach/hurd/bits/mman_ext.h |  7 ++++++-
>  sysdeps/mach/hurd/mmap.c          | 26 +++++++++++++++++---------
>  2 files changed, 23 insertions(+), 10 deletions(-)
> 
> diff --git a/sysdeps/mach/hurd/bits/mman_ext.h b/sysdeps/mach/hurd/bits/mman_ext.h
> index bbb94743..9658cdd6 100644
> --- a/sysdeps/mach/hurd/bits/mman_ext.h
> +++ b/sysdeps/mach/hurd/bits/mman_ext.h
> @@ -22,5 +22,10 @@
>  
>  #ifdef __USE_GNU
>  # define SHM_ANON	((const char *) 1)
> -# define MAP_32BIT	0x1000
> +
> +# define MAP_32BIT	0x1000	/* Map in the lower 2 GB.  */
> +# define MAP_EXCL	0x4000	/* With MAP_FIXED, don't replace existing mappings.  */
> +
> +# define MAP_TRYFIXED		(MAP_FIXED | MAP_EXCL)	/* BSD name.  */
> +# define MAP_FIXED_NOREPLACE	(MAP_FIXED | MAP_EXCL)	/* Linux name.  */
>  #endif /* __USE_GNU  */
> diff --git a/sysdeps/mach/hurd/mmap.c b/sysdeps/mach/hurd/mmap.c
> index 33672cf6..20264a77 100644
> --- a/sysdeps/mach/hurd/mmap.c
> +++ b/sysdeps/mach/hurd/mmap.c
> @@ -46,6 +46,9 @@ __mmap (void *addr, size_t len, int prot, int flags, int fd, off_t offset)
>    if ((mapaddr & (__vm_page_size - 1)) || (offset & (__vm_page_size - 1)))
>      return (void *) (long int) __hurd_fail (EINVAL);
>  
> +  if ((flags & MAP_EXCL) && ! (flags & MAP_FIXED))
> +    return (void *) (long int) __hurd_fail (EINVAL);
> +
>    vmprot = VM_PROT_NONE;
>    if (prot & PROT_READ)
>      vmprot |= VM_PROT_READ;
> @@ -156,15 +159,20 @@ __mmap (void *addr, size_t len, int prot, int flags, int fd, off_t offset)
>      {
>        if (err == KERN_NO_SPACE)
>  	{
> -	  /* XXX this is not atomic as it is in unix! */
> -	  /* The region is already allocated; deallocate it first.  */
> -	  err = __vm_deallocate (__mach_task_self (), mapaddr, len);
> -	  if (! err)
> -	    err = __vm_map (__mach_task_self (),
> -			    &mapaddr, (vm_size_t) len, mask,
> -			    0, memobj, (vm_offset_t) offset,
> -			    copy, vmprot, max_vmprot,
> -			    copy ? VM_INHERIT_COPY : VM_INHERIT_SHARE);
> +	  if (flags & MAP_EXCL)
> +	    err = EEXIST;
> +	  else
> +	    {
> +	      /* The region is already allocated; deallocate it first.  */
> +	      /* XXX this is not atomic as it is in unix! */
> +	      err = __vm_deallocate (__mach_task_self (), mapaddr, len);
> +	      if (! err)
> +		err = __vm_map (__mach_task_self (),
> +				&mapaddr, (vm_size_t) len, mask,
> +				0, memobj, (vm_offset_t) offset,
> +				copy, vmprot, max_vmprot,
> +				copy ? VM_INHERIT_COPY : VM_INHERIT_SHARE);
> +	    }
>  	}
>      }
>    else
> -- 
> 2.41.0
> 

-- 
Samuel
---
Pour une évaluation indépendante, transparente et rigoureuse !
Je soutiens la Commission d'Évaluation de l'Inria.

Re: [PATCH 1/5] htl: Let Mach place thread stacks

[Samuel Thibault]

Applied, thanks!

Sergey Bugaev, le lun. 26 juin 2023 02:17:47 +0300, a ecrit:
> Instead of trying to allocate a thread stack at a specific address,
> looping over the address space, just set the ANYWHERE flag in
> vm_allocate (). The previous behavior:
> 
> - defeats ASLR (for Mach versions that support ASLR),
> - is particularly slow if the lower 4 GB of the address space are mapped
>   inaccessible, as we're planning to do on 64-bit Hurd,
> - is just silly.
> 
> Signed-off-by: Sergey Bugaev <bugaevc@gmail.com>
> ---
>  sysdeps/mach/htl/pt-stack-alloc.c | 35 ++++++-------------------------
>  1 file changed, 6 insertions(+), 29 deletions(-)
> 
> diff --git a/sysdeps/mach/htl/pt-stack-alloc.c b/sysdeps/mach/htl/pt-stack-alloc.c
> index 429ac2d9..97e6b445 100644
> --- a/sysdeps/mach/htl/pt-stack-alloc.c
> +++ b/sysdeps/mach/htl/pt-stack-alloc.c
> @@ -19,14 +19,9 @@
>  #include <errno.h>
>  
>  #include <mach.h>
> -#include <mach/machine/vm_param.h>
>  
>  #include <pt-internal.h>
>  
> -/* The next address to use for stack allocation.  */
> -static vm_address_t next_stack_base = VM_MIN_ADDRESS;
> -
> -
>  /* Allocate a new stack of size STACKSIZE.  If successful, store the
>     address of the newly allocated stack in *STACKADDR and return 0.
>     Otherwise return an error code (EINVAL for an invalid stack size,
> @@ -35,30 +30,12 @@ static vm_address_t next_stack_base = VM_MIN_ADDRESS;
>  int
>  __pthread_stack_alloc (void **stackaddr, size_t stacksize)
>  {
> -  vm_offset_t base;
> -  int i = 0;
> -
> -get_stack:
> -  i++;
> -  for (base = next_stack_base;
> -       base < VM_MAX_ADDRESS
> -       && __vm_allocate (__mach_task_self (), &base,
> -			 stacksize, FALSE) != KERN_SUCCESS; base += stacksize)
> -    ;
> -
> -  if (base >= VM_MAX_ADDRESS)
> -    {
> -      if (i == 1)
> -	{
> -	  next_stack_base = VM_MIN_ADDRESS;
> -	  goto get_stack;
> -	}
> -      else
> -	return EAGAIN;
> -    }
> +  error_t err;
>  
> -  next_stack_base = base + stacksize;
> +  err = __vm_allocate (__mach_task_self (), (vm_offset_t *) stackaddr,
> +		       stacksize, TRUE);
>  
> -  (*stackaddr) = (void *) base;
> -  return 0;
> +  if (err == KERN_NO_SPACE)
> +    err = EAGAIN;
> +  return err;
>  }
> -- 
> 2.41.0
> 
> 

-- 
Samuel
---
Pour une évaluation indépendante, transparente et rigoureuse !
Je soutiens la Commission d'Évaluation de l'Inria.