All the mail mirrored from lore.kernel.org
 help / color / mirror / Atom feed
From: Arnd Bergmann <arnd@kernel.org>
To: Hans Verkuil <hverkuil-cisco@xs4all.nl>,
	Mauro Carvalho Chehab <mchehab@kernel.org>
Cc: Arnd Bergmann <arnd@arndb.de>,
	"Lad, Prabhakar" <prabhakar.csengg@gmail.com>,
	Eduardo Valentin <edubezval@gmail.com>,
	Sakari Ailus <sakari.ailus@linux.intel.com>,
	Greg Kroah-Hartman <gregkh@linuxfoundation.org>,
	Vaibhav Gupta <vaibhavgupta40@gmail.com>,
	Liu Shixin <liushixin2@huawei.com>,
	Laurent Pinchart <laurent.pinchart@ideasonboard.com>,
	Jacopo Mondi <jacopo+renesas@jmondi.org>,
	Andy Shevchenko <andriy.shevchenko@linux.intel.com>,
	linux-kernel@vger.kernel.org, linux-media@vger.kernel.org,
	linux-staging@lists.linux.dev,
	syzbot+142888ffec98ab194028@syzkaller.appspotmail.com
Subject: [PATCH v3 1/8] media: v4l2-core: ignore native time32 ioctls on 64-bit
Date: Mon, 14 Jun 2021 12:34:02 +0200	[thread overview]
Message-ID: <20210614103409.3154127-2-arnd@kernel.org> (raw)
In-Reply-To: <20210614103409.3154127-1-arnd@kernel.org>

From: Arnd Bergmann <arnd@arndb.de>

Syzbot found that passing ioctl command 0xc0505609 into a 64-bit
kernel from a 32-bit process causes uninitialized kernel memory to
get passed to drivers instead of the user space data:

BUG: KMSAN: uninit-value in check_array_args drivers/media/v4l2-core/v4l2-ioctl.c:3041 [inline]
BUG: KMSAN: uninit-value in video_usercopy+0x1631/0x3d30 drivers/media/v4l2-core/v4l2-ioctl.c:3315
CPU: 0 PID: 19595 Comm: syz-executor.4 Not tainted 5.11.0-rc7-syzkaller #0
Hardware name: Google Google Compute Engine/Google Compute Engine, BIOS Google 01/01/2011
Call Trace:
 __dump_stack lib/dump_stack.c:79 [inline]
 dump_stack+0x21c/0x280 lib/dump_stack.c:120
 kmsan_report+0xfb/0x1e0 mm/kmsan/kmsan_report.c:118
 __msan_warning+0x5f/0xa0 mm/kmsan/kmsan_instr.c:197
 check_array_args drivers/media/v4l2-core/v4l2-ioctl.c:3041 [inline]
 video_usercopy+0x1631/0x3d30 drivers/media/v4l2-core/v4l2-ioctl.c:3315
 video_ioctl2+0x9f/0xb0 drivers/media/v4l2-core/v4l2-ioctl.c:3391
 v4l2_ioctl+0x255/0x290 drivers/media/v4l2-core/v4l2-dev.c:360
 v4l2_compat_ioctl32+0x2c6/0x370 drivers/media/v4l2-core/v4l2-compat-ioctl32.c:1248
 __do_compat_sys_ioctl fs/ioctl.c:842 [inline]
 __se_compat_sys_ioctl+0x53d/0x1100 fs/ioctl.c:793
 __ia32_compat_sys_ioctl+0x4a/0x70 fs/ioctl.c:793
 do_syscall_32_irqs_on arch/x86/entry/common.c:79 [inline]
 __do_fast_syscall_32+0x102/0x160 arch/x86/entry/common.c:141
 do_fast_syscall_32+0x6a/0xc0 arch/x86/entry/common.c:166
 do_SYSENTER_32+0x73/0x90 arch/x86/entry/common.c:209
 entry_SYSENTER_compat_after_hwframe+0x4d/0x5c

The time32 commands are defined but were never meant to be called on
64-bit machines, as those have always used time64 interfaces.  I missed
this in my patch that introduced the time64 handling on 32-bit platforms.

The problem in this case is the mismatch of one function checking for
the numeric value of the command and another function checking for the
type of process (native vs compat) instead, with the result being that
for this combination, nothing gets copied into the buffer at all.

Avoid this by only trying to convert the time32 commands when running
on a 32-bit kernel where these are defined in a meaningful way.

Fixes: 577c89b0ce72 ("media: v4l2-core: fix v4l2_buffer handling for time64 ABI")
Reported-by: syzbot+142888ffec98ab194028@syzkaller.appspotmail.com
Tested-by: Hans Verkuil <hverkuil-cisco@xs4all.nl>
Signed-off-by: Arnd Bergmann <arnd@arndb.de>
---
This patch adds two more changes than the version that Hans tested
Signed-off-by: Arnd Bergmann <arnd@arndb.de>
---
 drivers/media/v4l2-core/v4l2-ioctl.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/media/v4l2-core/v4l2-ioctl.c b/drivers/media/v4l2-core/v4l2-ioctl.c
