[PATCH] evh_bytechan: fix out of bounds accesses

[PATCH] evh_bytechan: fix out of bounds accesses

[Stephen Rothwell]

[-- Attachment #1: Type: text/plain, Size: 11388 bytes --]

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.

The warnings (from a powerpc allyesconfig build) are:

In file included from include/linux/byteorder/big_endian.h:5,
                 from arch/powerpc/include/uapi/asm/byteorder.h:14,
                 from include/asm-generic/bitops/le.h:6,
                 from arch/powerpc/include/asm/bitops.h:250,
                 from include/linux/bitops.h:29,
                 from include/linux/kernel.h:12,
                 from include/asm-generic/bug.h:19,
                 from arch/powerpc/include/asm/bug.h:109,
                 from include/linux/bug.h:5,
                 from include/linux/mmdebug.h:5,
                 from include/linux/gfp.h:5,
                 from include/linux/slab.h:15,
                 from drivers/tty/ehv_bytechan.c:24:
drivers/tty/ehv_bytechan.c: In function ‘ehv_bc_udbg_putc’:
arch/powerpc/include/asm/epapr_hcalls.h:298:20: warning: array subscript 1 is outside array bounds of ‘const char[1]’ [-Warray-bounds]
  298 |  r6 = be32_to_cpu(p[1]);
include/uapi/linux/byteorder/big_endian.h:40:51: note: in definition of macro ‘__be32_to_cpu’
   40 | #define __be32_to_cpu(x) ((__force __u32)(__be32)(x))
      |                                                   ^
arch/powerpc/include/asm/epapr_hcalls.h:298:7: note: in expansion of macro ‘be32_to_cpu’
  298 |  r6 = be32_to_cpu(p[1]);
      |       ^~~~~~~~~~~
drivers/tty/ehv_bytechan.c:166:13: note: while referencing ‘data’
  166 | static void ehv_bc_udbg_putc(char c)
      |             ^~~~~~~~~~~~~~~~
In file included from include/linux/byteorder/big_endian.h:5,
                 from arch/powerpc/include/uapi/asm/byteorder.h:14,
                 from include/asm-generic/bitops/le.h:6,
                 from arch/powerpc/include/asm/bitops.h:250,
                 from include/linux/bitops.h:29,
                 from include/linux/kernel.h:12,
                 from include/asm-generic/bug.h:19,
                 from arch/powerpc/include/asm/bug.h:109,
                 from include/linux/bug.h:5,
                 from include/linux/mmdebug.h:5,
                 from include/linux/gfp.h:5,
                 from include/linux/slab.h:15,
                 from drivers/tty/ehv_bytechan.c:24:
arch/powerpc/include/asm/epapr_hcalls.h:299:20: warning: array subscript 2 is outside array bounds of ‘const char[1]’ [-Warray-bounds]
  299 |  r7 = be32_to_cpu(p[2]);
include/uapi/linux/byteorder/big_endian.h:40:51: note: in definition of macro ‘__be32_to_cpu’
   40 | #define __be32_to_cpu(x) ((__force __u32)(__be32)(x))
      |                                                   ^
arch/powerpc/include/asm/epapr_hcalls.h:299:7: note: in expansion of macro ‘be32_to_cpu’
  299 |  r7 = be32_to_cpu(p[2]);
      |       ^~~~~~~~~~~
drivers/tty/ehv_bytechan.c:166:13: note: while referencing ‘data’
  166 | static void ehv_bc_udbg_putc(char c)
      |             ^~~~~~~~~~~~~~~~
In file included from include/linux/byteorder/big_endian.h:5,
                 from arch/powerpc/include/uapi/asm/byteorder.h:14,
                 from include/asm-generic/bitops/le.h:6,
                 from arch/powerpc/include/asm/bitops.h:250,
                 from include/linux/bitops.h:29,
                 from include/linux/kernel.h:12,
                 from include/asm-generic/bug.h:19,
                 from arch/powerpc/include/asm/bug.h:109,
                 from include/linux/bug.h:5,
                 from include/linux/mmdebug.h:5,
                 from include/linux/gfp.h:5,
                 from include/linux/slab.h:15,
                 from drivers/tty/ehv_bytechan.c:24:
arch/powerpc/include/asm/epapr_hcalls.h:300:20: warning: array subscript 3 is outside array bounds of ‘const char[1]’ [-Warray-bounds]
  300 |  r8 = be32_to_cpu(p[3]);
include/uapi/linux/byteorder/big_endian.h:40:51: note: in definition of macro ‘__be32_to_cpu’
   40 | #define __be32_to_cpu(x) ((__force __u32)(__be32)(x))
      |                                                   ^
arch/powerpc/include/asm/epapr_hcalls.h:300:7: note: in expansion of macro ‘be32_to_cpu’
  300 |  r8 = be32_to_cpu(p[3]);
      |       ^~~~~~~~~~~
drivers/tty/ehv_bytechan.c:166:13: note: while referencing ‘data’
  166 | static void ehv_bc_udbg_putc(char c)
      |             ^~~~~~~~~~~~~~~~
In file included from include/linux/byteorder/big_endian.h:5,
                 from arch/powerpc/include/uapi/asm/byteorder.h:14,
                 from include/asm-generic/bitops/le.h:6,
                 from arch/powerpc/include/asm/bitops.h:250,
                 from include/linux/bitops.h:29,
                 from include/linux/kernel.h:12,
                 from include/asm-generic/bug.h:19,
                 from arch/powerpc/include/asm/bug.h:109,
                 from include/linux/bug.h:5,
                 from include/linux/mmdebug.h:5,
                 from include/linux/gfp.h:5,
                 from include/linux/slab.h:15,
                 from drivers/tty/ehv_bytechan.c:24:
arch/powerpc/include/asm/epapr_hcalls.h:298:20: warning: array subscript 1 is outside array bounds of ‘const char[1]’ [-Warray-bounds]
  298 |  r6 = be32_to_cpu(p[1]);
include/uapi/linux/byteorder/big_endian.h:40:51: note: in definition of macro ‘__be32_to_cpu’
   40 | #define __be32_to_cpu(x) ((__force __u32)(__be32)(x))
      |                                                   ^
arch/powerpc/include/asm/epapr_hcalls.h:298:7: note: in expansion of macro ‘be32_to_cpu’
  298 |  r6 = be32_to_cpu(p[1]);
      |       ^~~~~~~~~~~
drivers/tty/ehv_bytechan.c:166:13: note: while referencing ‘data’
  166 | static void ehv_bc_udbg_putc(char c)
      |             ^~~~~~~~~~~~~~~~
In file included from include/linux/byteorder/big_endian.h:5,
                 from arch/powerpc/include/uapi/asm/byteorder.h:14,
                 from include/asm-generic/bitops/le.h:6,
                 from arch/powerpc/include/asm/bitops.h:250,
                 from include/linux/bitops.h:29,
                 from include/linux/kernel.h:12,
                 from include/asm-generic/bug.h:19,
                 from arch/powerpc/include/asm/bug.h:109,
                 from include/linux/bug.h:5,
                 from include/linux/mmdebug.h:5,
                 from include/linux/gfp.h:5,
                 from include/linux/slab.h:15,
                 from drivers/tty/ehv_bytechan.c:24:
arch/powerpc/include/asm/epapr_hcalls.h:299:20: warning: array subscript 2 is outside array bounds of ‘const char[1]’ [-Warray-bounds]
  299 |  r7 = be32_to_cpu(p[2]);
include/uapi/linux/byteorder/big_endian.h:40:51: note: in definition of macro ‘__be32_to_cpu’
   40 | #define __be32_to_cpu(x) ((__force __u32)(__be32)(x))
      |                                                   ^
arch/powerpc/include/asm/epapr_hcalls.h:299:7: note: in expansion of macro ‘be32_to_cpu’
  299 |  r7 = be32_to_cpu(p[2]);
      |       ^~~~~~~~~~~
drivers/tty/ehv_bytechan.c:166:13: note: while referencing ‘data’
  166 | static void ehv_bc_udbg_putc(char c)
      |             ^~~~~~~~~~~~~~~~
In file included from include/linux/byteorder/big_endian.h:5,
                 from arch/powerpc/include/uapi/asm/byteorder.h:14,
                 from include/asm-generic/bitops/le.h:6,
                 from arch/powerpc/include/asm/bitops.h:250,
                 from include/linux/bitops.h:29,
                 from include/linux/kernel.h:12,
                 from include/asm-generic/bug.h:19,
                 from arch/powerpc/include/asm/bug.h:109,
                 from include/linux/bug.h:5,
                 from include/linux/mmdebug.h:5,
                 from include/linux/gfp.h:5,
                 from include/linux/slab.h:15,
                 from drivers/tty/ehv_bytechan.c:24:
arch/powerpc/include/asm/epapr_hcalls.h:300:20: warning: array subscript 3 is outside array bounds of ‘const char[1]’ [-Warray-bounds]
  300 |  r8 = be32_to_cpu(p[3]);
include/uapi/linux/byteorder/big_endian.h:40:51: note: in definition of macro ‘__be32_to_cpu’
   40 | #define __be32_to_cpu(x) ((__force __u32)(__be32)(x))
      |                                                   ^
arch/powerpc/include/asm/epapr_hcalls.h:300:7: note: in expansion of macro ‘be32_to_cpu’
  300 |  r8 = be32_to_cpu(p[3]);
      |       ^~~~~~~~~~~
drivers/tty/ehv_bytechan.c:166:13: note: while referencing ‘data’
  166 | static void ehv_bc_udbg_putc(char c)
      |             ^~~~~~~~~~~~~~~~

Fixes: dcd83aaff1c8 ("tty/powerpc: introduce the ePAPR embedded hypervisor byte channel driver")
Cc: Michael Ellerman <mpe@ellerman.id.au>
Cc: PowerPC Mailing List <linuxppc-dev@lists.ozlabs.org>
Signed-off-by: Stephen Rothwell <sfr@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.

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

[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

Re: [PATCH] evh_bytechan: fix out of bounds accesses

[Michael Ellerman]

Stephen Rothwell <sfr@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@ellerman.id.au>
> Cc: PowerPC Mailing List <linuxppc-dev@lists.ozlabs.org>
> Signed-off-by: Stephen Rothwell <sfr@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

Re: [PATCH] evh_bytechan: fix out of bounds accesses

[Timur Tabi]

On 1/13/20 6:26 AM, Michael Ellerman wrote:
> 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.

Yes, there is/was a Freescale hypervisor that I and a few others worked 
on.  I've added a couple people on CC that might be able to tell the 
current disposition of it.

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

I wouldn't be completely opposed to that if there really are no more 
users.  There really weren't any users even when I wrote the driver.

I haven't had a chance to study the patch, but my first instinct is that 
there's got to be a better way to fix this than introducing a memcpy.

Re: [PATCH] evh_bytechan: fix out of bounds accesses

[Laurentiu Tudor]

Hello,

On 13.01.2020 15:48, Timur Tabi wrote:
> On 1/13/20 6:26 AM, Michael Ellerman wrote:
>> 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.
> 
> Yes, there is/was a Freescale hypervisor that I and a few others worked 
> on.  I've added a couple people on CC that might be able to tell the 
> current disposition of it.

More info on this: it's opensource and it's published here [1]. We still 
claim to maintain it but there wasn't much activity past years, as one 
might notice.

>> But maybe it's time to remove it if it's not being maintained/used by
>> anyone?
> 
> I wouldn't be completely opposed to that if there really are no more 
> users.  There really weren't any users even when I wrote the driver.

There are a few users that I know of, but I can't tell if that's enough 
to justify keeping the driver.

[1] https://source.codeaurora.org/external/qoriq/qoriq-yocto-sdk/hypervisor/

---
Best Regards, Laurentiu

Re: [PATCH] evh_bytechan: fix out of bounds accesses

[Timur Tabi]

On 1/13/20 8:34 AM, Laurentiu Tudor wrote:
> There are a few users that I know of, but I can't tell if that's enough
> to justify keeping the driver.
> 
> [1]https://source.codeaurora.org/external/qoriq/qoriq-yocto-sdk/hypervisor/

IIRC, the driver is the only reasonable way to get a serial console from 
a guest.  So if there are users of the hypervisor, then I think there's 
a good chance at least one is using the byte channel driver.

Re: [PATCH] evh_bytechan: fix out of bounds accesses

[Timur Tabi]

On Thu, Jan 9, 2020 at 1:41 AM Stephen Rothwell <sfr@canb.auug.org.au> wrote:
>
> 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.

...

> +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);
> +}

