Re: [Linux-kernel-mentees] [PATCH v3] media/v4l2-core: Fix kernel-infoleak in video_put_user()

[Dan Carpenter]

On Mon, Jul 27, 2020 at 06:04:56PM -0400, Peilin Ye wrote:
> On Mon, Jul 27, 2020 at 04:16:08PM +0300, Dan Carpenter wrote:
> > drivers/char/hpet.c:675 hpet_ioctl() warn: check that 'info' doesn't leak information (struct has a hole after 'hi_timer')
> 
> This one seems like a false positive.

Yep.

> 
> drivers/char/hpet.c:670:
> 
> 	mutex_lock(&hpet_mutex);
> 	err = hpet_ioctl_common(file->private_data, cmd, arg, &info);
> 	mutex_unlock(&hpet_mutex);
> 
> 	if ((cmd == HPET_INFO) && !err &&
> 	    (copy_to_user((void __user *)arg, &info, sizeof(info))))
> 		err = -EFAULT;

When Smatch parses hpet_ioctl_common() it says there are two success
paths:

drivers/char/hpet.c | hpet_ioctl_common | 170 |             0 |   PARAM_LIMIT |   1 |                    $ |                26625 |

The first success path is for when cmd is HPET_IE_ON.  We don't care
about that.

drivers/char/hpet.c | hpet_ioctl_common | 185 |             0 |   PARAM_LIMIT |   1 |                    $ | 26626,26628-26629,1074292742,2149083139 |

The second success path is for when cmd is HPET_IE_OFF, HPET_INFO,
HPET_EPI, HPET_DPI, or HPET_IRQFREQ.  If Smatch tracked the HPET_INFO
by itself then this wouldn't print a warning, but since Smatch groups
all those cmds together then it does print a warning.

It's not impossible to make Smatch split apart the success paths some
more but it's complicated because it means storing more data and slows
down the parsing.  The cross function database is already 27GB on my
system.

regards,
dan carpenter