`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.
Reported-by: [email protected]
Fixes: 06164d2b72aa ("VMCI: queue pairs implementation.")
Signed-off-by: Alexander Potapenko <[email protected]>
---
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
> On Nov 4, 2022, at 10:58 AM, Alexander Potapenko <[email protected]> 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: [email protected]
> Fixes: 06164d2b72aa ("VMCI: queue pairs implementation.")
> Signed-off-by: Alexander Potapenko <[email protected]>
Reviewed-by: Vishnu Dasa <[email protected]>
> ---
> 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
>