Re: [PATCH v2 1/2] Drivers: hv: vmbus: add vmbus_establish_gpadl_caller_decrypted()

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:

Pre-existing issues:
- [High] In `vmbus_teardown_gpadl()`, if `vmbus_post_msg()` fails, the resulting error code is unconditionally overwritten, causing the function to silently return success instead of propagating the failure.
--

--- Patch [1]: [PATCH v2 1/2] Drivers: hv: vmbus: add vmbus_establish_gpadl_caller_decrypted() ---
commit aab401fb7e15b1b2168ba8341d7d172ef6d0525f
Author: Kameron Carr <kameroncarr@linux.microsoft.com>

Drivers: hv: vmbus: add vmbus_establish_gpadl_caller_decrypted()

This commit introduces a new function for callers to establish a GPADL for a
buffer they have already decrypted themselves. It adds a new GPADL type
HV_GPADL_BUFFER_DECRYPTED to track this decryption status. Existing callers
are unaffected by this change.

> diff --git a/drivers/hv/channel.c b/drivers/hv/channel.c
> index 6821f225248b1..4782f5070bba9 100644
> --- a/drivers/hv/channel.c
> +++ b/drivers/hv/channel.c

[ ... ]

> @@ -460,7 +462,8 @@ static int __vmbus_establish_gpadl(struct vmbus_channel *channel,
>  	}
>  
>  	gpadl->decrypted = !((channel->co_external_memory && type == HV_GPADL_BUFFER) ||
> -		(channel->co_ring_buffer && type == HV_GPADL_RING));
> +		(channel->co_ring_buffer && type == HV_GPADL_RING) ||
> +		(type == HV_GPADL_BUFFER_DECRYPTED));
>  	if (gpadl->decrypted) {
>  		/*
>  		 * The "decrypted" flag being true assumes that set_memory_decrypted() succeeds.

[Severity: High]
This is a pre-existing issue, but I noticed a potential regression in how
GPADL teardowns are handled.

Looking at drivers/hv/channel.c in vmbus_teardown_gpadl(), if the call to
vmbus_post_msg() fails (for example, returning -ENOBUFS due to transient
hypervisor resource exhaustion), the code jumps to the post_msg_err label.
However, the error code in ret is unconditionally overwritten at the end of
the function:

post_msg_err:
    ...
    if (gpadl->decrypted)
        ret = set_memory_encrypted((unsigned long)gpadl->buffer,
                    PFN_UP(gpadl->size));
    else
        ret = 0;

Can this mask the communication failure with the hypervisor?

Because the failure is masked, the caller incorrectly assumes the hypervisor
has successfully torn down the GPADL and frees the memory back to the guest
page allocator, but the hypervisor still retains the GPADL mapping and may
continue to write to it.

Could this lead to guest memory corruption or a use-after-free when the
hypervisor accesses the released memory?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260730233359.3850612-1-kameroncarr@linux.microsoft.com?part=1