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 <[email protected]> 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/[email protected]?part=1
