> -----Original Message----- > From: Linuxppc-dev <linuxppc-dev- > bounces+laurentiu.tudor=nxp....@lists.ozlabs.org> On Behalf Of Stephen > Rothwell > Sent: Tuesday, January 14, 2020 8:32 AM > To: Timur Tabi <ti...@kernel.org> > Cc: b08...@gmail.com; Greg Kroah-Hartman <gre...@linuxfoundation.org>; > Jiri Slaby <jsl...@suse.com>; York Sun <york....@nxp.com>; PowerPC Mailing > List <linuxppc-dev@lists.ozlabs.org>; sw...@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 <ti...@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 <s...@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 <s...@canb.auug.org.au> > ---
Tested-by: Laurentiu Tudor <laurentiu.tu...@nxp.com> --- Best Regards, Laurentiu