All of lore.kernel.org
 help / color / mirror / Atom feed
From: Dan Carpenter <dan.carpenter@oracle.com>
To: Arnd Bergmann <arnd@arndb.de>
Cc: "Peilin Ye" <yepeilin.cs@gmail.com>,
	"Mauro Carvalho Chehab" <mchehab@kernel.org>,
	"Greg Kroah-Hartman" <gregkh@linuxfoundation.org>,
	syzkaller-bugs <syzkaller-bugs@googlegroups.com>,
	"Hans Verkuil" <hverkuil-cisco@xs4all.nl>,
	"Sakari Ailus" <sakari.ailus@linux.intel.com>,
	"Laurent Pinchart" <laurent.pinchart@ideasonboard.com>,
	"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 Mailing List" <linux-media@vger.kernel.org>,
	"linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>
Subject: Re: [Linux-kernel-mentees] [PATCH v3] media/v4l2-core: Fix kernel-infoleak in video_put_user()
Date: Mon, 27 Jul 2020 16:16:08 +0300	[thread overview]
Message-ID: <20200727131608.GD1913@kadam> (raw)
In-Reply-To: <CAK8P3a3NB2BVo9fH-Wcinrhhs-QJ=9dK59Ds83TvgLmEkRy3qA@mail.gmail.com>

On Mon, Jul 27, 2020 at 09:25:16AM +0200, Arnd Bergmann wrote:
> On Mon, Jul 27, 2020 at 12:28 AM Peilin Ye <yepeilin.cs@gmail.com> wrote:
> >
> > video_put_user() is copying uninitialized stack memory to userspace due
> > to the compiler not initializing holes in the structures declared on the
> > stack. Fix it by initializing `ev32` and `vb32` using memset().
> >
> > Reported-and-tested-by: syzbot+79d751604cb6f29fbf59@syzkaller.appspotmail.com
> > Link: https://syzkaller.appspot.com/bug?extid=79d751604cb6f29fbf59
> > Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> > Signed-off-by: Peilin Ye <yepeilin.cs@gmail.com>
> 
> Thanks a lot for addressing this! I now see that I actually created a similar
> bugfix for it back in January, but for some reason that got stuck in my
> backlog and I never wrote a proper description for it or sent it out to the
> list, sorry about that. I would hope we could find a way to have either
> the compiler or sparse warn if we copy uninitialized data to user space,
> but we now don't even check for that within the kernel any more.

Here are my latest warnings on linux-next from Friday.

