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

Pre-existing issues:
- [High] `vmbus_teardown_gpadl` silently overwrites its error code on failure, 
hiding GPADL teardown failures and defeating the memory leak safeguard.
--

commit 67f15c255388157724c433ac1cc78395f8454d37
Author: Michael Kelley <[email protected]>

hv_netvsc: Leak send/recv buffers if GPADL teardown fails

This commit updates the receive and send buffer teardown paths to call
vmbus_leak_buffer() when GPADL teardown fails. This ensures the buffers
are intentionally leaked rather than freed while the Hyper-V host might
still retain access to them.

Link: 
https://lore.kernel.org/linux-hyperv/[email protected]/

> diff --git a/drivers/net/hyperv/netvsc.c b/drivers/net/hyperv/netvsc.c
> index 5cd084e5696cd..449dc928cc445 100644
> --- a/drivers/net/hyperv/netvsc.c
> +++ b/drivers/net/hyperv/netvsc.c
> @@ -316,6 +316,9 @@ static void netvsc_teardown_recv_gpadl(struct hv_device 
> *device,
>                * rather than continue and a bugchk
>                */
>               if (ret != 0) {

[Severity: High]
This is a pre-existing issue, but this intended safeguard appears to be
ineffective because vmbus_teardown_gpadl() silently overwrites its error
code on failure.

If vmbus_post_msg() fails inside vmbus_teardown_gpadl(), the original error
code in ret is unconditionally overwritten with 0 when gpadl->decrypted is
false (the standard VM case):

drivers/hv/channel.c:vmbus_teardown_gpadl() {
    ...
post_msg_err:
        if (gpadl->decrypted)
                ret = set_memory_encrypted((unsigned long)gpadl->buffer,
                                           PFN_UP(gpadl->size));
        else
                ret = 0;
        ...
        return ret;
}

Because vmbus_teardown_gpadl() returns success despite failing, the
if (ret != 0) condition here will not evaluate to true, and the memory leak
safeguard will be skipped. Will this allow the buffers to be freed while the
Hyper-V host retains access, potentially causing memory corruption or a
use-after-free?

> +                     vmbus_leak_buffer(&net_device->recv_buf,
> +                                       &net_device->recv_buf_chunks,
> +                                       &net_device->recv_buf_chunk_cnt);
>                       netdev_err(ndev,
>                                  "unable to teardown receive buffer's 
> gpadl\n");
>                       return;

-- 
Sashiko AI review ยท 
https://sashiko.dev/#/patchset/[email protected]?part=2

Reply via email to