Hi Andi, Thanks for the patch but I would suggest to use strlcpy instead, this will guard msg.name overwriting and add the NULL termination in case of truncation: - memcpy(msg.name, name, sizeof(msg.name)); - msg.name[sizeof(msg.name) - 1] = 0; + strlcpy(msg.name, name, sizeof(msg.name));
Best regards, Hugues. On 12/22/2017 01:12 AM, Andi Kleen wrote: > From: Andi Kleen <a...@linux.intel.com> > > The single caller passes a string to delta_ipc_open, which copies with a > fixed size larger than the string. So it copies some random data after > the original string the ro segment. > > If the string was at the end of a page it may fault. > > Just copy the string with a normal strcpy after clearing the field. > > Found by a LTO build (which errors out) > because the compiler inlines the functions and can resolve > the string sizes and triggers the compile time checks in memcpy. > > In function ‘memcpy’, > inlined from ‘delta_ipc_open.constprop’ at > linux/drivers/media/platform/sti/delta/delta-ipc.c:178:0, > inlined from ‘delta_mjpeg_ipc_open’ at > linux/drivers/media/platform/sti/delta/delta-mjpeg-dec.c:227:0, > inlined from ‘delta_mjpeg_decode’ at > linux/drivers/media/platform/sti/delta/delta-mjpeg-dec.c:403:0: > /home/andi/lsrc/linux/include/linux/string.h:337:0: error: call to > ‘__read_overflow2’ declared with attribute error: detected read beyond size > of object passed as 2nd parameter > __read_overflow2(); > > Cc: hugues.fruc...@st.com > Cc: mche...@s-opensource.com > Signed-off-by: Andi Kleen <a...@linux.intel.com> > --- > drivers/media/platform/sti/delta/delta-ipc.c | 4 ++-- > 1 file changed, 2 insertions(+), 2 deletions(-) > > diff --git a/drivers/media/platform/sti/delta/delta-ipc.c > b/drivers/media/platform/sti/delta/delta-ipc.c > index 41e4a4c259b3..b6c256e3ceb6 100644 > --- a/drivers/media/platform/sti/delta/delta-ipc.c > +++ b/drivers/media/platform/sti/delta/delta-ipc.c > @@ -175,8 +175,8 @@ int delta_ipc_open(struct delta_ctx *pctx, const char > *name, > msg.ipc_buf_size = ipc_buf_size; > msg.ipc_buf_paddr = ctx->ipc_buf->paddr; > > - memcpy(msg.name, name, sizeof(msg.name)); > - msg.name[sizeof(msg.name) - 1] = 0; > + memset(msg.name, 0, sizeof(msg.name)); > + strcpy(msg.name, name); > > msg.param_size = param->size; > memcpy(ctx->ipc_buf->vaddr, param->data, msg.param_size); >
