Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider: - [High] hv_netvsc: host bugcheck DoS due to GPADL teardown after failed revoke --
commit 418182ccc1e62bfbe8d59179d5fb1baf3aaa6d53 Author: Emerson Busson <[email protected]> hv: use owned VMBus buffers in NetVSC and UIO Move the remaining buffer consumers to the allocation, GPADL and release entry points that carry one descriptor throughout their lifetime. Preserve established exported signatures throughout the series. > diff --git a/drivers/net/hyperv/netvsc.c b/drivers/net/hyperv/netvsc.c > index ffba1443396ab..fd8aa7a3dcb32 100644 > --- a/drivers/net/hyperv/netvsc.c > +++ b/drivers/net/hyperv/netvsc.c > @@ -243,7 +243,6 @@ static void netvsc_revoke_recv_buf(struct hv_device > *device, > if (ret != 0) { > netdev_err(ndev, "unable to send " > "revoke receive buffer to netvsp\n"); > - net_device->recv_buffer.leak = true; > return; > } > net_device->recv_section_cnt = 0; > @@ -295,7 +294,6 @@ static void netvsc_revoke_send_buf(struct hv_device > *device, > if (ret != 0) { > netdev_err(ndev, "unable to send " > "revoke send buffer to netvsp\n"); > - net_device->send_buffer.leak = true; > return; > } > net_device->send_section_cnt = 0; > @@ -308,18 +306,14 @@ static void netvsc_teardown_recv_gpadl(struct hv_device > *device, > { > int ret; > > - if (net_device->recv_buffer.leak) > - return; > - > if (net_device->recv_buffer.gpadl.gpadl_handle) { > - ret = vmbus_teardown_gpadl(device->channel, > - &net_device->recv_buffer.gpadl); > + ret = vmbus_teardown_gpadl_owned(device->channel, > + &net_device->recv_buffer); [Severity: High] With the removal of the leak check here and the manual assignment in netvsc_revoke_recv_buf(), is there a risk of a host bugcheck if the protocol-level revoke message fails? If vmbus_sendpacket() returns an error (like -EAGAIN from a full ring) in netvsc_revoke_recv_buf(), the NetVSP layer won't receive the revoke message and might still actively map the GPADL. Unconditionally sending a VMBus GPADL teardown request in this scenario violates the protocol invariant. Could this lead to a Denial of Service via a host bugcheck or guest termination? > > /* If we failed here, we might as well return and have a leak > * rather than continue and a bugchk > */ > if (ret != 0) { > - net_device->recv_buffer.leak = true; > netdev_err(ndev, > "unable to teardown receive buffer's > gpadl\n"); > return; > @@ -333,18 +327,14 @@ static void netvsc_teardown_send_gpadl(struct hv_device > *device, > { > int ret; > > - if (net_device->send_buffer.leak) > - return; > - > if (net_device->send_buffer.gpadl.gpadl_handle) { > - ret = vmbus_teardown_gpadl(device->channel, > - &net_device->send_buffer.gpadl); > + ret = vmbus_teardown_gpadl_owned(device->channel, > + &net_device->send_buffer); [Severity: High] Similarly for the send buffer, does removing the leak check and flag tracking in netvsc_revoke_send_buf() expose the host to the same bugcheck risk if the revoke message fails to send? > > /* If we failed here, we might as well return and have a leak > * rather than continue and a bugchk > */ > if (ret != 0) { > - net_device->send_buffer.leak = true; > netdev_err(ndev, > "unable to teardown send buffer's gpadl\n"); > return; -- Sashiko AI review ยท https://sashiko.dev/#/patchset/[email protected]?part=9
