From: Laurentiu Tudor <laurentiu.tudor@nxp.com>
To: Stephen Rothwell <sfr@canb.auug.org.au>, Timur Tabi <timur@kernel.org>
Cc: "b08248@gmail.com" <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" <swood@redhat.com>
Subject: RE: [PATCH] evh_bytechan: fix out of bounds accesses
Date: Wed, 15 Jan 2020 12:33:41 +0000 [thread overview]
Message-ID: <AM6PR04MB58780399521A6A561483019BEC370@AM6PR04MB5878.eurprd04.prod.outlook.com> (raw)
In-Reply-To: <20200114173141.29564b25@canb.auug.org.au>
> -----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
next prev parent reply other threads:[~2020-01-15 12:37 UTC|newest]
Thread overview: 28+ messages / expand[flat|nested] mbox.gz Atom feed top
2020-01-09 7:39 [PATCH] evh_bytechan: fix out of bounds accesses Stephen Rothwell
2020-01-13 12:26 ` 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 [this message]
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=AM6PR04MB58780399521A6A561483019BEC370@AM6PR04MB5878.eurprd04.prod.outlook.com \
--to=laurentiu.tudor@nxp.com \
--cc=b08248@gmail.com \
--cc=gregkh@linuxfoundation.org \
--cc=jslaby@suse.com \
--cc=linuxppc-dev@lists.ozlabs.org \
--cc=sfr@canb.auug.org.au \
--cc=swood@redhat.com \
--cc=timur@kernel.org \
--cc=york.sun@nxp.com \
/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.