index 2673f51aafa4..58df927aec7e 100644
--- a/drivers/media/v4l2-core/v4l2-ioctl.c
+++ b/drivers/media/v4l2-core/v4l2-ioctl.c
@@ -3073,7 +3073,7 @@ static int check_array_args(unsigned int cmd, void *parg, size_t *array_size,
 static unsigned int video_translate_cmd(unsigned int cmd)
 {
 	switch (cmd) {
-#ifdef CONFIG_COMPAT_32BIT_TIME
+#if !defined(CONFIG_64BIT) && defined(CONFIG_COMPAT_32BIT_TIME)
 	case VIDIOC_DQEVENT_TIME32:
 		return VIDIOC_DQEVENT;
 	case VIDIOC_QUERYBUF_TIME32:
@@ -3127,7 +3127,7 @@ static int video_get_user(void __user *arg, void *parg,
 		err = v4l2_compat_get_user(arg, parg, cmd);
 	} else {
 		switch (cmd) {
-#ifdef CONFIG_COMPAT_32BIT_TIME
+#if !defined(CONFIG_64BIT) && defined(CONFIG_COMPAT_32BIT_TIME)
 		case VIDIOC_QUERYBUF_TIME32:
 		case VIDIOC_QBUF_TIME32:
 		case VIDIOC_DQBUF_TIME32:
@@ -3182,7 +3182,7 @@ static int video_put_user(void __user *arg, void *parg,
 		return v4l2_compat_put_user(arg, parg, cmd);
 
 	switch (cmd) {
-#ifdef CONFIG_COMPAT_32BIT_TIME
+#if !defined(CONFIG_64BIT) && defined(CONFIG_COMPAT_32BIT_TIME)
 	case VIDIOC_DQEVENT_TIME32: {
 		struct v4l2_event *ev = parg;
 		struct v4l2_event_time32 ev32;
-- 
2.29.2


  reply	other threads:[~2021-06-14 10:49 UTC|newest]

Thread overview: 24+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-06-14 10:34 [PATCH v3 0/7] media: v4l2: compat ioctl fixes Arnd Bergmann
2021-06-14 10:34 ` Arnd Bergmann [this message]
2021-06-14 13:24   ` [PATCH v3 1/8] media: v4l2-core: ignore native time32 ioctls on 64-bit Andy Shevchenko
2021-06-14 16:50   ` Laurent Pinchart
2021-06-14 10:34 ` [PATCH v3 2/8] media: v4l2-core: explicitly clear ioctl input data Arnd Bergmann
2021-06-14 16:56   ` Laurent Pinchart
2021-06-14 10:34 ` [PATCH v3 3/8] media: v4l2-core: fix whitespace damage in video_get_user() Arnd Bergmann
2021-06-14 16:58   ` Laurent Pinchart
2021-06-14 10:34 ` [PATCH v3 4/8] media: subdev: remove VIDIOC_DQEVENT_TIME32 handling Arnd Bergmann
2021-06-14 17:02   ` Laurent Pinchart
2021-06-15  8:43     ` Arnd Bergmann
2021-06-15  8:48       ` Hans Verkuil
2021-06-15  9:30         ` Arnd Bergmann
2021-06-14 10:34 ` [PATCH v3 5/8] media: v4l2-core: return -ENODEV from ioctl when not registered Arnd Bergmann
2021-06-14 17:04   ` Laurent Pinchart
2021-06-14 17:04   ` Laurent Pinchart
2021-06-14 10:34 ` [PATCH v3 6/8] media: atomisp: remove compat_ioctl32 code Arnd Bergmann
2021-06-14 17:07   ` Laurent Pinchart
2021-06-14 10:34 ` [PATCH v3 7/8] media: subdev: fix compat_ioctl32 Arnd Bergmann
2021-06-14 17:18   ` Laurent Pinchart
2021-06-15  8:26     ` Hans Verkuil
2021-06-15  8:39       ` Arnd Bergmann
2021-06-14 10:34 ` [PATCH v3 8/8] media: subdev: disallow ioctl for saa6588/davinci Arnd Bergmann
2021-06-14 17:21   ` Laurent Pinchart

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=20210614103409.3154127-2-arnd@kernel.org \
    --to=arnd@kernel.org \
    --cc=andriy.shevchenko@linux.intel.com \
    --cc=arnd@arndb.de \
    --cc=edubezval@gmail.com \
    --cc=gregkh@linuxfoundation.org \
    --cc=hverkuil-cisco@xs4all.nl \
    --cc=jacopo+renesas@jmondi.org \
    --cc=laurent.pinchart@ideasonboard.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-media@vger.kernel.org \
    --cc=linux-staging@lists.linux.dev \
    --cc=liushixin2@huawei.com \
    --cc=mchehab@kernel.org \
    --cc=prabhakar.csengg@gmail.com \
    --cc=sakari.ailus@linux.intel.com \
    --cc=syzbot+142888ffec98ab194028@syzkaller.appspotmail.com \
    --cc=vaibhavgupta40@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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.