From: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
To: Peilin Ye <yepeilin.cs@gmail.com>
Cc: "Mauro Carvalho Chehab" <mchehab@kernel.org>,
"Greg Kroah-Hartman" <gregkh@linuxfoundation.org>,
syzkaller-bugs@googlegroups.com,
"Hans Verkuil" <hverkuil-cisco@xs4all.nl>,
"Sakari Ailus" <sakari.ailus@linux.intel.com>,
"Arnd Bergmann" <arnd@arndb.de>,
"Vandana BN" <bnvandana@gmail.com>,
"Ezequiel Garcia" <ezequiel@collabora.com>,
"Niklas Söderlund" <niklas.soderlund+renesas@ragnatech.se>,
linux-kernel-mentees@lists.linuxfoundation.org,
linux-media@vger.kernel.org, linux-kernel@vger.kernel.org
Subject: Re: [Linux-kernel-mentees] [PATCH] media/v4l2-core: Fix kernel-infoleak in video_put_user()
Date: Mon, 27 Jul 2020 01:08:23 +0300 [thread overview]
Message-ID: <20200726220823.GI28704@pendragon.ideasonboard.com> (raw)
In-Reply-To: <20200726180752.GA49356@PWN>
Hi Peilin,
On Sun, Jul 26, 2020 at 02:07:52PM -0400, Peilin Ye wrote:
> On Sun, Jul 26, 2020 at 08:30:44PM +0300, Laurent Pinchart wrote:
> > Hi Peilin,
> >
> > Thank you for the patch.
> >
> > On Sun, Jul 26, 2020 at 12:44:39PM -0400, Peilin Ye wrote:
> > > video_put_user() is copying uninitialized stack memory to userspace. Fix
> > > it by initializing `vb32` using memset().
> >
> > What makes you think this will fix the issue ? When initializing a
> > structure at declaration time, the fields that are not explicitly
> > specified should be initialized to 0 by the compiler. See
> > https://www.ibm.com/support/knowledgecenter/en/SSLTBW_2.3.0/com.ibm.zos.v2r3.cbclx01/strin.htm:
>
> Hi Mr. Pinchart!
>
> First of all, syzbot tested this patch, and it says it's "OK":
>
> https://syzkaller.appspot.com/bug?extid=79d751604cb6f29fbf59
>
> > If a structure variable is partially initialized, all the uninitialized
> > structure members are implicitly initialized to zero no matter what the
> > storage class of the structure variable is. See the following example:
> >
> > struct one {
> > int a;
> > int b;
> > int c;
> > };
> >
> > void main() {
> > struct one z1; // Members in z1 do not have default initial values.
> > static struct one z2; // z2.a=0, z2.b=0, and z2.c=0.
> > struct one z3 = {1}; // z3.a=1, z3.b=0, and z3.c=0.
> > }
>
> Yes, I understand that. I can safely printk() all members of that struct
> without triggering a KMSAN warning, which means they have been properly
> initialized.
>
> However, if I do something like:
>
> char *p = (char *)&vb32;
> int i;
>
> for (i = 0; i < sizeof(struct vb32); i++, p++)
> printk("*(p + i): %d", *(p + i));
>
> This tries to print out `vb32` as "raw memory" one byte at a time, and
> triggers a KMSAN warning somewhere in the middle (when `i` equals to 25
> or 26).
>
> According to a previous discussion with Mr. Kroah-Hartman, as well as
> this LWN article:
>
> "Structure holes and information leaks"
> https://lwn.net/Articles/417989/
>
> Initializing a struct by assigning (both partially or fully) leaves the
> "padding" part of it uninitialized, thus potentially leads to kernel
> information leak if the structure in question is going to be copied to
> userspace.
>
> memset() sets these "uninitialized paddings" to zero, therefore (I
> think) should solve the problem.
You're absolutely right. I wasn't aware the compiler wouldn't initialize
holes in the structure. Thank you for educating me :-)
For the patch,
Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
--
Regards,
Laurent Pinchart
next prev parent reply other threads:[~2020-07-26 22:08 UTC|newest]
Thread overview: 33+ messages / expand[flat|nested] mbox.gz Atom feed top
2020-07-26 16:44 [Linux-kernel-mentees] [PATCH] media/v4l2-core: Fix kernel-infoleak in video_put_user() Peilin Ye
2020-07-26 17:30 ` Laurent Pinchart
2020-07-26 18:07 ` Peilin Ye
2020-07-26 22:08 ` Laurent Pinchart [this message]
2020-07-26 22:15 ` Peilin Ye
2020-07-26 18:12 ` Peilin Ye
2020-07-26 22:05 ` [Linux-kernel-mentees] [PATCH v2] " Peilin Ye
2020-07-26 22:10 ` Laurent Pinchart
2020-07-26 22:16 ` Peilin Ye
2020-07-26 22:27 ` [Linux-kernel-mentees] [PATCH v3] " Peilin Ye
2020-07-27 7:25 ` Arnd Bergmann
2020-07-27 7:56 ` Peilin Ye
2020-07-27 13:16 ` Dan Carpenter
2020-07-27 14:05 ` Arnd Bergmann
2020-07-27 14:14 ` Peilin Ye
2020-07-27 14:20 ` Arnd Bergmann
2020-07-27 14:46 ` Dan Carpenter
2020-07-27 15:30 ` Peilin Ye
2020-07-27 14:43 ` Dan Carpenter
2020-07-27 14:55 ` Arnd Bergmann
2020-07-27 22:04 ` Peilin Ye
2020-07-28 9:00 ` Arnd Bergmann
2020-07-28 10:02 ` Dan Carpenter
2020-07-28 12:22 ` Linus Walleij
2020-07-28 13:06 ` Dan Carpenter
2020-07-28 13:58 ` Arnd Bergmann
2020-07-30 8:07 ` Bartosz Golaszewski
2020-07-30 8:15 ` Arnd Bergmann
2020-07-30 8:38 ` Andy Shevchenko
2020-07-30 9:18 ` Arnd Bergmann
2020-07-30 11:48 ` Andy Shevchenko
2020-07-30 13:49 ` Arnd Bergmann
2020-07-27 8:00 ` [Linux-kernel-mentees] [PATCH v4] " Peilin Ye
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20200726220823.GI28704@pendragon.ideasonboard.com \
--to=laurent.pinchart@ideasonboard.com \
--cc=arnd@arndb.de \
--cc=bnvandana@gmail.com \
--cc=ezequiel@collabora.com \
--cc=gregkh@linuxfoundation.org \
--cc=hverkuil-cisco@xs4all.nl \
--cc=linux-kernel-mentees@lists.linuxfoundation.org \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-media@vger.kernel.org \
--cc=mchehab@kernel.org \
--cc=niklas.soderlund+renesas@ragnatech.se \
--cc=sakari.ailus@linux.intel.com \
--cc=syzkaller-bugs@googlegroups.com \
--cc=yepeilin.cs@gmail.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).