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

From: Timur Tabi <timur@kernel.org>
To: Stephen Rothwell <sfr@canb.auug.org.au>,
	york sun <york.sun@nxp.com>,
	b08248@gmail.com, swood@redhat.com
Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>,
	PowerPC Mailing List <linuxppc-dev@lists.ozlabs.org>,
	Jiri Slaby <jslaby@suse.com>
Subject: Re: [PATCH] evh_bytechan: fix out of bounds accesses
Date: Mon, 13 Jan 2020 10:03:18 -0600	[thread overview]
Message-ID: <CAOZdJXXiKgz=hOoiaTrxgbnwzyvp1Zfn3aCz+0__i17vyFngRg@mail.gmail.com> (raw)
In-Reply-To: <20200109183912.5fcb52aa@canb.auug.org.au>

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.