LKML Archive on lore.kernel.org
help / color / mirror / Atom feed
From: "Michael S. Tsirkin" <mst@redhat.com>
To: Al Viro <viro@zeniv.linux.org.uk>
Cc: syzbot <syzbot+87cfa083e727a224754b@syzkaller.appspotmail.com>,
	avagin@openvz.org, davem@davemloft.net, dingtianhong@huawei.com,
	edumazet@google.com, elena.reshetova@intel.com,
	jasowang@redhat.com, kvm@vger.kernel.org,
	linux-kernel@vger.kernel.org, matthew@mjdsystems.ca,
	mingo@kernel.org, netdev@vger.kernel.org, pabeni@redhat.com,
	syzkaller-bugs@googlegroups.com,
	virtualization@lists.linux-foundation.org, willemb@google.com
Subject: Re: KMSAN: uninit-value in _copy_to_iter (2)
Date: Thu, 7 Jun 2018 22:29:13 +0300	[thread overview]
Message-ID: <20180607222728-mutt-send-email-mst@kernel.org> (raw)
In-Reply-To: <20180607180449.GS30522@ZenIV.linux.org.uk>

On Thu, Jun 07, 2018 at 07:04:49PM +0100, Al Viro wrote:
> On Thu, Jun 07, 2018 at 08:59:06PM +0300, Michael S. Tsirkin wrote:
> > On Thu, Jun 07, 2018 at 06:43:55PM +0100, Al Viro wrote:
> > > On Thu, Jun 07, 2018 at 06:38:48PM +0300, Michael S. Tsirkin wrote:
> > > > #syz test: https://github.com/google/kmsan.git/master d2d741e5d1898dfde1a75ea3d29a9a3e2edf0617
> > > > 
> > > > Subject: vhost: fix info leak
> > > > 
> > > > Fixes: CVE-2018-1118
> > > > Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
> > > > ---
> > > > diff --git a/drivers/vhost/vhost.c b/drivers/vhost/vhost.c
> > > > index f0be5f35ab28..9beefa6ed1ce 100644
> > > > --- a/drivers/vhost/vhost.c
> > > > +++ b/drivers/vhost/vhost.c
> > > > @@ -2345,6 +2345,9 @@ struct vhost_msg_node *vhost_new_msg(struct vhost_virtqueue *vq, int type)
> > > >  	struct vhost_msg_node *node = kmalloc(sizeof *node, GFP_KERNEL);
> > > >  	if (!node)
> > > >  		return NULL;
> > > > +
> > > > +	/* Make sure all padding within the structure is initialized. */
> > > > +	memset(&node->msg, 0, sizeof node->msg);
> > > 
> > > Umm...  Maybe kzalloc(), then?  You have
> > > 
> > > struct vhost_msg_node {
> > >   struct vhost_msg msg;
> > >   struct vhost_virtqueue *vq;
> > >   struct list_head node;
> > > };
> > > 
> > > and that's what, 68 bytes in msg, then either 4 bytes pointer or
> > > 4 bytes padding + 8 bytes pointer, then two pointers?  How much
> > > does explicit partial memset() save you here?
> > 
> > Yes but 0 isn't a nop here so if this struct is used without
> > a sensible initialization, it will crash elsewhere.
> > I prefer KASAN to catch such uses.
> > 
> > 
> > > >  	node->vq = vq;
> > > >  	node->msg.type = type;
> 
> IDGI - what would your variant catch that kzalloc + 2 assignments won't?
> Accesses to uninitialized ->node?  Because that's the only difference in
> what is and is not initialized between those variants...

For now yes but we'll likely add more fields in this structure
down the road, which is where I'd expect new bugs to come from.

-- 
MST

  reply	other threads:[~2018-06-07 19:29 UTC|newest]

Thread overview: 30+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2018-04-23 16:56 syzbot
2018-04-25 19:19 ` syzbot
2018-06-07 15:38   ` Michael S. Tsirkin
2018-06-07 15:38     ` syzbot
2018-06-07 16:25     ` Dmitry Vyukov
2018-06-07 17:04       ` syzbot
2018-06-07 17:43     ` Al Viro
2018-06-07 17:59       ` Michael S. Tsirkin
2018-06-07 18:04         ` Al Viro
2018-06-07 19:29           ` Michael S. Tsirkin [this message]
2018-06-07 17:10   ` Michael S. Tsirkin
2018-06-07 17:31     ` syzbot
2018-04-27 15:45 ` [PATCH net] vhost: Use kzalloc() to allocate vhost_msg_node Kevin Easton
2018-04-27 16:05   ` Michael S. Tsirkin
2018-04-27 16:11     ` Dmitry Vyukov
2018-04-27 16:15       ` Michael S. Tsirkin
2018-04-27 16:25         ` Dmitry Vyukov
2018-04-27 16:29           ` Dmitry Vyukov
2018-04-27 19:36       ` Michael S. Tsirkin
2018-04-29  8:10         ` Dmitry Vyukov
2018-04-28  1:07     ` Kevin Easton
2018-04-28  1:51       ` Kevin Easton
2018-04-28  2:23         ` Jason Wang
2018-05-07 13:03   ` Michael S. Tsirkin
2018-05-07 13:12     ` Dmitry Vyukov
2018-05-08  8:27     ` Kevin Easton
2018-05-29 22:19   ` [net] " Guenter Roeck
2018-05-30  3:01     ` Michael S. Tsirkin
2018-05-30  3:42       ` Guenter Roeck
2018-06-04 12:34       ` Dmitry Vyukov

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=20180607222728-mutt-send-email-mst@kernel.org \
    --to=mst@redhat.com \
    --cc=avagin@openvz.org \
    --cc=davem@davemloft.net \
    --cc=dingtianhong@huawei.com \
    --cc=edumazet@google.com \
    --cc=elena.reshetova@intel.com \
    --cc=jasowang@redhat.com \
    --cc=kvm@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=matthew@mjdsystems.ca \
    --cc=mingo@kernel.org \
    --cc=netdev@vger.kernel.org \
    --cc=pabeni@redhat.com \
    --cc=syzbot+87cfa083e727a224754b@syzkaller.appspotmail.com \
    --cc=syzkaller-bugs@googlegroups.com \
    --cc=viro@zeniv.linux.org.uk \
    --cc=virtualization@lists.linux-foundation.org \
    --cc=willemb@google.com \
    --subject='Re: KMSAN: uninit-value in _copy_to_iter (2)' \
    /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

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).