RE: [PATCH 1/6] Drivers: hv: vmbus: Initialize memory to be sent to the host
From: Michael Kelley <hidden>
Date: 2020-12-06 17:00:38
Also in:
lkml
From: Andrea Parri (Microsoft) <parri.andrea@gmail.com> Sent: Wednesday, November 18, 2020 6:37 AM
quoted hunk ↗ jump to hunk
__vmbus_open() and vmbus_teardown_gpadl() do not inizialite the memory for the vmbus_channel_open_channel and the vmbus_channel_gpadl_teardown objects they allocate respectively. These objects contain padding bytes and fields that are left uninitialized and that are later sent to the host, potentially leaking guest data. Zero initialize such fields to avoid leaking sensitive information to the host. Reported-by: Juan Vazquez <redacted> Signed-off-by: Andrea Parri (Microsoft) <parri.andrea@gmail.com> --- drivers/hv/channel.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)diff --git a/drivers/hv/channel.c b/drivers/hv/channel.c index 0d63862d65518..9aa789e5f22bb 100644 --- a/drivers/hv/channel.c +++ b/drivers/hv/channel.c@@ -621,7 +621,7 @@ static int __vmbus_open(struct vmbus_channel *newchannel, goto error_clean_ring; /* Create and init the channel open message */ - open_info = kmalloc(sizeof(*open_info) + + open_info = kzalloc(sizeof(*open_info) + sizeof(struct vmbus_channel_open_channel), GFP_KERNEL); if (!open_info) {@@ -748,7 +748,7 @@ int vmbus_teardown_gpadl(struct vmbus_channel *channel, u32gpadl_handle) unsigned long flags; int ret; - info = kmalloc(sizeof(*info) + + info = kzalloc(sizeof(*info) + sizeof(struct vmbus_channel_gpadl_teardown), GFP_KERNEL); if (!info) return -ENOMEM; -- 2.25.1
This change is actually zero'ing more memory than is necessary. Only the 'msg' portion is sent to Hyper-V, so that's all that needs to be zero'ed. But this code is not performance sensitive, and doing the tighter zero'ing would add lines of code with no real value. So, Reviewed-by: Michael Kelley <redacted>