block/scsi_ioctl.c:707 scsi_put_cdrom_generic_arg() warn: check that 'cgc32' doesn't leak information (struct has a hole after 'data_direction')
drivers/input/misc/uinput.c:743 uinput_ff_upload_to_user() warn: check that 'ff_up_compat' doesn't leak information (struct has a hole after 'replay')
drivers/input/misc/uinput.c:958 uinput_ioctl_handler() warn: check that 'ff_up' doesn't leak information (struct has a hole after 'replay')
drivers/firewire/core-cdev.c:463 ioctl_get_info() warn: check that 'bus_reset' doesn't leak information (struct has a hole after 'generation')
drivers/scsi/megaraid/megaraid_mm.c:824 kioc_to_mimd() warn: check that 'cinfo.base' doesn't leak information
drivers/gpio/gpiolib-cdev.c:473 lineevent_read() warn: check that 'ge' doesn't leak information (struct has a hole after 'id')
drivers/gpu/drm/i915/i915_query.c:136 query_engine_info() warn: check that 'query.num_engines' doesn't leak information
drivers/gpu/drm/drm_bufs.c:1357 copy_one_buf() warn: check that 'v' doesn't leak information (struct has a hole after 'flags')
drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c:785 amdgpu_info_ioctl() warn: check that 'dev_info' doesn't leak information (struct has a hole after 'pa_sc_tile_steering_override')
drivers/block/floppy.c:3132 raw_cmd_copyout() warn: check that 'cmd' doesn't leak information (struct has a hole after 'flags')
drivers/char/hpet.c:675 hpet_ioctl() warn: check that 'info' doesn't leak information (struct has a hole after 'hi_timer')
drivers/media/v4l2-core/v4l2-ioctl.c:3204 video_put_user() warn: check that 'ev32' doesn't leak information (struct has a hole after 'type')
drivers/media/v4l2-core/v4l2-ioctl.c:3229 video_put_user() warn: check that 'vb32' doesn't leak information (struct has a hole after 'memory')
drivers/net/wan/sbni.c:1320 sbni_ioctl() warn: check that 'flags' doesn't leak information (struct has a hole after 'rxl')
drivers/infiniband/hw/qedr/verbs.c:1816 qedr_create_user_qp() warn: check that 'uresp' doesn't leak information (struct has a hole after 'sq_icid')
drivers/infiniband/hw/cxgb4/provider.c:107 c4iw_alloc_ucontext() warn: check that 'uresp.reserved' doesn't leak information
drivers/tty/vt/vt_ioctl.c:1218 vt_compat_ioctl() warn: check that 'op' doesn't leak information (struct has a hole after 'charcount')
net/smc/smc_diag.c:181 __smc_diag_dump() warn: check that 'dinfo' doesn't leak information (struct has a hole after 'linkid')
net/sched/act_ife.c:638 tcf_ife_dump() warn: check that 'opt' doesn't leak information (struct has a hole after 'flags')
net/sched/act_skbmod.c:232 tcf_skbmod_dump() warn: check that 'opt' doesn't leak information (struct has a hole after 'bindcnt')
net/sched/act_connmark.c:187 tcf_connmark_dump() warn: check that 'opt' doesn't leak information (struct has a hole after 'zone')
net/openvswitch/conntrack.c:311 ovs_ct_put_key() warn: check that 'orig' doesn't leak information (struct has a hole after 'ipv4_proto')
net/openvswitch/conntrack.c:322 ovs_ct_put_key() warn: check that 'orig' doesn't leak information (struct has a hole after 'ipv6_proto')
net/rds/recv.c:492 rds_notify_queue_get() warn: check that 'cmsg' doesn't leak information (struct has a hole after 'status')
net/xdp/xsk.c:870 xsk_getsockopt() warn: check that 'stats.rx_ring_full' doesn't leak information
kernel/signal.c:3524 __do_sys_rt_sigtimedwait() warn: check that 'info' doesn't leak information (struct has a hole after 'si_code')
kernel/signal.c:3524 __do_sys_rt_sigtimedwait() warn: check that 'info' doesn't leak information (struct has a hole after 'si_code')
kernel/signal.c:3556 __do_sys_rt_sigtimedwait_time32() warn: check that 'info' doesn't leak information (struct has a hole after 'si_code')
kernel/signal.c:3556 __do_sys_rt_sigtimedwait_time32() warn: check that 'info' doesn't leak information (struct has a hole after 'si_code')
kernel/signal.c:4055 __do_sys_sigaltstack() warn: check that 'old' doesn't leak information (struct has a hole after 'ss_flags')
kernel/signal.c:4055 __do_sys_sigaltstack() warn: check that 'old' doesn't leak information (struct has a hole after 'ss_flags')
kernel/ptrace.c:998 ptrace_get_syscall_info() warn: check that 'info' doesn't leak information (struct has a hole after 'op')

regards,
dan carpenter

