_hurd_intr_rpc_mach_msg: handle message iteration correctly.

Message ID ZVlGVD6eEN-dXsOr@jupiter.tail36e24.ts.net
State Committed
Commit f11a92993c45a91c3237733486ce2b8735af2ba0
Headers
Series _hurd_intr_rpc_mach_msg: handle message iteration correctly. |

Checks

Context Check Description
redhat-pt-bot/TryBot-apply_patch success Patch applied to master at the time it was sent
redhat-pt-bot/TryBot-32bit success Build for i686
linaro-tcwg-bot/tcwg_glibc_check--master-aarch64 warning Patch is already merged
linaro-tcwg-bot/tcwg_glibc_build--master-arm warning Patch is already merged
linaro-tcwg-bot/tcwg_glibc_check--master-arm warning Patch is already merged
linaro-tcwg-bot/tcwg_glibc_build--master-aarch64 warning Patch is already merged

Commit Message

Flavio Cruz Nov. 18, 2023, 11:18 p.m. UTC
  The `ty` pointer is only set at the end of the loop so that
`msgtl_header.msgt_inline` and `msgtl_header.msgt_deallocate` remain
valid. Also, when deallocating memory, we use the length from the
message directly rather than hard coding mach_port_t since we want to
deallocate any kind of OOL data.
---
 hurd/intr-msg.c | 24 ++++++++++++------------
 1 file changed, 12 insertions(+), 12 deletions(-)
  

Comments

Samuel Thibault Nov. 18, 2023, 11:37 p.m. UTC | #1
Applied, thanks!

