All of lore.kernel.org
 help / color / mirror / Atom feed
From: Stephen Rothwell <sfr@canb.auug.org.au>
To: Greg Kroah-Hartman <gregkh@linuxfoundation.org>,
	Jiri Slaby <jslaby@suse.com>
Cc: PowerPC Mailing List <linuxppc-dev@lists.ozlabs.org>
Subject: [PATCH] evh_bytechan: fix out of bounds accesses
Date: Thu, 9 Jan 2020 18:39:12 +1100	[thread overview]
Message-ID: <20200109183912.5fcb52aa@canb.auug.org.au> (raw)

[-- 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 --]

             reply	other threads:[~2020-01-09  7:41 UTC|newest]

Thread overview: 28+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-01-09  7:39 Stephen Rothwell [this message]
2020-01-13 12:26 ` [PATCH] evh_bytechan: fix out of bounds accesses Michael Ellerman
2020-01-13 13:48   ` Timur Tabi
2020-01-13 14:34     ` Laurentiu Tudor
2020-01-13 15:48       ` Timur Tabi
2020-01-14  1:10       ` Michael Ellerman
2020-01-14  9:18         ` Laurentiu Tudor
2020-01-14 11:01           ` Timur Tabi
2020-01-13 16:03 ` Timur Tabi
2020-01-13 20:25   ` Stephen Rothwell
2020-01-14  1:10     ` Timur Tabi
2020-01-14  1:13       ` Timur Tabi
2020-01-14  1:17         ` Scott Wood
2020-01-14  6:31       ` Stephen Rothwell
2020-01-15 12:33         ` Laurentiu Tudor
2020-01-15 13:25         ` Timur Tabi
2020-01-15 19:42           ` Stephen Rothwell
2020-01-15 20:01             ` Scott Wood
2020-01-16  0:37               ` Stephen Rothwell
2020-02-20 23:57                 ` Stephen Rothwell
2020-02-25  9:54                   ` Laurentiu Tudor
2020-02-25 20:56                     ` Stephen Rothwell
2020-02-26  9:43                       ` Laurentiu Tudor
2020-01-16  2:29               ` Timur Tabi
2020-01-14  8:29       ` Segher Boessenkool
2020-01-14 11:53         ` Timur Tabi
2020-01-14 12:24           ` Segher Boessenkool
2020-03-17 13:14 ` Michael Ellerman

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20200109183912.5fcb52aa@canb.auug.org.au \
    --to=sfr@canb.auug.org.au \
    --cc=gregkh@linuxfoundation.org \
    --cc=jslaby@suse.com \
    --cc=linuxppc-dev@lists.ozlabs.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body. 
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.