On Tue, Jul 28, 2020 at 11:41:37AM +0300, Dan Carpenter wrote:

On Mon, Jul 27, 2020 at 05:02:35PM -0400, Peilin Ye wrote:

quoted

hinfo_to_cinfo() does no operation on `cinfo` when `hinfo` is NULL,
causing kioc_to_mimd() to copy uninitialized stack memory to userspace.
Fix it by initializing `cinfo` with memset().

But "hinfo" can't be NULL so this patch isn't required.  It's a bit
hard for Smatch to follow the code.

We know that "opcode" is 82 so the buffer is allocated by mimd_to_kioc()
-> mraid_mm_attach_buf().

You are right. mraid_mm_ioctl() returns -ENOMEM and never reaches
kioc_to_mimd() if mraid_mm_attach_buf() failed to get a buffer, so
`hinfo` can never be NULL for kioc_to_mimd().

Next time I will trace the data flow more carefully. Thank you for
pointing this out!

Peilin Ye

Generally, don't silence static checker warnings unless it makes the
code more readable.  It's the checker writer's job to fix their own code.
In this case, that's me, but parsing the code is quite complicated and I
don't have a plan for how to fix it.

regards,
dan carpenter

_______________________________________________
Linux-kernel-mentees mailing list
Linux-kernel-mentees@lists.linuxfoundation.org
https://lists.linuxfoundation.org/mailman/listinfo/linux-kernel-mentees