Flavio Cruz, le sam. 18 nov. 2023 18:18:44 -0500, a ecrit:
> The `ty` pointer is only set at the end of the loop so that
> `msgtl_header.msgt_inline` and `msgtl_header.msgt_deallocate` remain
> valid. Also, when deallocating memory, we use the length from the
> message directly rather than hard coding mach_port_t since we want to
> deallocate any kind of OOL data.
> ---
>  hurd/intr-msg.c | 24 ++++++++++++------------
>  1 file changed, 12 insertions(+), 12 deletions(-)
> 
> diff --git a/hurd/intr-msg.c b/hurd/intr-msg.c
> index 98f588f2..737bfe0f 100644
> --- a/hurd/intr-msg.c
> +++ b/hurd/intr-msg.c
> @@ -186,12 +186,14 @@ _hurd_intr_rpc_mach_msg (mach_msg_header_t *msg,
>  	      mach_msg_type_size_t size;
>  	      mach_msg_type_number_t number;
>  
> -	      inline void clean_ports (mach_port_t *ports, int dealloc)
> +	      inline void clean_ports_and_memory (char *data, const vm_size_t length,
> +						int dealloc)
>  		{
>  		  mach_msg_type_number_t i;
>  		  switch (name)
>  		    {
>  		    case MACH_MSG_TYPE_MOVE_SEND:
> +		      mach_port_t *ports = (mach_port_t *) data;
>  		      for (i = 0; i < number; i++)
>  			__mach_port_deallocate (__mach_task_self (), *ports++);
>  		      if (ty->msgtl_header.msgt_longform)
> @@ -207,40 +209,38 @@ _hurd_intr_rpc_mach_msg (mach_msg_header_t *msg,
>  			assert (! "unexpected port type in interruptible RPC");
>  		    }
>  		  if (dealloc)
> -		    __vm_deallocate (__mach_task_self (),
> -				     (vm_address_t) ports,
> -				     number * sizeof (mach_port_t));
> +		    __vm_deallocate (__mach_task_self (), (vm_address_t) data, length);
>  		}
>  
> +	      char *data;
>  	      if (ty->msgtl_header.msgt_longform)
>  		{
>  		  name = ty->msgtl_name;
>  		  size = ty->msgtl_size;
>  		  number = ty->msgtl_number;
> -		  ty = (void *) ty + sizeof (mach_msg_type_long_t);
> +		  data = (char *) ty + sizeof (mach_msg_type_long_t);
>  		}
>  	      else
>  		{
>  		  name = ty->msgtl_header.msgt_name;
>  		  size = ty->msgtl_header.msgt_size;
>  		  number = ty->msgtl_header.msgt_number;
> -		  ty = (void *) ty + sizeof (mach_msg_type_t);
> +		  data = (char *) ty + sizeof (mach_msg_type_t);
>  		}
>  
> +	      const vm_size_t length = ((number * size) + 7) >> 3;
>  	      if (ty->msgtl_header.msgt_inline)
>  		{
>  		  /* Calculate length of data in bytes.  */
> -		  const vm_size_t length = ((number * size) + 7) >> 3;
> -		  clean_ports ((void *) ty, 0);
> +		  clean_ports_and_memory (data, length, 0);
>  		  /* Move to the next argument.  */
> -		  ty = (void *) PTR_ALIGN_UP ((char *) ty + length,
> -		      __alignof__ (uintptr_t));
> +		  ty = (void *) PTR_ALIGN_UP (data + length, __alignof__ (uintptr_t));
>  		}
>  	      else
>  		{
> -		  clean_ports (*(void **) ty,
> +		  clean_ports_and_memory (*(void **) data, length,
>  			       ty->msgtl_header.msgt_deallocate);
> -		  ty = (void *) ty + sizeof (void *);
> +		  ty = (void *) data + sizeof (void *);
>  		}
>  	    }
>  #else  /* Untyped Mach IPC flavor. */
> -- 
> 2.39.2
>
  

Patch

diff --git a/hurd/intr-msg.c b/hurd/intr-msg.c
index 98f588f2..737bfe0f 100644
--- a/hurd/intr-msg.c
+++ b/hurd/intr-msg.c
@@ -186,12 +186,14 @@  _hurd_intr_rpc_mach_msg (mach_msg_header_t *msg,
 	      mach_msg_type_size_t size;
 	      mach_msg_type_number_t number;
 
-	      inline void clean_ports (mach_port_t *ports, int dealloc)
+	      inline void clean_ports_and_memory (char *data, const vm_size_t length,
+						int dealloc)
 		{
 		  mach_msg_type_number_t i;
 		  switch (name)
 		    {
 		    case MACH_MSG_TYPE_MOVE_SEND:
+		      mach_port_t *ports = (mach_port_t *) data;
 		      for (i = 0; i < number; i++)
 			__mach_port_deallocate (__mach_task_self (), *ports++);
 		      if (ty->msgtl_header.msgt_longform)
@@ -207,40 +209,38 @@  _hurd_intr_rpc_mach_msg (mach_msg_header_t *msg,
 			assert (! "unexpected port type in interruptible RPC");
 		    }
 		  if (dealloc)
-		    __vm_deallocate (__mach_task_self (),
-				     (vm_address_t) ports,
-				     number * sizeof (mach_port_t));
+		    __vm_deallocate (__mach_task_self (), (vm_address_t) data, length);
 		}
 
+	      char *data;
 	      if (ty->msgtl_header.msgt_longform)
 		{
 		  name = ty->msgtl_name;
 		  size = ty->msgtl_size;
 		  number = ty->msgtl_number;
-		  ty = (void *) ty + sizeof (mach_msg_type_long_t);
+		  data = (char *) ty + sizeof (mach_msg_type_long_t);
 		}
 	      else
 		{
 		  name = ty->msgtl_header.msgt_name;
 		  size = ty->msgtl_header.msgt_size;
 		  number = ty->msgtl_header.msgt_number;
-		  ty = (void *) ty + sizeof (mach_msg_type_t);
+		  data = (char *) ty + sizeof (mach_msg_type_t);
 		}
 
+	      const vm_size_t length = ((number * size) + 7) >> 3;
 	      if (ty->msgtl_header.msgt_inline)
 		{
 		  /* Calculate length of data in bytes.  */
-		  const vm_size_t length = ((number * size) + 7) >> 3;
-		  clean_ports ((void *) ty, 0);
+		  clean_ports_and_memory (data, length, 0);
 		  /* Move to the next argument.  */
-		  ty = (void *) PTR_ALIGN_UP ((char *) ty + length,
-		      __alignof__ (uintptr_t));
+		  ty = (void *) PTR_ALIGN_UP (data + length, __alignof__ (uintptr_t));
 		}
 	      else
 		{
-		  clean_ports (*(void **) ty,
+		  clean_ports_and_memory (*(void **) data, length,
 			       ty->msgtl_header.msgt_deallocate);
-		  ty = (void *) ty + sizeof (void *);
+		  ty = (void *) data + sizeof (void *);
 		}
 	    }
 #else  /* Untyped Mach IPC flavor. */