Re: [PATCH v2 1/2] Drivers: hv: vmbus: add vmbus_establish_gpadl_caller_decrypted()
[email protected] Fri, 31 Jul 2026 23:34:56 +0000
| Newsgroups | org.kernel.vger.linux-hyperv |
|---|---|
| Message-ID | <[email protected]> |
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 resu= lting error code is unconditionally overwritten, causing the function to si= lently 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_chann= el *channel, > } > =20 > gpadl->decrypted =3D !((channel->co_external_memory && type =3D=3D HV_G= PADL_BUFFER) || > - (channel->co_ring_buffer && type =3D=3D HV_GPADL_RING)); > + (channel->co_ring_buffer && type =3D=3D HV_GPADL_RING) || > + (type =3D=3D 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 =3D set_memory_encrypted((unsigned long)gpadl->buffer, PFN_UP(gpadl->size)); else ret =3D 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? --=20 Sashiko AI review =C2=B7 https://sashiko.dev/#/patchset/20260730233359.3850= [email protected]?part=3D1