Why not simply correct the parameters of ev_byte_channel_send?

static inline unsigned int ev_byte_channel_send(unsigned int handle,
-unsigned int *count, const char buffer[EV_BYTE_CHANNEL_MAX_BYTES])
+unsigned int *count, const char *buffer)

Back then, I probably thought I was just being clever with this code,
but I realize now that it doesn't make sense to do the way I did.

Re: [PATCH] evh_bytechan: fix out of bounds accesses

[Stephen Rothwell]

[-- Attachment #1: Type: text/plain, Size: 906 bytes --]

Hi Timur,

On Mon, 13 Jan 2020 10:03:18 -0600 Timur Tabi <timur@kernel.org> wrote:
>
> Why not simply correct the parameters of ev_byte_channel_send?
> 
> static inline unsigned int ev_byte_channel_send(unsigned int handle,
> -unsigned int *count, const char buffer[EV_BYTE_CHANNEL_MAX_BYTES])
> +unsigned int *count, const char *buffer)
> 
> Back then, I probably thought I was just being clever with this code,
> but I realize now that it doesn't make sense to do the way I did.

The problem is not really the declaration, the problem is that
ev_byte_channel_send always accesses 16 bytes from the buffer and it is
not always passed a buffer that long (in one case it is passed a
pointer to a single byte).  So the alternative to the memcpy approach I
have take is to complicate ev_byte_channel_send so that only accesses
count bytes from the buffer.

-- 
Cheers,
Stephen Rothwell

