linux-kernel - Re: [Linux-kernel-mentees] [PATCH v3] media/v4l2-core: Fix kernel-infoleak in video_put

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:   Tue, 28 Jul 2020 13:02:37 +0300
From:   Dan Carpenter <dan.carpenter@...cle.com>
To:     Peilin Ye <yepeilin.cs@...il.com>
Cc:     Arnd Bergmann <arnd@...db.de>,
        Mauro Carvalho Chehab <mchehab@...nel.org>,
        Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
        syzkaller-bugs <syzkaller-bugs@...glegroups.com>,
        Hans Verkuil <hverkuil-cisco@...all.nl>,
        Sakari Ailus <sakari.ailus@...ux.intel.com>,
        Laurent Pinchart <laurent.pinchart@...asonboard.com>,
        Vandana BN <bnvandana@...il.com>,
        Ezequiel Garcia <ezequiel@...labora.com>,
        Niklas Söderlund 
        <niklas.soderlund+renesas@...natech.se>,
        linux-kernel-mentees@...ts.linuxfoundation.org,
        Linux Media Mailing List <linux-media@...r.kernel.org>,
        "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>
Subject: Re: [Linux-kernel-mentees] [PATCH v3] media/v4l2-core: Fix
 kernel-infoleak in video_put_user()

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