[PATCH] evh_bytechan: fix out of bounds accesses

[Michael Ellerman]

Stephen Rothwell <sfr at canb.auug.org.au> writes:
> ev_byte_channel_send() assumes that its third argument is a 16 byte array.
> Some places where it is called it may not be (or we can't easily tell
> if it is).  Newer compilers have started producing warnings about this,
> so make sure we actually pass a 16 byte array.
>
> There may be more elegant solutions to this, but the driver is quite
> old and hasn't been updated in many years.
...
> Fixes: dcd83aaff1c8 ("tty/powerpc: introduce the ePAPR embedded hypervisor byte channel driver")
> Cc: Michael Ellerman <mpe at ellerman.id.au>
> Cc: PowerPC Mailing List <linuxppc-dev at lists.ozlabs.org>
> Signed-off-by: Stephen Rothwell <sfr at canb.auug.org.au>
> ---
>  drivers/tty/ehv_bytechan.c | 20 +++++++++++++++++---
>  1 file changed, 17 insertions(+), 3 deletions(-)
>
> I have only build tested this change so it would be good to get some
> response from the PowerPC maintainers/developers.

I've never heard of it, and I have no idea how to test it.

It's not used by qemu, I guess there is/was a Freescale hypervisor that
used it.

But maybe it's time to remove it if it's not being maintained/used by
anyone?

cheers


> diff --git a/drivers/tty/ehv_bytechan.c b/drivers/tty/ehv_bytechan.c
> index 769e0a5d1dfc..546f80c49ae6 100644
> --- a/drivers/tty/ehv_bytechan.c
> +++ b/drivers/tty/ehv_bytechan.c
> @@ -136,6 +136,20 @@ static int find_console_handle(void)
>  	return 1;
>  }
>  
> +static unsigned int local_ev_byte_channel_send(unsigned int handle,
> +        unsigned int *count, const char *p)
> +{
> +	char buffer[EV_BYTE_CHANNEL_MAX_BYTES];
> +	unsigned int c = *count;
> +
> +	if (c < sizeof(buffer)) {
> +		memcpy(buffer, p, c);
> +		memset(&buffer[c], 0, sizeof(buffer) - c);
> +		p = buffer;
> +	}
> +	return ev_byte_channel_send(handle, count, p);
> +}
> +
>  /*************************** EARLY CONSOLE DRIVER ***************************/
>  
>  #ifdef CONFIG_PPC_EARLY_DEBUG_EHV_BC
> @@ -154,7 +168,7 @@ static void byte_channel_spin_send(const char data)
>  
>  	do {
>  		count = 1;
> -		ret = ev_byte_channel_send(CONFIG_PPC_EARLY_DEBUG_EHV_BC_HANDLE,
> +		ret = local_ev_byte_channel_send(CONFIG_PPC_EARLY_DEBUG_EHV_BC_HANDLE,
>  					   &count, &data);
>  	} while (ret == EV_EAGAIN);
>  }
> @@ -221,7 +235,7 @@ static int ehv_bc_console_byte_channel_send(unsigned int handle, const char *s,
>  	while (count) {
>  		len = min_t(unsigned int, count, EV_BYTE_CHANNEL_MAX_BYTES);
>  		do {
> -			ret = ev_byte_channel_send(handle, &len, s);
> +			ret = local_ev_byte_channel_send(handle, &len, s);
>  		} while (ret == EV_EAGAIN);
>  		count -= len;
>  		s += len;
> @@ -401,7 +415,7 @@ static void ehv_bc_tx_dequeue(struct ehv_bc_data *bc)
>  			    CIRC_CNT_TO_END(bc->head, bc->tail, BUF_SIZE),
>  			    EV_BYTE_CHANNEL_MAX_BYTES);
>  
> -		ret = ev_byte_channel_send(bc->handle, &len, bc->buf + bc->tail);
> +		ret = local_ev_byte_channel_send(bc->handle, &len, bc->buf + bc->tail);
>  
>  		/* 'len' is valid only if the return code is 0 or EV_EAGAIN */
>  		if (!ret || (ret == EV_EAGAIN))
> -- 
> 2.25.0.rc1
>
> -- 
> Cheers,
> Stephen Rothwell