[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

Re: [PATCH] evh_bytechan: fix out of bounds accesses

[Timur Tabi]

On 1/13/20 2:25 PM, Stephen Rothwell wrote:
> The problem is not really the declaration, the problem is that
> ev_byte_channel_send always accesses 16 bytes from the buffer and it is
> not always passed a buffer that long (in one case it is passed a
> pointer to a single byte).  So the alternative to the memcpy approach I
> have take is to complicate ev_byte_channel_send so that only accesses
> count bytes from the buffer.

Ah, I see now.  This is all coming back to me.

I would prefer that ev_byte_channel_send() is updated to access only 
'count' bytes.  If that means adding a memcpy to the 
ev_byte_channel_send() itself, then so be it.  Trying to figure out how 
to stuff n bytes into 4 32-bit registers is probably not worth the effort.

Re: [PATCH] evh_bytechan: fix out of bounds accesses

[Michael Ellerman]

Laurentiu Tudor <laurentiu.tudor@nxp.com> writes:
> Hello,
>
> On 13.01.2020 15:48, Timur Tabi wrote:
>> On 1/13/20 6:26 AM, Michael Ellerman wrote:
>>> 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.
>> 
>> Yes, there is/was a Freescale hypervisor that I and a few others worked 
>> on.  I've added a couple people on CC that might be able to tell the 
>> current disposition of it.
>
> More info on this: it's opensource and it's published here [1]. We still 
> claim to maintain it but there wasn't much activity past years, as one 
> might notice.
>
>>> But maybe it's time to remove it if it's not being maintained/used by
>>> anyone?
>> 
>> I wouldn't be completely opposed to that if there really are no more 
>> users.  There really weren't any users even when I wrote the driver.
>
> There are a few users that I know of, but I can't tell if that's enough 
> to justify keeping the driver.

It is, I don't want to remove code that people are actually using,
unless it's causing unsustainable maintenance burden.

But this should be easy enough to get fixed.

Could you or someone else at NXP volunteer to maintain this driver? That
shouldn't involve much work, other than fixing small things like this
warning.

cheers

Re: [PATCH] evh_bytechan: fix out of bounds accesses

[Timur Tabi]

On 1/13/20 7:10 PM, Timur Tabi wrote:
> 
> I would prefer that ev_byte_channel_send() is updated to access only 
> 'count' bytes.  If that means adding a memcpy to the 
> ev_byte_channel_send() itself, then so be it.  Trying to figure out how 
> to stuff n bytes into 4 32-bit registers is probably not worth the effort.

Looks like ev_byte_channel_receive() has the same bug, but in reverse.

Re: [PATCH] evh_bytechan: fix out of bounds accesses

[Scott Wood]

On Mon, 2020-01-13 at 19:13 -0600, Timur Tabi wrote:
> On 1/13/20 7:10 PM, Timur Tabi wrote:
> > I would prefer that ev_byte_channel_send() is updated to access only 
> > 'count' bytes.  If that means adding a memcpy to the 
> > ev_byte_channel_send() itself, then so be it.  Trying to figure out how 
> > to stuff n bytes into 4 32-
> > bit registers is probably not worth the effort.
> 
> Looks like ev_byte_channel_receive() has the same bug, but in reverse.

It only has one user, which always passes in a 16-byte buffer.

-Scott

Re: [PATCH] evh_bytechan: fix out of bounds accesses

[Stephen Rothwell]

[-- Attachment #1: Type: text/plain, Size: 13861 bytes --]

Hi Timur,

On Mon, 13 Jan 2020 19:10:11 -0600 Timur Tabi <timur@kernel.org> wrote:
>
> On 1/13/20 2:25 PM, Stephen Rothwell wrote:
> > The problem is not really the declaration, the problem is that
> > ev_byte_channel_send always accesses 16 bytes from the buffer and it is
> > not always passed a buffer that long (in one case it is passed a
> > pointer to a single byte).  So the alternative to the memcpy approach I
> > have take is to complicate ev_byte_channel_send so that only accesses
> > count bytes from the buffer.  
> 
> Ah, I see now.  This is all coming back to me.
> 
> I would prefer that ev_byte_channel_send() is updated to access only 
> 'count' bytes.  If that means adding a memcpy to the 
> ev_byte_channel_send() itself, then so be it.  Trying to figure out how 
> to stuff n bytes into 4 32-bit registers is probably not worth the effort.

Like this (I have compile tested this):

From: Stephen Rothwell <sfr@canb.auug.org.au>
Date: Thu, 9 Jan 2020 18:23:48 +1100
Subject: [PATCH v2] evh_bytechan: fix out of bounds accesses

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 copy the bytes to send into a local array if the passed length is
to short.

Since all the calls of ev_byte_channel_send() are in one file, lets move
it there from the header file and let the compiler decide if it wants
to inline it.

The warnings are:

In file included from include/linux/byteorder/big_endian.h:5,
                 from arch/powerpc/include/uapi/asm/byteorder.h:14,
                 from include/asm-generic/bitops/le.h:6,
                 from arch/powerpc/include/asm/bitops.h:250,
                 from include/linux/bitops.h:29,
                 from include/linux/kernel.h:12,
                 from include/asm-generic/bug.h:19,
                 from arch/powerpc/include/asm/bug.h:109,
                 from include/linux/bug.h:5,
                 from include/linux/mmdebug.h:5,
                 from include/linux/gfp.h:5,
                 from include/linux/slab.h:15,
                 from drivers/tty/ehv_bytechan.c:24:
drivers/tty/ehv_bytechan.c: In function ‘ehv_bc_udbg_putc’:
arch/powerpc/include/asm/epapr_hcalls.h:298:20: warning: array subscript 1 is outside array bounds of ‘const char[1]’ [-Warray-bounds]
  298 |  r6 = be32_to_cpu(p[1]);
include/uapi/linux/byteorder/big_endian.h:40:51: note: in definition of macro ‘__be32_to_cpu’
   40 | #define __be32_to_cpu(x) ((__force __u32)(__be32)(x))
      |                                                   ^
arch/powerpc/include/asm/epapr_hcalls.h:298:7: note: in expansion of macro ‘be32_to_cpu’
  298 |  r6 = be32_to_cpu(p[1]);
      |       ^~~~~~~~~~~
drivers/tty/ehv_bytechan.c:166:13: note: while referencing ‘data’
  166 | static void ehv_bc_udbg_putc(char c)
      |             ^~~~~~~~~~~~~~~~
In file included from include/linux/byteorder/big_endian.h:5,
                 from arch/powerpc/include/uapi/asm/byteorder.h:14,
                 from include/asm-generic/bitops/le.h:6,
                 from arch/powerpc/include/asm/bitops.h:250,
                 from include/linux/bitops.h:29,
                 from include/linux/kernel.h:12,
                 from include/asm-generic/bug.h:19,
                 from arch/powerpc/include/asm/bug.h:109,
                 from include/linux/bug.h:5,
                 from include/linux/mmdebug.h:5,
                 from include/linux/gfp.h:5,
                 from include/linux/slab.h:15,
                 from drivers/tty/ehv_bytechan.c:24:
arch/powerpc/include/asm/epapr_hcalls.h:299:20: warning: array subscript 2 is outside array bounds of ‘const char[1]’ [-Warray-bounds]
  299 |  r7 = be32_to_cpu(p[2]);
include/uapi/linux/byteorder/big_endian.h:40:51: note: in definition of macro ‘__be32_to_cpu’
   40 | #define __be32_to_cpu(x) ((__force __u32)(__be32)(x))
      |                                                   ^
arch/powerpc/include/asm/epapr_hcalls.h:299:7: note: in expansion of macro ‘be32_to_cpu’
  299 |  r7 = be32_to_cpu(p[2]);
      |       ^~~~~~~~~~~
drivers/tty/ehv_bytechan.c:166:13: note: while referencing ‘data’
  166 | static void ehv_bc_udbg_putc(char c)
      |             ^~~~~~~~~~~~~~~~
In file included from include/linux/byteorder/big_endian.h:5,
                 from arch/powerpc/include/uapi/asm/byteorder.h:14,
                 from include/asm-generic/bitops/le.h:6,
                 from arch/powerpc/include/asm/bitops.h:250,
                 from include/linux/bitops.h:29,
                 from include/linux/kernel.h:12,
                 from include/asm-generic/bug.h:19,
                 from arch/powerpc/include/asm/bug.h:109,
                 from include/linux/bug.h:5,
                 from include/linux/mmdebug.h:5,
                 from include/linux/gfp.h:5,
                 from include/linux/slab.h:15,
                 from drivers/tty/ehv_bytechan.c:24:
arch/powerpc/include/asm/epapr_hcalls.h:300:20: warning: array subscript 3 is outside array bounds of ‘const char[1]’ [-Warray-bounds]
  300 |  r8 = be32_to_cpu(p[3]);
include/uapi/linux/byteorder/big_endian.h:40:51: note: in definition of macro ‘__be32_to_cpu’
   40 | #define __be32_to_cpu(x) ((__force __u32)(__be32)(x))
      |                                                   ^
arch/powerpc/include/asm/epapr_hcalls.h:300:7: note: in expansion of macro ‘be32_to_cpu’
  300 |  r8 = be32_to_cpu(p[3]);
      |       ^~~~~~~~~~~
drivers/tty/ehv_bytechan.c:166:13: note: while referencing ‘data’
  166 | static void ehv_bc_udbg_putc(char c)
      |             ^~~~~~~~~~~~~~~~
In file included from include/linux/byteorder/big_endian.h:5,
                 from arch/powerpc/include/uapi/asm/byteorder.h:14,
                 from include/asm-generic/bitops/le.h:6,
                 from arch/powerpc/include/asm/bitops.h:250,
                 from include/linux/bitops.h:29,
                 from include/linux/kernel.h:12,
                 from include/asm-generic/bug.h:19,
                 from arch/powerpc/include/asm/bug.h:109,
                 from include/linux/bug.h:5,
                 from include/linux/mmdebug.h:5,
                 from include/linux/gfp.h:5,
                 from include/linux/slab.h:15,
                 from drivers/tty/ehv_bytechan.c:24:
arch/powerpc/include/asm/epapr_hcalls.h:298:20: warning: array subscript 1 is outside array bounds of ‘const char[1]’ [-Warray-bounds]
  298 |  r6 = be32_to_cpu(p[1]);
include/uapi/linux/byteorder/big_endian.h:40:51: note: in definition of macro ‘__be32_to_cpu’
   40 | #define __be32_to_cpu(x) ((__force __u32)(__be32)(x))
      |                                                   ^
arch/powerpc/include/asm/epapr_hcalls.h:298:7: note: in expansion of macro ‘be32_to_cpu’
  298 |  r6 = be32_to_cpu(p[1]);
      |       ^~~~~~~~~~~
drivers/tty/ehv_bytechan.c:166:13: note: while referencing ‘data’
  166 | static void ehv_bc_udbg_putc(char c)
      |             ^~~~~~~~~~~~~~~~
In file included from include/linux/byteorder/big_endian.h:5,
                 from arch/powerpc/include/uapi/asm/byteorder.h:14,
                 from include/asm-generic/bitops/le.h:6,
                 from arch/powerpc/include/asm/bitops.h:250,
                 from include/linux/bitops.h:29,
                 from include/linux/kernel.h:12,
                 from include/asm-generic/bug.h:19,
                 from arch/powerpc/include/asm/bug.h:109,
                 from include/linux/bug.h:5,
                 from include/linux/mmdebug.h:5,
                 from include/linux/gfp.h:5,
                 from include/linux/slab.h:15,
                 from drivers/tty/ehv_bytechan.c:24:
arch/powerpc/include/asm/epapr_hcalls.h:299:20: warning: array subscript 2 is outside array bounds of ‘const char[1]’ [-Warray-bounds]
  299 |  r7 = be32_to_cpu(p[2]);
include/uapi/linux/byteorder/big_endian.h:40:51: note: in definition of macro ‘__be32_to_cpu’
   40 | #define __be32_to_cpu(x) ((__force __u32)(__be32)(x))
      |                                                   ^
arch/powerpc/include/asm/epapr_hcalls.h:299:7: note: in expansion of macro ‘be32_to_cpu’
  299 |  r7 = be32_to_cpu(p[2]);
      |       ^~~~~~~~~~~
drivers/tty/ehv_bytechan.c:166:13: note: while referencing ‘data’
  166 | static void ehv_bc_udbg_putc(char c)
      |             ^~~~~~~~~~~~~~~~
In file included from include/linux/byteorder/big_endian.h:5,
                 from arch/powerpc/include/uapi/asm/byteorder.h:14,
                 from include/asm-generic/bitops/le.h:6,
                 from arch/powerpc/include/asm/bitops.h:250,
                 from include/linux/bitops.h:29,
                 from include/linux/kernel.h:12,
                 from include/asm-generic/bug.h:19,
                 from arch/powerpc/include/asm/bug.h:109,
                 from include/linux/bug.h:5,
                 from include/linux/mmdebug.h:5,
                 from include/linux/gfp.h:5,
                 from include/linux/slab.h:15,
                 from drivers/tty/ehv_bytechan.c:24:
arch/powerpc/include/asm/epapr_hcalls.h:300:20: warning: array subscript 3 is outside array bounds of ‘const char[1]’ [-Warray-bounds]
  300 |  r8 = be32_to_cpu(p[3]);
include/uapi/linux/byteorder/big_endian.h:40:51: note: in definition of macro ‘__be32_to_cpu’
   40 | #define __be32_to_cpu(x) ((__force __u32)(__be32)(x))
      |                                                   ^
arch/powerpc/include/asm/epapr_hcalls.h:300:7: note: in expansion of macro ‘be32_to_cpu’
  300 |  r8 = be32_to_cpu(p[3]);
      |       ^~~~~~~~~~~
drivers/tty/ehv_bytechan.c:166:13: note: while referencing ‘data’
  166 | static void ehv_bc_udbg_putc(char c)
      |             ^~~~~~~~~~~~~~~~

Signed-off-by: Stephen Rothwell <sfr@canb.auug.org.au>
---
 arch/powerpc/include/asm/epapr_hcalls.h | 42 ----------------------
 drivers/tty/ehv_bytechan.c              | 48 +++++++++++++++++++++++++
 2 files changed, 48 insertions(+), 42 deletions(-)

diff --git a/arch/powerpc/include/asm/epapr_hcalls.h b/arch/powerpc/include/asm/epapr_hcalls.h
index d3a7e36f1402..75c5943c9f85 100644
--- a/arch/powerpc/include/asm/epapr_hcalls.h
+++ b/arch/powerpc/include/asm/epapr_hcalls.h
@@ -268,48 +268,6 @@ static inline unsigned int ev_int_eoi(unsigned int interrupt)
 	return r3;
 }
 
-/**
- * ev_byte_channel_send - send characters to a byte stream
- * @handle: byte stream handle
- * @count: (input) num of chars to send, (output) num chars sent
- * @buffer: pointer to a 16-byte buffer
- *
- * @buffer must be at least 16 bytes long, because all 16 bytes will be
- * read from memory into registers, even if count < 16.
- *
- * Returns 0 for success, or an error code.
- */
-static inline unsigned int ev_byte_channel_send(unsigned int handle,
-	unsigned int *count, const char buffer[EV_BYTE_CHANNEL_MAX_BYTES])
-{
-	register uintptr_t r11 __asm__("r11");
-	register uintptr_t r3 __asm__("r3");
-	register uintptr_t r4 __asm__("r4");
-	register uintptr_t r5 __asm__("r5");
-	register uintptr_t r6 __asm__("r6");
-	register uintptr_t r7 __asm__("r7");
-	register uintptr_t r8 __asm__("r8");
-	const uint32_t *p = (const uint32_t *) buffer;
-
-	r11 = EV_HCALL_TOKEN(EV_BYTE_CHANNEL_SEND);
-	r3 = handle;
-	r4 = *count;
-	r5 = be32_to_cpu(p[0]);
-	r6 = be32_to_cpu(p[1]);
-	r7 = be32_to_cpu(p[2]);
-	r8 = be32_to_cpu(p[3]);
-
-	asm volatile("bl	epapr_hypercall_start"
-		: "+r" (r11), "+r" (r3),
-		  "+r" (r4), "+r" (r5), "+r" (r6), "+r" (r7), "+r" (r8)
-		: : EV_HCALL_CLOBBERS6
-	);
-
-	*count = r4;
-
-	return r3;
-}
-
 /**
  * ev_byte_channel_receive - fetch characters from a byte channel
  * @handle: byte channel handle
diff --git a/drivers/tty/ehv_bytechan.c b/drivers/tty/ehv_bytechan.c
index 769e0a5d1dfc..a5512745d0f9 100644
--- a/drivers/tty/ehv_bytechan.c
+++ b/drivers/tty/ehv_bytechan.c
@@ -136,6 +136,54 @@ static int find_console_handle(void)
 	return 1;
 }
 
+/**
+ * ev_byte_channel_send - send characters to a byte stream
+ * @handle: byte stream handle
+ * @count: (input) num of chars to send, (output) num chars sent
+ * @bp: pointer to chars to send
+ *
+ * Returns 0 for success, or an error code.
+ */
+static unsigned int ev_byte_channel_send(unsigned int handle,
+	unsigned int *count, const char *bp)
+{
+	register uintptr_t r11 __asm__("r11");
+	register uintptr_t r3 __asm__("r3");
+	register uintptr_t r4 __asm__("r4");
+	register uintptr_t r5 __asm__("r5");
+	register uintptr_t r6 __asm__("r6");
+	register uintptr_t r7 __asm__("r7");
+	register uintptr_t r8 __asm__("r8");
+	const uint32_t *p;
+	char buffer[EV_BYTE_CHANNEL_MAX_BYTES];
+	unsigned int c = *count;
+
+	if (c < sizeof(buffer)) {
+		memcpy(buffer, bp, c);
+		memset(&buffer[c], 0, sizeof(buffer) - c);
+		p = (const uint32_t *)buffer;
+	} else {
+		p = (const uint32_t *)bp;
+	}
+	r11 = EV_HCALL_TOKEN(EV_BYTE_CHANNEL_SEND);
+	r3 = handle;
+	r4 = *count;
+	r5 = be32_to_cpu(p[0]);
+	r6 = be32_to_cpu(p[1]);
+	r7 = be32_to_cpu(p[2]);
+	r8 = be32_to_cpu(p[3]);
+
+	asm volatile("bl	epapr_hypercall_start"
+		: "+r" (r11), "+r" (r3),
+		  "+r" (r4), "+r" (r5), "+r" (r6), "+r" (r7), "+r" (r8)
+		: : EV_HCALL_CLOBBERS6
+	);
+
+	*count = r4;
+
+	return r3;
+}
+
 /*************************** EARLY CONSOLE DRIVER ***************************/
 
 #ifdef CONFIG_PPC_EARLY_DEBUG_EHV_BC
-- 
2.25.0.rc2

-- 
Cheers,
Stephen Rothwell

[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

Re: [PATCH] evh_bytechan: fix out of bounds accesses

[Segher Boessenkool]

On Mon, Jan 13, 2020 at 07:10:11PM -0600, Timur Tabi wrote:
> Ah, I see now.  This is all coming back to me.
> 
> I would prefer that ev_byte_channel_send() is updated to access only 
> 'count' bytes.  If that means adding a memcpy to the 
> ev_byte_channel_send() itself, then so be it.  Trying to figure out how 
> to stuff n bytes into 4 32-bit registers is probably not worth the effort.

You have no working lswx I suppose?  :-)

/me slowly backs away


Segher

Re: [PATCH] evh_bytechan: fix out of bounds accesses

[Laurentiu Tudor]

On 14.01.2020 03:10, Michael Ellerman wrote:
> Laurentiu Tudor <laurentiu.tudor@nxp.com> writes:
>> Hello,
>>
>> On 13.01.2020 15:48, Timur Tabi wrote:
>>> On 1/13/20 6:26 AM, Michael Ellerman wrote:
>>>> 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.
>>>
>>> Yes, there is/was a Freescale hypervisor that I and a few others worked
>>> on.  I've added a couple people on CC that might be able to tell the
>>> current disposition of it.
>>
>> More info on this: it's opensource and it's published here [1]. We still
>> claim to maintain it but there wasn't much activity past years, as one
>> might notice.
>>
>>>> But maybe it's time to remove it if it's not being maintained/used by
>>>> anyone?
>>>
>>> I wouldn't be completely opposed to that if there really are no more
>>> users.  There really weren't any users even when I wrote the driver.
>>
>> There are a few users that I know of, but I can't tell if that's enough
>> to justify keeping the driver.
> 
> It is, I don't want to remove code that people are actually using,
> unless it's causing unsustainable maintenance burden.
> 
> But this should be easy enough to get fixed.

Right. I see that Stephen already came up with a proposal for a fix.

> Could you or someone else at NXP volunteer to maintain this driver? That
> shouldn't involve much work, other than fixing small things like this
> warning.

I can offer myself. I'll send a MAINTAINERS patch if nobody is against it.

---
Best Regards, Laurentiu

Re: [PATCH] evh_bytechan: fix out of bounds accesses

[Timur Tabi]

On 1/14/20 3:18 AM, Laurentiu Tudor wrote:
> I can offer myself. I'll send a MAINTAINERS patch if nobody is against it.

Yes, please do.  I'm always available if you have any questions on the code.

Re: [PATCH] evh_bytechan: fix out of bounds accesses

[Timur Tabi]

On 1/14/20 2:29 AM, Segher Boessenkool wrote:
> You have no working lswx I suppose?:-)

