All of lore.kernel.org
 help / color / mirror / Atom feed
From: Vishnu Dasa <vdasa@vmware.com>
To: Alexander Potapenko <glider@google.com>
Cc: "linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
	"gregkh@linuxfoundation.org" <gregkh@linuxfoundation.org>,
	"georgezhang@vmware.com" <georgezhang@vmware.com>,
	Bryan Tan <bryantan@vmware.com>,
	Pv-drivers <Pv-drivers@vmware.com>,
	"syzbot+39be4da489ed2493ba25@syzkaller.appspotmail.com" 
	<syzbot+39be4da489ed2493ba25@syzkaller.appspotmail.com>
Subject: Re: [PATCH] misc/vmw_vmci: fix an infoleak in vmci_host_do_receive_datagram()
Date: Mon, 7 Nov 2022 19:46:42 +0000	[thread overview]
Message-ID: <86C4301A-585C-40EF-9939-DAE5671DF5BF@vmware.com> (raw)
In-Reply-To: <20221104175849.2782567-1-glider@google.com>



> On Nov 4, 2022, at 10:58 AM, Alexander Potapenko <glider@google.com> wrote:
> 
> `struct vmci_event_qp` allocated by qp_notify_peer() contains padding,
> which may carry uninitialized data to the userspace, as observed by
> KMSAN:
> 
>  BUG: KMSAN: kernel-infoleak in instrument_copy_to_user ./include/linux/instrumented.h:121
>   instrument_copy_to_user ./include/linux/instrumented.h:121
>   _copy_to_user+0x5f/0xb0 lib/usercopy.c:33
>   copy_to_user ./include/linux/uaccess.h:169
>   vmci_host_do_receive_datagram drivers/misc/vmw_vmci/vmci_host.c:431
>   vmci_host_unlocked_ioctl+0x33d/0x43d0 drivers/misc/vmw_vmci/vmci_host.c:925
>   vfs_ioctl fs/ioctl.c:51
>  ...
> 
>  Uninit was stored to memory at:
>   kmemdup+0x74/0xb0 mm/util.c:131
>   dg_dispatch_as_host drivers/misc/vmw_vmci/vmci_datagram.c:271
>   vmci_datagram_dispatch+0x4f8/0xfc0 drivers/misc/vmw_vmci/vmci_datagram.c:339
>   qp_notify_peer+0x19a/0x290 drivers/misc/vmw_vmci/vmci_queue_pair.c:1479
>   qp_broker_attach drivers/misc/vmw_vmci/vmci_queue_pair.c:1662
>   qp_broker_alloc+0x2977/0x2f30 drivers/misc/vmw_vmci/vmci_queue_pair.c:1750
>   vmci_qp_broker_alloc+0x96/0xd0 drivers/misc/vmw_vmci/vmci_queue_pair.c:1940
>   vmci_host_do_alloc_queuepair drivers/misc/vmw_vmci/vmci_host.c:488
>   vmci_host_unlocked_ioctl+0x24fd/0x43d0 drivers/misc/vmw_vmci/vmci_host.c:927
>  ...
> 
>  Local variable ev created at:
>   qp_notify_peer+0x54/0x290 drivers/misc/vmw_vmci/vmci_queue_pair.c:1456
>   qp_broker_attach drivers/misc/vmw_vmci/vmci_queue_pair.c:1662
>   qp_broker_alloc+0x2977/0x2f30 drivers/misc/vmw_vmci/vmci_queue_pair.c:1750
> 
>  Bytes 28-31 of 48 are uninitialized
>  Memory access of size 48 starts at ffff888035155e00
>  Data copied to user address 0000000020000100
> 
> Use memset() to prevent the infoleaks.
> 
> Also speculatively fix qp_notify_peer_local(), which may suffer from the
> same problem.
> 

Thanks for the fix!

Regards,
Vishnu

> Reported-by: syzbot+39be4da489ed2493ba25@syzkaller.appspotmail.com
> Fixes: 06164d2b72aa ("VMCI: queue pairs implementation.")
> Signed-off-by: Alexander Potapenko <glider@google.com>

Reviewed-by: Vishnu Dasa <vdasa@vmware.com>

> ---
> drivers/misc/vmw_vmci/vmci_queue_pair.c | 2 ++
> 1 file changed, 2 insertions(+)
> 
> diff --git a/drivers/misc/vmw_vmci/vmci_queue_pair.c b/drivers/misc/vmw_vmci/vmci_queue_pair.c
> index e71068f7759b3..844264e1b88cc 100644
> --- a/drivers/misc/vmw_vmci/vmci_queue_pair.c
> +++ b/drivers/misc/vmw_vmci/vmci_queue_pair.c
> @@ -854,6 +854,7 @@ static int qp_notify_peer_local(bool attach, struct vmci_handle handle)
> 	u32 context_id = vmci_get_context_id();
> 	struct vmci_event_qp ev;
> 
> +	memset(&ev, 0, sizeof(ev));
> 	ev.msg.hdr.dst = vmci_make_handle(context_id, VMCI_EVENT_HANDLER);
> 	ev.msg.hdr.src = vmci_make_handle(VMCI_HYPERVISOR_CONTEXT_ID,
> 					  VMCI_CONTEXT_RESOURCE_ID);
> @@ -1467,6 +1468,7 @@ static int qp_notify_peer(bool attach,
> 	 * kernel.
> 	 */
> 
> +	memset(&ev, 0, sizeof(ev));
> 	ev.msg.hdr.dst = vmci_make_handle(peer_id, VMCI_EVENT_HANDLER);
> 	ev.msg.hdr.src = vmci_make_handle(VMCI_HYPERVISOR_CONTEXT_ID,
> 					  VMCI_CONTEXT_RESOURCE_ID);
> -- 
> 2.38.1.431.g37b22c650d-goog
> 


      reply	other threads:[~2022-11-07 19:46 UTC|newest]

Thread overview: 2+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2022-11-04 17:58 [PATCH] misc/vmw_vmci: fix an infoleak in vmci_host_do_receive_datagram() Alexander Potapenko
2022-11-07 19:46 ` Vishnu Dasa [this message]

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=86C4301A-585C-40EF-9939-DAE5671DF5BF@vmware.com \
    --to=vdasa@vmware.com \
    --cc=Pv-drivers@vmware.com \
    --cc=bryantan@vmware.com \
    --cc=georgezhang@vmware.com \
    --cc=glider@google.com \
    --cc=gregkh@linuxfoundation.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=syzbot+39be4da489ed2493ba25@syzkaller.appspotmail.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.