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

Message ID 20230625231751.404120-4-bugaevc@gmail.com
State Committed
Commit 19c3b318127005444e55feb35e27d877a6af8461
Headers
Series [1/5] htl: Let Mach place thread stacks |

Checks

Context Check Description
redhat-pt-bot/TryBot-apply_patch success Patch applied to master at the time it was sent
linaro-tcwg-bot/tcwg_glibc_build--master-arm success Testing passed
linaro-tcwg-bot/tcwg_glibc_check--master-arm fail Testing failed
linaro-tcwg-bot/tcwg_glibc_build--master-aarch64 success Testing passed
linaro-tcwg-bot/tcwg_glibc_check--master-aarch64 success Testing passed

Commit Message

Sergey Bugaev June 25, 2023, 11:17 p.m. UTC
  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(-)
  

Comments

Samuel Thibault July 2, 2023, 11:33 p.m. UTC | #1
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
> 
>
  

Patch

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,