WARNING: multiple messages have this Message-ID (diff)
From: Dan Carpenter <dan.carpenter@oracle.com>
To: Arnd Bergmann <arnd@arndb.de>
Cc: "Niklas Söderlund" <niklas.soderlund+renesas@ragnatech.se>,
	syzkaller-bugs <syzkaller-bugs@googlegroups.com>,
	"Linux Media Mailing List" <linux-media@vger.kernel.org>,
	"linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
	"Laurent Pinchart" <laurent.pinchart@ideasonboard.com>,
	"Sakari Ailus" <sakari.ailus@linux.intel.com>,
	"Vandana BN" <bnvandana@gmail.com>,
	"Hans Verkuil" <hverkuil-cisco@xs4all.nl>,
	"Mauro Carvalho Chehab" <mchehab@kernel.org>,
	"Ezequiel Garcia" <ezequiel@collabora.com>,
	"Peilin Ye" <yepeilin.cs@gmail.com>,
	linux-kernel-mentees@lists.linuxfoundation.org
Subject: Re: [Linux-kernel-mentees] [PATCH v3] media/v4l2-core: Fix kernel-infoleak in video_put_user()
Date: Mon, 27 Jul 2020 16:16:08 +0300	[thread overview]
Message-ID: <20200727131608.GD1913@kadam> (raw)
In-Reply-To: <CAK8P3a3NB2BVo9fH-Wcinrhhs-QJ=9dK59Ds83TvgLmEkRy3qA@mail.gmail.com>

On Mon, Jul 27, 2020 at 09:25:16AM +0200, Arnd Bergmann wrote:
> On Mon, Jul 27, 2020 at 12:28 AM Peilin Ye <yepeilin.cs@gmail.com> wrote:
> >
> > video_put_user() is copying uninitialized stack memory to userspace due
> > to the compiler not initializing holes in the structures declared on the
> > stack. Fix it by initializing `ev32` and `vb32` using memset().
> >
> > Reported-and-tested-by: syzbot+79d751604cb6f29fbf59@syzkaller.appspotmail.com
> > Link: https://syzkaller.appspot.com/bug?extid=79d751604cb6f29fbf59
> > Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> > Signed-off-by: Peilin Ye <yepeilin.cs@gmail.com>
> 
> Thanks a lot for addressing this! I now see that I actually created a similar
> bugfix for it back in January, but for some reason that got stuck in my
> backlog and I never wrote a proper description for it or sent it out to the
> list, sorry about that. I would hope we could find a way to have either
> the compiler or sparse warn if we copy uninitialized data to user space,
> but we now don't even check for that within the kernel any more.

Here are my latest warnings on linux-next from Friday.

