2021-02-24 13:44:33

by Sasha Levin

Subject: [PATCH AUTOSEL 5.10 40/56] Drivers: hv: vmbus: Initialize memory to be sent to the host

From: "Andrea Parri (Microsoft)" <[email protected]>

[ Upstream commit e99c4afbee07e9323e9191a20b24d74dbf815bdf ]

__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 <[email protected]>
Signed-off-by: Andrea Parri (Microsoft) <[email protected]>
Reviewed-by: Michael Kelley <[email protected]>
Link: https://lore.kernel.org/r/[email protected]
Signed-off-by: Wei Liu <[email protected]>
Signed-off-by: Sasha Levin <[email protected]>
---
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 fbdda9938039a..f9f04b5cd303f 100644
--- a/drivers/hv/channel.c
+++ b/drivers/hv/channel.c
@@ -548,7 +548,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) {
@@ -674,7 +674,7 @@ int vmbus_teardown_gpadl(struct vmbus_channel *channel, u32 gpadl_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.27.0

2021-02-24 14:44:01

by Andrea Parri

Subject: Re: [PATCH AUTOSEL 5.10 40/56] Drivers: hv: vmbus: Initialize memory to be sent to the host

On Wed, Feb 24, 2021 at 07:51:56AM -0500, Sasha Levin wrote:
> From: "Andrea Parri (Microsoft)" <[email protected]>
>
> [ Upstream commit e99c4afbee07e9323e9191a20b24d74dbf815bdf ]
>
> __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 <[email protected]>
> Signed-off-by: Andrea Parri (Microsoft) <[email protected]>
> Reviewed-by: Michael Kelley <[email protected]>
> Link: https://lore.kernel.org/r/[email protected]
> Signed-off-by: Wei Liu <[email protected]>
> Signed-off-by: Sasha Levin <[email protected]>

Same here.

Andrea


> ---
> 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 fbdda9938039a..f9f04b5cd303f 100644
> --- a/drivers/hv/channel.c
> +++ b/drivers/hv/channel.c
> @@ -548,7 +548,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) {
> @@ -674,7 +674,7 @@ int vmbus_teardown_gpadl(struct vmbus_channel *channel, u32 gpadl_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.27.0
>