Re: [PATCH 6.1] Drivers: hv: vmbus: Leak pages if set_memory_encrypted() fails

[Xiangyu Chen <xiangyu.chen@windriver.com>]

Hello Greg,


On 10/9/24 21:33, Greg KH wrote:
> CAUTION: This email comes from a non Wind River email account!
> Do not click links or open attachments unless you recognize the sender and know the content is safe.
>
> On Wed, Oct 09, 2024 at 04:16:26PM +0800, Xiangyu Chen wrote:
>> From: Rick Edgecombe <rick.p.edgecombe@intel.com>
>>
>> In CoCo VMs it is possible for the untrusted host to cause
>> set_memory_encrypted() or set_memory_decrypted() to fail such that an
>> error is returned and the resulting memory is shared. Callers need to
>> take care to handle these errors to avoid returning decrypted (shared)
>> memory to the page allocator, which could lead to functional or security
>> issues.
>>
>> VMBus code could free decrypted pages if set_memory_encrypted()/decrypted()
>> fails. Leak the pages if this happens.
>>
>> Signed-off-by: Rick Edgecombe <rick.p.edgecombe@intel.com>
>> Signed-off-by: Michael Kelley <mhklinux@outlook.com>
>> Reviewed-by: Kuppuswamy Sathyanarayanan <sathyanarayanan.kuppuswamy@linux.intel.com>
>> Acked-by: Kirill A. Shutemov <kirill.shutemov@linux.intel.com>
>> Link: https://lore.kernel.org/r/20240311161558.1310-2-mhklinux@outlook.com
>> Signed-off-by: Wei Liu <wei.liu@kernel.org>
>> Message-ID: <20240311161558.1310-2-mhklinux@outlook.com>
>> [Xiangyu: Modified to apply on 6.1.y]
>> Signed-off-by: Xiangyu Chen <xiangyu.chen@windriver.com>
>> ---
>>   drivers/hv/connection.c | 11 ++++++++++-
>>   1 file changed, 10 insertions(+), 1 deletion(-)
> Are you sure?  This is _VERY_ different from what you suggested for
> 5.15.y and what is in mainline.  Also, you didn't show the git id for
> the upstream commit.


This commit is a fix for CVE-2024-36913,  currently, if we fully apply 
the commit, we have to backport the

following commits from upstream:

a5ddb745 : Drivers: hv: vmbus: Remove second mapping of VMBus monitor pages

d786e00d : drivers: hv, hyperv_fb: Untangle and refactor Hyper-V panic 
notifiers

9c318a1d : Drivers: hv: move panic report code from vmbus to hv early 
init code

a6fe0438 : Drivers: hv: Change hv_free_hyperv_page() to take void * argument

03f5a999 : Drivers: hv: vmbus: Leak pages if set_memory_encrypted() fails

Some of them are features, it might not be merged to current stable 
branch, so another solution

is modify the connect.c by manual.


 From the upstream commit 03f5a999(Drivers: hv: vmbus: Leak pages if 
set_memory_encrypted() fails) we can see that

the commit aim to check set_memory_decrypted() result, if fails, the 
encryption of memory state is unknown, so leak the

memory.

The commit modified 2 functions, vmbus_connect() and vmbus_disconnect().

In vmbus_connect(), when set_memory_decrypted() fails, marking 
vmbus_connection.monitor_pages[0]/[1] to NULL.

In vmbus_disconnect(),  checking the monitor_pages[0]/[1] is valid, and 
checking set_memory_encrypted() status, if fails, free and

leak it.

On current v6.1 branch, vmbus_disconnect() will free those memory 
whatever set_memory_encrypted() is success or fails, so we can just

add a monitor_pages valid checking in it.


Could you please give a suggestion that which solution is following the 
stable-branch rule, I will resend a V2 patch, thanks.



Br,

Xiangyu

>
> Please work to figure this out and resend working versions for ALL
> affected branches as new patches.
>
> thanks,
>
> greg k-h