block/scsi_ioctl.c:707 scsi_put_cdrom_generic_arg() warn: check that 'cgc32' doesn't leak information (struct has a hole after 'data_direction')
drivers/input/misc/uinput.c:743 uinput_ff_upload_to_user() warn: check that 'ff_up_compat' doesn't leak information (struct has a hole after 'replay')
drivers/input/misc/uinput.c:958 uinput_ioctl_handler() warn: check that 'ff_up' doesn't leak information (struct has a hole after 'replay')
drivers/firewire/core-cdev.c:463 ioctl_get_info() warn: check that 'bus_reset' doesn't leak information (struct has a hole after 'generation')
drivers/scsi/megaraid/megaraid_mm.c:824 kioc_to_mimd() warn: check that 'cinfo.base' doesn't leak information
drivers/gpio/gpiolib-cdev.c:473 lineevent_read() warn: check that 'ge' doesn't leak information (struct has a hole after 'id')
drivers/gpu/drm/i915/i915_query.c:136 query_engine_info() warn: check that 'query.num_engines' doesn't leak information
drivers/gpu/drm/drm_bufs.c:1357 copy_one_buf() warn: check that 'v' doesn't leak information (struct has a hole after 'flags')
drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c:785 amdgpu_info_ioctl() warn: check that 'dev_info' doesn't leak information (struct has a hole after 'pa_sc_tile_steering_override')
drivers/block/floppy.c:3132 raw_cmd_copyout() warn: check that 'cmd' doesn't leak information (struct has a hole after 'flags')
drivers/char/hpet.c:675 hpet_ioctl() warn: check that 'info' doesn't leak information (struct has a hole after 'hi_timer')
drivers/media/v4l2-core/v4l2-ioctl.c:3204 video_put_user() warn: check that 'ev32' doesn't leak information (struct has a hole after 'type')
drivers/media/v4l2-core/v4l2-ioctl.c:3229 video_put_user() warn: check that 'vb32' doesn't leak information (struct has a hole after 'memory')
drivers/net/wan/sbni.c:1320 sbni_ioctl() warn: check that 'flags' doesn't leak information (struct has a hole after 'rxl')
drivers/infiniband/hw/qedr/verbs.c:1816 qedr_create_user_qp() warn: check that 'uresp' doesn't leak information (struct has a hole after 'sq_icid')
drivers/infiniband/hw/cxgb4/provider.c:107 c4iw_alloc_ucontext() warn: check that 'uresp.reserved' doesn't leak information
drivers/tty/vt/vt_ioctl.c:1218 vt_compat_ioctl() warn: check that 'op' doesn't leak information (struct has a hole after 'charcount')
net/smc/smc_diag.c:181 __smc_diag_dump() warn: check that 'dinfo' doesn't leak information (struct has a hole after 'linkid')
net/sched/act_ife.c:638 tcf_ife_dump() warn: check that 'opt' doesn't leak information (struct has a hole after 'flags')
net/sched/act_skbmod.c:232 tcf_skbmod_dump() warn: check that 'opt' doesn't leak information (struct has a hole after 'bindcnt')
net/sched/act_connmark.c:187 tcf_connmark_dump() warn: check that 'opt' doesn't leak information (struct has a hole after 'zone')
net/openvswitch/conntrack.c:311 ovs_ct_put_key() warn: check that 'orig' doesn't leak information (struct has a hole after 'ipv4_proto')
net/openvswitch/conntrack.c:322 ovs_ct_put_key() warn: check that 'orig' doesn't leak information (struct has a hole after 'ipv6_proto')
net/rds/recv.c:492 rds_notify_queue_get() warn: check that 'cmsg' doesn't leak information (struct has a hole after 'status')
net/xdp/xsk.c:870 xsk_getsockopt() warn: check that 'stats.rx_ring_full' doesn't leak information
kernel/signal.c:3524 __do_sys_rt_sigtimedwait() warn: check that 'info' doesn't leak information (struct has a hole after 'si_code')
kernel/signal.c:3524 __do_sys_rt_sigtimedwait() warn: check that 'info' doesn't leak information (struct has a hole after 'si_code')
kernel/signal.c:3556 __do_sys_rt_sigtimedwait_time32() warn: check that 'info' doesn't leak information (struct has a hole after 'si_code')
kernel/signal.c:3556 __do_sys_rt_sigtimedwait_time32() warn: check that 'info' doesn't leak information (struct has a hole after 'si_code')
kernel/signal.c:4055 __do_sys_sigaltstack() warn: check that 'old' doesn't leak information (struct has a hole after 'ss_flags')
kernel/signal.c:4055 __do_sys_sigaltstack() warn: check that 'old' doesn't leak information (struct has a hole after 'ss_flags')
kernel/ptrace.c:998 ptrace_get_syscall_info() warn: check that 'info' doesn't leak information (struct has a hole after 'op')

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

  parent reply	other threads:[~2020-07-27 13:17 UTC|newest]