I don't know if the P4080 supports lswx, but it does, than that would be 
an elegant way to fix this bug.

Re: [PATCH] evh_bytechan: fix out of bounds accesses

[Segher Boessenkool]

On Tue, Jan 14, 2020 at 05:53:41AM -0600, Timur Tabi wrote:
> On 1/14/20 2:29 AM, Segher Boessenkool wrote:
> >You have no working lswx I suppose?:-)
> 
> I don't know if the P4080 supports lswx, but it does, than that would be 
> an elegant way to fix this bug.

No e500 version supports it.  Many other CPUs do not allow it in little-
endian mode, or not in real mode, etc.


Segher

RE: [PATCH] evh_bytechan: fix out of bounds accesses

[Laurentiu Tudor]

> -----Original Message-----
> From: Linuxppc-dev <linuxppc-dev-
> bounces+laurentiu.tudor=nxp.com@lists.ozlabs.org> On Behalf Of Stephen
> Rothwell
> Sent: Tuesday, January 14, 2020 8:32 AM
> To: Timur Tabi <timur@kernel.org>
> Cc: b08248@gmail.com; Greg Kroah-Hartman <gregkh@linuxfoundation.org>;
> Jiri Slaby <jslaby@suse.com>; York Sun <york.sun@nxp.com>; PowerPC Mailing
> List <linuxppc-dev@lists.ozlabs.org>; swood@redhat.com
> Subject: Re: [PATCH] evh_bytechan: fix out of bounds accesses
> 
> Hi Timur,
> 
> On Mon, 13 Jan 2020 19:10:11 -0600 Timur Tabi <timur@kernel.org> wrote:
> >
> > On 1/13/20 2:25 PM, Stephen Rothwell wrote:
> > > The problem is not really the declaration, the problem is that
> > > ev_byte_channel_send always accesses 16 bytes from the buffer and it
> is
> > > not always passed a buffer that long (in one case it is passed a
> > > pointer to a single byte).  So the alternative to the memcpy approach
> I
> > > have take is to complicate ev_byte_channel_send so that only accesses
> > > count bytes from the buffer.
> >
> > Ah, I see now.  This is all coming back to me.
> >
> > I would prefer that ev_byte_channel_send() is updated to access only
> > 'count' bytes.  If that means adding a memcpy to the
> > ev_byte_channel_send() itself, then so be it.  Trying to figure out how
> > to stuff n bytes into 4 32-bit registers is probably not worth the
> effort.
> 
> Like this (I have compile tested this):
> 
> From: Stephen Rothwell <sfr@canb.auug.org.au>
> Date: Thu, 9 Jan 2020 18:23:48 +1100
> Subject: [PATCH v2] evh_bytechan: fix out of bounds accesses
> 
> 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 copy the bytes to send into a local array if the passed length is
> to short.
> 
> Since all the calls of ev_byte_channel_send() are in one file, lets move
> it there from the header file and let the compiler decide if it wants
> to inline it.
> 
> The warnings are:
> 
> In file included from include/linux/byteorder/big_endian.h:5,
>                  from arch/powerpc/include/uapi/asm/byteorder.h:14,
>                  from include/asm-generic/bitops/le.h:6,
>                  from arch/powerpc/include/asm/bitops.h:250,
>                  from include/linux/bitops.h:29,
>                  from include/linux/kernel.h:12,
>                  from include/asm-generic/bug.h:19,
>                  from arch/powerpc/include/asm/bug.h:109,
>                  from include/linux/bug.h:5,
>                  from include/linux/mmdebug.h:5,
>                  from include/linux/gfp.h:5,
>                  from include/linux/slab.h:15,
>                  from drivers/tty/ehv_bytechan.c:24:
> drivers/tty/ehv_bytechan.c: In function ‘ehv_bc_udbg_putc’:
> arch/powerpc/include/asm/epapr_hcalls.h:298:20: warning: array subscript 1
> is outside array bounds of ‘const char[1]’ [-Warray-bounds]
>   298 |  r6 = be32_to_cpu(p[1]);
> include/uapi/linux/byteorder/big_endian.h:40:51: note: in definition of
> macro ‘__be32_to_cpu’
>    40 | #define __be32_to_cpu(x) ((__force __u32)(__be32)(x))
>       |                                                   ^
> arch/powerpc/include/asm/epapr_hcalls.h:298:7: note: in expansion of macro
> ‘be32_to_cpu’
>   298 |  r6 = be32_to_cpu(p[1]);
>       |       ^~~~~~~~~~~
> drivers/tty/ehv_bytechan.c:166:13: note: while referencing ‘data’
>   166 | static void ehv_bc_udbg_putc(char c)
>       |             ^~~~~~~~~~~~~~~~
> In file included from include/linux/byteorder/big_endian.h:5,
>                  from arch/powerpc/include/uapi/asm/byteorder.h:14,
>                  from include/asm-generic/bitops/le.h:6,
>                  from arch/powerpc/include/asm/bitops.h:250,
>                  from include/linux/bitops.h:29,
>                  from include/linux/kernel.h:12,
>                  from include/asm-generic/bug.h:19,
>                  from arch/powerpc/include/asm/bug.h:109,
>                  from include/linux/bug.h:5,
>                  from include/linux/mmdebug.h:5,
>                  from include/linux/gfp.h:5,
>                  from include/linux/slab.h:15,
>                  from drivers/tty/ehv_bytechan.c:24:
> arch/powerpc/include/asm/epapr_hcalls.h:299:20: warning: array subscript 2
> is outside array bounds of ‘const char[1]’ [-Warray-bounds]
>   299 |  r7 = be32_to_cpu(p[2]);
> include/uapi/linux/byteorder/big_endian.h:40:51: note: in definition of
> macro ‘__be32_to_cpu’
>    40 | #define __be32_to_cpu(x) ((__force __u32)(__be32)(x))
>       |                                                   ^
> arch/powerpc/include/asm/epapr_hcalls.h:299:7: note: in expansion of macro
> ‘be32_to_cpu’
>   299 |  r7 = be32_to_cpu(p[2]);
>       |       ^~~~~~~~~~~
> drivers/tty/ehv_bytechan.c:166:13: note: while referencing ‘data’
>   166 | static void ehv_bc_udbg_putc(char c)
>       |             ^~~~~~~~~~~~~~~~
> In file included from include/linux/byteorder/big_endian.h:5,
>                  from arch/powerpc/include/uapi/asm/byteorder.h:14,
>                  from include/asm-generic/bitops/le.h:6,
>                  from arch/powerpc/include/asm/bitops.h:250,
>                  from include/linux/bitops.h:29,
>                  from include/linux/kernel.h:12,
>                  from include/asm-generic/bug.h:19,
>                  from arch/powerpc/include/asm/bug.h:109,
>                  from include/linux/bug.h:5,
>                  from include/linux/mmdebug.h:5,
>                  from include/linux/gfp.h:5,
>                  from include/linux/slab.h:15,
>                  from drivers/tty/ehv_bytechan.c:24:
> arch/powerpc/include/asm/epapr_hcalls.h:300:20: warning: array subscript 3
> is outside array bounds of ‘const char[1]’ [-Warray-bounds]
>   300 |  r8 = be32_to_cpu(p[3]);
> include/uapi/linux/byteorder/big_endian.h:40:51: note: in definition of
> macro ‘__be32_to_cpu’
>    40 | #define __be32_to_cpu(x) ((__force __u32)(__be32)(x))
>       |                                                   ^
> arch/powerpc/include/asm/epapr_hcalls.h:300:7: note: in expansion of macro
> ‘be32_to_cpu’
>   300 |  r8 = be32_to_cpu(p[3]);
>       |       ^~~~~~~~~~~
> drivers/tty/ehv_bytechan.c:166:13: note: while referencing ‘data’
>   166 | static void ehv_bc_udbg_putc(char c)
>       |             ^~~~~~~~~~~~~~~~
> In file included from include/linux/byteorder/big_endian.h:5,
>                  from arch/powerpc/include/uapi/asm/byteorder.h:14,
>                  from include/asm-generic/bitops/le.h:6,
>                  from arch/powerpc/include/asm/bitops.h:250,
>                  from include/linux/bitops.h:29,
>                  from include/linux/kernel.h:12,
>                  from include/asm-generic/bug.h:19,
>                  from arch/powerpc/include/asm/bug.h:109,
>                  from include/linux/bug.h:5,
>                  from include/linux/mmdebug.h:5,
>                  from include/linux/gfp.h:5,
>                  from include/linux/slab.h:15,
>                  from drivers/tty/ehv_bytechan.c:24:
> arch/powerpc/include/asm/epapr_hcalls.h:298:20: warning: array subscript 1
> is outside array bounds of ‘const char[1]’ [-Warray-bounds]
>   298 |  r6 = be32_to_cpu(p[1]);
> include/uapi/linux/byteorder/big_endian.h:40:51: note: in definition of
> macro ‘__be32_to_cpu’
>    40 | #define __be32_to_cpu(x) ((__force __u32)(__be32)(x))
>       |                                                   ^
> arch/powerpc/include/asm/epapr_hcalls.h:298:7: note: in expansion of macro
> ‘be32_to_cpu’
>   298 |  r6 = be32_to_cpu(p[1]);
>       |       ^~~~~~~~~~~
> drivers/tty/ehv_bytechan.c:166:13: note: while referencing ‘data’
>   166 | static void ehv_bc_udbg_putc(char c)
>       |             ^~~~~~~~~~~~~~~~
> In file included from include/linux/byteorder/big_endian.h:5,
>                  from arch/powerpc/include/uapi/asm/byteorder.h:14,
>                  from include/asm-generic/bitops/le.h:6,
>                  from arch/powerpc/include/asm/bitops.h:250,
>                  from include/linux/bitops.h:29,
>                  from include/linux/kernel.h:12,
>                  from include/asm-generic/bug.h:19,
>                  from arch/powerpc/include/asm/bug.h:109,
>                  from include/linux/bug.h:5,
>                  from include/linux/mmdebug.h:5,
>                  from include/linux/gfp.h:5,
>                  from include/linux/slab.h:15,
>                  from drivers/tty/ehv_bytechan.c:24:
> arch/powerpc/include/asm/epapr_hcalls.h:299:20: warning: array subscript 2
> is outside array bounds of ‘const char[1]’ [-Warray-bounds]
>   299 |  r7 = be32_to_cpu(p[2]);
> include/uapi/linux/byteorder/big_endian.h:40:51: note: in definition of
> macro ‘__be32_to_cpu’
>    40 | #define __be32_to_cpu(x) ((__force __u32)(__be32)(x))
>       |                                                   ^
> arch/powerpc/include/asm/epapr_hcalls.h:299:7: note: in expansion of macro
> ‘be32_to_cpu’
>   299 |  r7 = be32_to_cpu(p[2]);
>       |       ^~~~~~~~~~~
> drivers/tty/ehv_bytechan.c:166:13: note: while referencing ‘data’
>   166 | static void ehv_bc_udbg_putc(char c)
>       |             ^~~~~~~~~~~~~~~~
> In file included from include/linux/byteorder/big_endian.h:5,
>                  from arch/powerpc/include/uapi/asm/byteorder.h:14,
>                  from include/asm-generic/bitops/le.h:6,
>                  from arch/powerpc/include/asm/bitops.h:250,
>                  from include/linux/bitops.h:29,
>                  from include/linux/kernel.h:12,
>                  from include/asm-generic/bug.h:19,
>                  from arch/powerpc/include/asm/bug.h:109,
>                  from include/linux/bug.h:5,
>                  from include/linux/mmdebug.h:5,
>                  from include/linux/gfp.h:5,
>                  from include/linux/slab.h:15,
>                  from drivers/tty/ehv_bytechan.c:24:
> arch/powerpc/include/asm/epapr_hcalls.h:300:20: warning: array subscript 3
> is outside array bounds of ‘const char[1]’ [-Warray-bounds]
>   300 |  r8 = be32_to_cpu(p[3]);
> include/uapi/linux/byteorder/big_endian.h:40:51: note: in definition of
> macro ‘__be32_to_cpu’
>    40 | #define __be32_to_cpu(x) ((__force __u32)(__be32)(x))
>       |                                                   ^
> arch/powerpc/include/asm/epapr_hcalls.h:300:7: note: in expansion of macro
> ‘be32_to_cpu’
>   300 |  r8 = be32_to_cpu(p[3]);
>       |       ^~~~~~~~~~~
> drivers/tty/ehv_bytechan.c:166:13: note: while referencing ‘data’
>   166 | static void ehv_bc_udbg_putc(char c)
>       |             ^~~~~~~~~~~~~~~~
> 
> Signed-off-by: Stephen Rothwell <sfr@canb.auug.org.au>
> ---

