Re: Fwd:

Hello,

> diff --git a/gnumach/ipc/ipc_kmsg.c b/gnumach/ipc/ipc_kmsg.c
> index 8ab0e2e..19a76f3 100644
> --- a/gnumach/ipc/ipc_kmsg.c
> +++ b/gnumach/ipc/ipc_kmsg.c
> @@ -450,14 +450,21 @@ ipc_kmsg_clean_partial(
>  void
>  ipc_kmsg_free(ipc_kmsg_t kmsg)
>  {
> -	vm_size_t size = kmsg->ikm_size;
> +	vm_size_t size;
> +
> +	/* Early exit: NULL kmsg protection */
> +	if (unlikely(kmsg == IKM_NULL))
> +		return;

No, we don't want to silently ignore this. An ipc_kmsg_free caller is
not supposed to pass it a null pointer. Ignoring this means hiding an
more profound issue. And hiding bugs is the worse.

>  	switch (size) {
>  	    case IKM_SIZE_NETWORK:
>  		net_kmsg_put(kmsg);
>  		break;
>  
>  	    default:
> +		if (unlikely(size == 0 || size > IKM_SAVED_MAX))
> +			return;

Again, hiding bugs (and kfree already checks for 0)

>  		kfree((vm_offset_t) kmsg, size);
>  		break;
>  	}
>  }
> @@ -485,6 +492,11 @@ ipc_kmsg_get(
>  	if ((size < sizeof(mach_msg_user_header_t)) || mach_msg_user_is_misaligned(size))
>  		return MACH_SEND_MSG_TOO_SMALL;
>  
> +	/* Bounds check & integer overflow protection */
> +	if (unlikely(size > IKM_SAVED_MAX || 

What is that constant?

> +		     size > (MACH_MSG_SIZE_MAX / IKM_EXPAND_FACTOR)))

That does not really protect the kernel, user would still be able to
give a *very* large value.

> +		return MACH_SEND_NO_BUFFER;
> +
>  	if (ksize <= IKM_SAVED_MSG_SIZE) {
>  		kmsg = ikm_cache_alloc();
>  		if (kmsg == IKM_NULL)
> @@ -497,7 +509,7 @@ ipc_kmsg_get(
>  	}
>  
>  	if (copyinmsg(msg, &kmsg->ikm_header, size, kmsg->ikm_size)) {
> -		ikm_free(kmsg);
> +		ipc_kmsg_free(kmsg);

Why? It's the same.

Samuel