Thread overview: 76+ 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 16:44 ` Peilin Ye
2020-07-26 17:30 ` Laurent Pinchart
2020-07-26 17:30   ` Laurent Pinchart
2020-07-26 18:07   ` Peilin Ye
2020-07-26 18:07     ` Peilin Ye
2020-07-26 22:08     ` Laurent Pinchart
2020-07-26 22:08       ` Laurent Pinchart
2020-07-26 22:15       ` Peilin Ye
2020-07-26 22:15         ` Peilin Ye
2020-07-26 18:12   ` 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:05   ` Peilin Ye
2020-07-26 22:10   ` Laurent Pinchart
2020-07-26 22:10     ` Laurent Pinchart
2020-07-26 22:16     ` Peilin Ye
2020-07-26 22:16       ` Peilin Ye
2020-07-26 22:27   ` [Linux-kernel-mentees] [PATCH v3] " Peilin Ye
2020-07-26 22:27     ` Peilin Ye
2020-07-27  7:25     ` Arnd Bergmann
2020-07-27  7:25       ` Arnd Bergmann
2020-07-27  7:56       ` Peilin Ye
2020-07-27  7:56         ` Peilin Ye
2020-07-27 13:16       ` Dan Carpenter [this message]
2020-07-27 13:16         ` Dan Carpenter
2020-07-27 14:05         ` Arnd Bergmann
2020-07-27 14:05           ` Arnd Bergmann
2020-07-27 14:14           ` Peilin Ye
2020-07-27 14:14             ` Peilin Ye
2020-07-27 14:20             ` Arnd Bergmann
2020-07-27 14:20               ` Arnd Bergmann
2020-07-27 14:46             ` Dan Carpenter
2020-07-27 14:46               ` Dan Carpenter
2020-07-27 15:30               ` Peilin Ye
2020-07-27 15:30                 ` Peilin Ye
2020-07-27 14:43           ` Dan Carpenter
2020-07-27 14:43             ` Dan Carpenter
2020-07-27 14:55             ` Arnd Bergmann
2020-07-27 14:55               ` Arnd Bergmann
2020-07-27 22:04         ` Peilin Ye
2020-07-27 22:04           ` Peilin Ye
2020-07-28  9:00           ` Arnd Bergmann
2020-07-28  9:00             ` Arnd Bergmann
2020-07-28 10:02           ` Dan Carpenter
2020-07-28 10:02             ` Dan Carpenter
2020-07-27 22:33         ` Peilin Ye
2020-07-27 22:33           ` Peilin Ye
2020-07-28  9:10           ` Arnd Bergmann
2020-07-28  9:10             ` Arnd Bergmann
2020-07-28  9:47           ` Dan Carpenter
2020-07-28  9:47             ` Dan Carpenter
2020-07-28 13:13             ` Peilin Ye
2020-07-28 13:13               ` Peilin Ye
2020-07-28 12:22         ` Linus Walleij
2020-07-28 12:22           ` Linus Walleij
2020-07-28 13:06           ` Dan Carpenter
2020-07-28 13:06             ` Dan Carpenter
2020-07-28 13:58             ` Arnd Bergmann
2020-07-28 13:58               ` Arnd Bergmann
2020-07-30  8:07               ` Bartosz Golaszewski
2020-07-30  8:07                 ` Bartosz Golaszewski
2020-07-30  8:15                 ` Arnd Bergmann
2020-07-30  8:15                   ` Arnd Bergmann
2020-07-30  8:38                   ` Andy Shevchenko
2020-07-30  8:38                     ` Andy Shevchenko
2020-07-30  9:18                     ` Arnd Bergmann
2020-07-30  9:18                       ` Arnd Bergmann
2020-07-30 11:48                       ` Andy Shevchenko
2020-07-30 11:48                         ` Andy Shevchenko
2020-07-30 13:49                         ` Arnd Bergmann
2020-07-30 13:49                           ` Arnd Bergmann
2020-08-02 16:55         ` Peilin Ye
2020-08-02 16:55           ` Peilin Ye
2020-07-27  8:00     ` [Linux-kernel-mentees] [PATCH v4] " Peilin Ye
2020-07-27  8:00       ` 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=20200727131608.GD1913@kadam \
    --to=dan.carpenter@oracle.com \
    --cc=arnd@arndb.de \
    --cc=bnvandana@gmail.com \
    --cc=ezequiel@collabora.com \
    --cc=gregkh@linuxfoundation.org \
    --cc=hverkuil-cisco@xs4all.nl \
    --cc=laurent.pinchart@ideasonboard.com \
    --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 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.