Tested-by: Laurentiu Tudor <laurentiu.tudor@nxp.com>

---
Best Regards, Laurentiu

Re: [PATCH] evh_bytechan: fix out of bounds accesses

[Timur Tabi]

On 1/14/20 12:31 AM, Stephen Rothwell wrote:
> +/**
> + * ev_byte_channel_send - send characters to a byte stream
> + * @handle: byte stream handle
> + * @count: (input) num of chars to send, (output) num chars sent
> + * @bp: pointer to chars to send
> + *
> + * Returns 0 for success, or an error code.
> + */
> +static unsigned int ev_byte_channel_send(unsigned int handle,
> +	unsigned int *count, const char *bp)

Well, now you've moved this into the .c file and it is no longer 
available to other callers.  Anything wrong with keeping it in the .h file?

Re: [PATCH] evh_bytechan: fix out of bounds accesses

[Stephen Rothwell]

[-- Attachment #1: Type: text/plain, Size: 897 bytes --]

Hi Timur,

On Wed, 15 Jan 2020 07:25:45 -0600 Timur Tabi <timur@kernel.org> wrote:
>
> On 1/14/20 12:31 AM, Stephen Rothwell wrote:
> > +/**
> > + * ev_byte_channel_send - send characters to a byte stream
> > + * @handle: byte stream handle
> > + * @count: (input) num of chars to send, (output) num chars sent
> > + * @bp: pointer to chars to send
> > + *
> > + * Returns 0 for success, or an error code.
> > + */
> > +static unsigned int ev_byte_channel_send(unsigned int handle,
> > +	unsigned int *count, const char *bp)  
> 
> Well, now you've moved this into the .c file and it is no longer 
> available to other callers.  Anything wrong with keeping it in the .h file?

There are currently no other callers - are there likely to be in the
future?  Even if there are, is it time critical enough that it needs to
be inlined everywhere?

-- 
Cheers,
Stephen Rothwell

[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

Re: [PATCH] evh_bytechan: fix out of bounds accesses

[Scott Wood]

On Thu, 2020-01-16 at 06:42 +1100, Stephen Rothwell wrote:
> Hi Timur,
> 
> On Wed, 15 Jan 2020 07:25:45 -0600 Timur Tabi <timur@kernel.org> wrote:
> > On 1/14/20 12:31 AM, Stephen Rothwell wrote:
> > > +/**
> > > + * ev_byte_channel_send - send characters to a byte stream
> > > + * @handle: byte stream handle
> > > + * @count: (input) num of chars to send, (output) num chars sent
> > > + * @bp: pointer to chars to send
> > > + *
> > > + * Returns 0 for success, or an error code.
> > > + */
> > > +static unsigned int ev_byte_channel_send(unsigned int handle,
> > > +	unsigned int *count, const char *bp)  
> > 
> > Well, now you've moved this into the .c file and it is no longer 
> > available to other callers.  Anything wrong with keeping it in the .h
> > file?
> 
> There are currently no other callers - are there likely to be in the
> future?  Even if there are, is it time critical enough that it needs to
> be inlined everywhere?

It's not performance critical and there aren't likely to be other users --
just a matter of what's cleaner.  FWIW I'd rather see the original patch,
that keeps the raw asm hcall stuff as simple wrappers in one place.

-Scott

Re: [PATCH] evh_bytechan: fix out of bounds accesses

[Stephen Rothwell]

[-- Attachment #1: Type: text/plain, Size: 1454 bytes --]

Hi Scott,

On Wed, 15 Jan 2020 14:01:35 -0600 Scott Wood <swood@redhat.com> wrote:
>
> On Thu, 2020-01-16 at 06:42 +1100, Stephen Rothwell wrote:
> > Hi Timur,
> > 
> > On Wed, 15 Jan 2020 07:25:45 -0600 Timur Tabi <timur@kernel.org> wrote:  
> > > On 1/14/20 12:31 AM, Stephen Rothwell wrote:  
> > > > +/**
> > > > + * ev_byte_channel_send - send characters to a byte stream
> > > > + * @handle: byte stream handle
> > > > + * @count: (input) num of chars to send, (output) num chars sent
> > > > + * @bp: pointer to chars to send
> > > > + *
> > > > + * Returns 0 for success, or an error code.
> > > > + */
> > > > +static unsigned int ev_byte_channel_send(unsigned int handle,
> > > > +	unsigned int *count, const char *bp)    
> > > 
> > > Well, now you've moved this into the .c file and it is no longer 
> > > available to other callers.  Anything wrong with keeping it in the .h
> > > file?  
> > 
> > There are currently no other callers - are there likely to be in the
> > future?  Even if there are, is it time critical enough that it needs to
> > be inlined everywhere?  
> 
> It's not performance critical and there aren't likely to be other users --
> just a matter of what's cleaner.  FWIW I'd rather see the original patch,
> that keeps the raw asm hcall stuff as simple wrappers in one place.

And I don't mind either way :-)

I just want to get rid of the warnings.

-- 
Cheers,
Stephen Rothwell

[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

Re: [PATCH] evh_bytechan: fix out of bounds accesses

[Timur Tabi]

On 1/15/20 2:01 PM, Scott Wood wrote:
> FWIW I'd rather see the original patch,
> that keeps the raw asm hcall stuff as simple wrappers in one place.

I can live with that.

Re: [PATCH] evh_bytechan: fix out of bounds accesses

[Stephen Rothwell]

[-- Attachment #1: Type: text/plain, Size: 1643 bytes --]

Hi all,

On Thu, 16 Jan 2020 11:37:14 +1100 Stephen Rothwell <sfr@canb.auug.org.au> wrote:
>
> On Wed, 15 Jan 2020 14:01:35 -0600 Scott Wood <swood@redhat.com> wrote:
> >
> > On Thu, 2020-01-16 at 06:42 +1100, Stephen Rothwell wrote:  
> > > Hi Timur,
> > > 
> > > On Wed, 15 Jan 2020 07:25:45 -0600 Timur Tabi <timur@kernel.org> wrote:    
> > > > On 1/14/20 12:31 AM, Stephen Rothwell wrote:    
> > > > > +/**
> > > > > + * ev_byte_channel_send - send characters to a byte stream
> > > > > + * @handle: byte stream handle
> > > > > + * @count: (input) num of chars to send, (output) num chars sent
> > > > > + * @bp: pointer to chars to send
> > > > > + *
> > > > > + * Returns 0 for success, or an error code.
> > > > > + */
> > > > > +static unsigned int ev_byte_channel_send(unsigned int handle,
> > > > > +	unsigned int *count, const char *bp)      
> > > > 
> > > > Well, now you've moved this into the .c file and it is no longer 
> > > > available to other callers.  Anything wrong with keeping it in the .h
> > > > file?    
> > > 
> > > There are currently no other callers - are there likely to be in the
> > > future?  Even if there are, is it time critical enough that it needs to
> > > be inlined everywhere?    
> > 
> > It's not performance critical and there aren't likely to be other users --
> > just a matter of what's cleaner.  FWIW I'd rather see the original patch,
> > that keeps the raw asm hcall stuff as simple wrappers in one place.  
> 
> And I don't mind either way :-)
> 
> I just want to get rid of the warnings.

Any progress with this?
-- 
Cheers,
Stephen Rothwell

[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

Re: [PATCH] evh_bytechan: fix out of bounds accesses

[Laurentiu Tudor]

On 21.02.2020 01:57, Stephen Rothwell wrote:
> Hi all,
> 
> On Thu, 16 Jan 2020 11:37:14 +1100 Stephen Rothwell <sfr@canb.auug.org.au> wrote:
>>
>> On Wed, 15 Jan 2020 14:01:35 -0600 Scott Wood <swood@redhat.com> wrote:
>>>
>>> On Thu, 2020-01-16 at 06:42 +1100, Stephen Rothwell wrote:
>>>> Hi Timur,
>>>>
>>>> On Wed, 15 Jan 2020 07:25:45 -0600 Timur Tabi <timur@kernel.org> wrote:
>>>>> On 1/14/20 12:31 AM, Stephen Rothwell wrote:
>>>>>> +/**
>>>>>> + * ev_byte_channel_send - send characters to a byte stream
>>>>>> + * @handle: byte stream handle
>>>>>> + * @count: (input) num of chars to send, (output) num chars sent
>>>>>> + * @bp: pointer to chars to send
>>>>>> + *
>>>>>> + * Returns 0 for success, or an error code.
>>>>>> + */
>>>>>> +static unsigned int ev_byte_channel_send(unsigned int handle,
>>>>>> +	unsigned int *count, const char *bp)
>>>>>
>>>>> Well, now you've moved this into the .c file and it is no longer
>>>>> available to other callers.  Anything wrong with keeping it in the .h
>>>>> file?
>>>>
>>>> There are currently no other callers - are there likely to be in the
>>>> future?  Even if there are, is it time critical enough that it needs to
>>>> be inlined everywhere?
>>>
>>> It's not performance critical and there aren't likely to be other users --
>>> just a matter of what's cleaner.  FWIW I'd rather see the original patch,
>>> that keeps the raw asm hcall stuff as simple wrappers in one place.
>>
>> And I don't mind either way :-)
>>
>> I just want to get rid of the warnings.
> 
> Any progress with this?
> 

I think that the consensus was to pick up the original patch that is, 
this one: https://patchwork.ozlabs.org/patch/1220186/

I've tested it too, so please feel free to add a:

Tested-by: Laurentiu Tudor <laurentiu.tudor@nxp.com>

---
Best Regards, Laurentiu

Re: [PATCH] evh_bytechan: fix out of bounds accesses

[Stephen Rothwell]

[-- Attachment #1: Type: text/plain, Size: 2090 bytes --]

Hi Laurentiu,

On Tue, 25 Feb 2020 11:54:17 +0200 Laurentiu Tudor <laurentiu.tudor@nxp.com> wrote:
>
> On 21.02.2020 01:57, Stephen Rothwell wrote:
> > 
> > On Thu, 16 Jan 2020 11:37:14 +1100 Stephen Rothwell <sfr@canb.auug.org.au> wrote:  
> >>
> >> On Wed, 15 Jan 2020 14:01:35 -0600 Scott Wood <swood@redhat.com> wrote:  
> >>>
> >>> On Thu, 2020-01-16 at 06:42 +1100, Stephen Rothwell wrote:  
> >>>>
> >>>> On Wed, 15 Jan 2020 07:25:45 -0600 Timur Tabi <timur@kernel.org> wrote:  
> >>>>> On 1/14/20 12:31 AM, Stephen Rothwell wrote:  
> >>>>>> +/**
> >>>>>> + * ev_byte_channel_send - send characters to a byte stream
> >>>>>> + * @handle: byte stream handle
> >>>>>> + * @count: (input) num of chars to send, (output) num chars sent
> >>>>>> + * @bp: pointer to chars to send
> >>>>>> + *
> >>>>>> + * Returns 0 for success, or an error code.
> >>>>>> + */
> >>>>>> +static unsigned int ev_byte_channel_send(unsigned int handle,
> >>>>>> +	unsigned int *count, const char *bp)  
> >>>>>
> >>>>> Well, now you've moved this into the .c file and it is no longer
> >>>>> available to other callers.  Anything wrong with keeping it in the .h
> >>>>> file?  
> >>>>
> >>>> There are currently no other callers - are there likely to be in the
> >>>> future?  Even if there are, is it time critical enough that it needs to
> >>>> be inlined everywhere?  
> >>>
> >>> It's not performance critical and there aren't likely to be other users --
> >>> just a matter of what's cleaner.  FWIW I'd rather see the original patch,
> >>> that keeps the raw asm hcall stuff as simple wrappers in one place.  
> >>
> >> And I don't mind either way :-)
> >>
> >> I just want to get rid of the warnings.  
> > 
> > Any progress with this?
> 
> I think that the consensus was to pick up the original patch that is, 
> this one: https://patchwork.ozlabs.org/patch/1220186/
> 
> I've tested it too, so please feel free to add a:
> 
> Tested-by: Laurentiu Tudor <laurentiu.tudor@nxp.com>

So, whose tree should his go via?

-- 
Cheers,
Stephen Rothwell

[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

Re: [PATCH] evh_bytechan: fix out of bounds accesses

[Laurentiu Tudor]

On 25.02.2020 22:56, Stephen Rothwell wrote:
> Hi Laurentiu,
> 
> On Tue, 25 Feb 2020 11:54:17 +0200 Laurentiu Tudor <laurentiu.tudor@nxp.com> wrote:
>>
>> On 21.02.2020 01:57, Stephen Rothwell wrote:
>>>
>>> On Thu, 16 Jan 2020 11:37:14 +1100 Stephen Rothwell <sfr@canb.auug.org.au> wrote:
>>>>
>>>> On Wed, 15 Jan 2020 14:01:35 -0600 Scott Wood <swood@redhat.com> wrote:
>>>>>
>>>>> On Thu, 2020-01-16 at 06:42 +1100, Stephen Rothwell wrote:
>>>>>>
>>>>>> On Wed, 15 Jan 2020 07:25:45 -0600 Timur Tabi <timur@kernel.org> wrote:
>>>>>>> On 1/14/20 12:31 AM, Stephen Rothwell wrote:
>>>>>>>> +/**
>>>>>>>> + * ev_byte_channel_send - send characters to a byte stream
>>>>>>>> + * @handle: byte stream handle
>>>>>>>> + * @count: (input) num of chars to send, (output) num chars sent
>>>>>>>> + * @bp: pointer to chars to send
>>>>>>>> + *
>>>>>>>> + * Returns 0 for success, or an error code.
>>>>>>>> + */
>>>>>>>> +static unsigned int ev_byte_channel_send(unsigned int handle,
>>>>>>>> +	unsigned int *count, const char *bp)
>>>>>>>
>>>>>>> Well, now you've moved this into the .c file and it is no longer
>>>>>>> available to other callers.  Anything wrong with keeping it in the .h
>>>>>>> file?
>>>>>>
>>>>>> There are currently no other callers - are there likely to be in the
>>>>>> future?  Even if there are, is it time critical enough that it needs to
>>>>>> be inlined everywhere?
>>>>>
>>>>> It's not performance critical and there aren't likely to be other users --
>>>>> just a matter of what's cleaner.  FWIW I'd rather see the original patch,
>>>>> that keeps the raw asm hcall stuff as simple wrappers in one place.
>>>>
>>>> And I don't mind either way :-)
>>>>
>>>> I just want to get rid of the warnings.
>>>
>>> Any progress with this?
>>
>> I think that the consensus was to pick up the original patch that is,
>> this one: https://patchwork.ozlabs.org/patch/1220186/
>>
>> I've tested it too, so please feel free to add a:
>>
>> Tested-by: Laurentiu Tudor <laurentiu.tudor@nxp.com>
> 
> So, whose tree should his go via?
> 

Maybe Scott or Michael can help us here. And while at it maybe, take a 
look at this patch [1] too.

[1] https://patchwork.ozlabs.org/patch/1227780/

---
Best Regards, Laurentiu

Re: [PATCH] evh_bytechan: fix out of bounds accesses

[Michael Ellerman]

On Thu, 2020-01-09 at 07:39:12 UTC, Stephen Rothwell wrote:
> 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.
> 
> The warnings (from a powerpc allyesconfig build) are:
> 
> In file included from include/linux/byteorder/big_endian.h:5,
>                  from arch/powerpc/include/uapi/asm/byteorder.h:14,
>                  from include/asm-generic/bitops/le.h:6,
>                  from arch/powerpc/include/asm/bitops.h:250,
>                  from include/linux/bitops.h:29,
>                  from include/linux/kernel.h:12,
>                  from include/asm-generic/bug.h:19,
>                  from arch/powerpc/include/asm/bug.h:109,
>                  from include/linux/bug.h:5,
>                  from include/linux/mmdebug.h:5,
>                  from include/linux/gfp.h:5,
>                  from include/linux/slab.h:15,
>                  from drivers/tty/ehv_bytechan.c:24:
> drivers/tty/ehv_bytechan.c: In function =E2=80=98ehv_bc_udbg_putc=E2=80=99:
> arch/powerpc/include/asm/epapr_hcalls.h:298:20: warning: array subscript 1 =
> is outside array bounds of =E2=80=98const char[1]=E2=80=99 [-Warray-bounds]
>   298 |  r6 =3D be32_to_cpu(p[1]);
> include/uapi/linux/byteorder/big_endian.h:40:51: note: in definition of mac=
> ro =E2=80=98__be32_to_cpu=E2=80=99
>    40 | #define __be32_to_cpu(x) ((__force __u32)(__be32)(x))
>       |                                                   ^
> arch/powerpc/include/asm/epapr_hcalls.h:298:7: note: in expansion of macro =
> =E2=80=98be32_to_cpu=E2=80=99
>   298 |  r6 =3D be32_to_cpu(p[1]);
>       |       ^~~~~~~~~~~
> drivers/tty/ehv_bytechan.c:166:13: note: while referencing =E2=80=98data=E2=
> =80=99
>   166 | static void ehv_bc_udbg_putc(char c)
>       |             ^~~~~~~~~~~~~~~~
> In file included from include/linux/byteorder/big_endian.h:5,
>                  from arch/powerpc/include/uapi/asm/byteorder.h:14,
>                  from include/asm-generic/bitops/le.h:6,
>                  from arch/powerpc/include/asm/bitops.h:250,
>                  from include/linux/bitops.h:29,
>                  from include/linux/kernel.h:12,
>                  from include/asm-generic/bug.h:19,
>                  from arch/powerpc/include/asm/bug.h:109,
>                  from include/linux/bug.h:5,
>                  from include/linux/mmdebug.h:5,
>                  from include/linux/gfp.h:5,
>                  from include/linux/slab.h:15,
>                  from drivers/tty/ehv_bytechan.c:24:
> arch/powerpc/include/asm/epapr_hcalls.h:299:20: warning: array subscript 2 =
> is outside array bounds of =E2=80=98const char[1]=E2=80=99 [-Warray-bounds]
>   299 |  r7 =3D be32_to_cpu(p[2]);
> include/uapi/linux/byteorder/big_endian.h:40:51: note: in definition of mac=
> ro =E2=80=98__be32_to_cpu=E2=80=99
>    40 | #define __be32_to_cpu(x) ((__force __u32)(__be32)(x))
>       |                                                   ^
> arch/powerpc/include/asm/epapr_hcalls.h:299:7: note: in expansion of macro =
> =E2=80=98be32_to_cpu=E2=80=99
>   299 |  r7 =3D be32_to_cpu(p[2]);
>       |       ^~~~~~~~~~~
> drivers/tty/ehv_bytechan.c:166:13: note: while referencing =E2=80=98data=E2=
> =80=99
>   166 | static void ehv_bc_udbg_putc(char c)
>       |             ^~~~~~~~~~~~~~~~
> In file included from include/linux/byteorder/big_endian.h:5,
>                  from arch/powerpc/include/uapi/asm/byteorder.h:14,
>                  from include/asm-generic/bitops/le.h:6,
>                  from arch/powerpc/include/asm/bitops.h:250,
>                  from include/linux/bitops.h:29,
>                  from include/linux/kernel.h:12,
>                  from include/asm-generic/bug.h:19,
>                  from arch/powerpc/include/asm/bug.h:109,
>                  from include/linux/bug.h:5,
>                  from include/linux/mmdebug.h:5,
>                  from include/linux/gfp.h:5,
>                  from include/linux/slab.h:15,
>                  from drivers/tty/ehv_bytechan.c:24:
> arch/powerpc/include/asm/epapr_hcalls.h:300:20: warning: array subscript 3 =
> is outside array bounds of =E2=80=98const char[1]=E2=80=99 [-Warray-bounds]
>   300 |  r8 =3D be32_to_cpu(p[3]);
> include/uapi/linux/byteorder/big_endian.h:40:51: note: in definition of mac=
> ro =E2=80=98__be32_to_cpu=E2=80=99
>    40 | #define __be32_to_cpu(x) ((__force __u32)(__be32)(x))
>       |                                                   ^
> arch/powerpc/include/asm/epapr_hcalls.h:300:7: note: in expansion of macro =
> =E2=80=98be32_to_cpu=E2=80=99
>   300 |  r8 =3D be32_to_cpu(p[3]);
>       |       ^~~~~~~~~~~
> drivers/tty/ehv_bytechan.c:166:13: note: while referencing =E2=80=98data=E2=
> =80=99
>   166 | static void ehv_bc_udbg_putc(char c)
>       |             ^~~~~~~~~~~~~~~~
> In file included from include/linux/byteorder/big_endian.h:5,
>                  from arch/powerpc/include/uapi/asm/byteorder.h:14,
>                  from include/asm-generic/bitops/le.h:6,
>                  from arch/powerpc/include/asm/bitops.h:250,
>                  from include/linux/bitops.h:29,
>                  from include/linux/kernel.h:12,
>                  from include/asm-generic/bug.h:19,
>                  from arch/powerpc/include/asm/bug.h:109,
>                  from include/linux/bug.h:5,
>                  from include/linux/mmdebug.h:5,
>                  from include/linux/gfp.h:5,
>                  from include/linux/slab.h:15,
>                  from drivers/tty/ehv_bytechan.c:24:
> arch/powerpc/include/asm/epapr_hcalls.h:298:20: warning: array subscript 1 =
> is outside array bounds of =E2=80=98const char[1]=E2=80=99 [-Warray-bounds]
>   298 |  r6 =3D be32_to_cpu(p[1]);
> include/uapi/linux/byteorder/big_endian.h:40:51: note: in definition of mac=
> ro =E2=80=98__be32_to_cpu=E2=80=99
>    40 | #define __be32_to_cpu(x) ((__force __u32)(__be32)(x))
>       |                                                   ^
> arch/powerpc/include/asm/epapr_hcalls.h:298:7: note: in expansion of macro =
> =E2=80=98be32_to_cpu=E2=80=99
>   298 |  r6 =3D be32_to_cpu(p[1]);
>       |       ^~~~~~~~~~~
> drivers/tty/ehv_bytechan.c:166:13: note: while referencing =E2=80=98data=E2=
> =80=99
>   166 | static void ehv_bc_udbg_putc(char c)
>       |             ^~~~~~~~~~~~~~~~
> In file included from include/linux/byteorder/big_endian.h:5,
>                  from arch/powerpc/include/uapi/asm/byteorder.h:14,
>                  from include/asm-generic/bitops/le.h:6,
>                  from arch/powerpc/include/asm/bitops.h:250,
>                  from include/linux/bitops.h:29,
>                  from include/linux/kernel.h:12,
>                  from include/asm-generic/bug.h:19,
>                  from arch/powerpc/include/asm/bug.h:109,
>                  from include/linux/bug.h:5,
>                  from include/linux/mmdebug.h:5,
>                  from include/linux/gfp.h:5,
>                  from include/linux/slab.h:15,
>                  from drivers/tty/ehv_bytechan.c:24:
> arch/powerpc/include/asm/epapr_hcalls.h:299:20: warning: array subscript 2 =
> is outside array bounds of =E2=80=98const char[1]=E2=80=99 [-Warray-bounds]
>   299 |  r7 =3D be32_to_cpu(p[2]);
> include/uapi/linux/byteorder/big_endian.h:40:51: note: in definition of mac=
> ro =E2=80=98__be32_to_cpu=E2=80=99
>    40 | #define __be32_to_cpu(x) ((__force __u32)(__be32)(x))
>       |                                                   ^
> arch/powerpc/include/asm/epapr_hcalls.h:299:7: note: in expansion of macro =
> =E2=80=98be32_to_cpu=E2=80=99
>   299 |  r7 =3D be32_to_cpu(p[2]);
>       |       ^~~~~~~~~~~
> drivers/tty/ehv_bytechan.c:166:13: note: while referencing =E2=80=98data=E2=
> =80=99
>   166 | static void ehv_bc_udbg_putc(char c)
>       |             ^~~~~~~~~~~~~~~~
> In file included from include/linux/byteorder/big_endian.h:5,
>                  from arch/powerpc/include/uapi/asm/byteorder.h:14,
>                  from include/asm-generic/bitops/le.h:6,
>                  from arch/powerpc/include/asm/bitops.h:250,
>                  from include/linux/bitops.h:29,
>                  from include/linux/kernel.h:12,
>                  from include/asm-generic/bug.h:19,
>                  from arch/powerpc/include/asm/bug.h:109,
>                  from include/linux/bug.h:5,
>                  from include/linux/mmdebug.h:5,
>                  from include/linux/gfp.h:5,
>                  from include/linux/slab.h:15,
>                  from drivers/tty/ehv_bytechan.c:24:
> arch/powerpc/include/asm/epapr_hcalls.h:300:20: warning: array subscript 3 =
> is outside array bounds of =E2=80=98const char[1]=E2=80=99 [-Warray-bounds]
>   300 |  r8 =3D be32_to_cpu(p[3]);
> include/uapi/linux/byteorder/big_endian.h:40:51: note: in definition of mac=
> ro =E2=80=98__be32_to_cpu=E2=80=99
>    40 | #define __be32_to_cpu(x) ((__force __u32)(__be32)(x))
>       |                                                   ^
> arch/powerpc/include/asm/epapr_hcalls.h:300:7: note: in expansion of macro =
> =E2=80=98be32_to_cpu=E2=80=99
>   300 |  r8 =3D be32_to_cpu(p[3]);
>       |       ^~~~~~~~~~~
> drivers/tty/ehv_bytechan.c:166:13: note: while referencing =E2=80=98data=E2=
> =80=99
>   166 | static void ehv_bc_udbg_putc(char c)
>       |             ^~~~~~~~~~~~~~~~
> 
> Fixes: dcd83aaff1c8 ("tty/powerpc: introduce the ePAPR embedded hypervisor =
> byte channel driver")
> Cc: Michael Ellerman <mpe@ellerman.id.au>
> Cc: PowerPC Mailing List <linuxppc-dev@lists.ozlabs.org>
> Signed-off-by: Stephen Rothwell <sfr@canb.auug.org.au>

Applied to powerpc next, thanks.

https://git.kernel.org/powerpc/c/3670664b5da555a2a481449b3baafff113b0ac35

cheers