Linux-HyperV List
 help / color / mirror / Atom feed
* [PATCH] hv: hv_balloon: validate unballoon range count
@ 2026-07-10  2:29 Michael Bommarito
  2026-07-10  2:42 ` sashiko-bot
  2026-07-11 18:09 ` Michael Kelley
  0 siblings, 2 replies; 3+ messages in thread
From: Michael Bommarito @ 2026-07-10  2:29 UTC (permalink / raw)
  To: kys, Haiyang Zhang, Wei Liu, Dexuan Cui, Long Li
  Cc: Greg Kroah-Hartman, linux-hyperv, linux-kernel, stable

The Hyper-V dynamic memory host supplies DM_UNBALLOON_REQUEST messages
with a header size and a range_count field. balloon_down() trusts
range_count and walks req->range_array without checking that the received
message contains that many ranges.

A malformed host or backend message can therefore make the guest read
past the received VMBus packet while freeing balloon ranges. Validate the
received message size and reject range_count values that exceed the
present range array before walking it.

Impact: A malicious Hyper-V host or backend can crash a guest by sending
a short unballoon request with an oversized range_count.

Fixes: 9aa8b50b2b3d ("Drivers: hv: Add Hyper-V balloon driver")
Cc: stable@vger.kernel.org
Assisted-by: Codex:gpt-5-5-xhigh
Signed-off-by: Michael Bommarito <michael.bommarito@gmail.com>
---
 drivers/hv/hv_balloon.c | 26 ++++++++++++++++++++++++--
 1 file changed, 24 insertions(+), 2 deletions(-)

diff --git a/drivers/hv/hv_balloon.c b/drivers/hv/hv_balloon.c
index a848400a59a2d..f5bc8c9fea7b9 100644
--- a/drivers/hv/hv_balloon.c
+++ b/drivers/hv/hv_balloon.c
@@ -1337,8 +1337,23 @@ static void balloon_up(struct work_struct *dummy)
 	}
 }
 
+static bool unballoon_request_valid(struct dm_unballoon_request *req,
+				    u32 msg_size)
+{
+	u32 max_ranges;
+
+	if (msg_size < sizeof(*req) || req->hdr.size < sizeof(*req) ||
+	    req->hdr.size > msg_size)
+		return false;
+
+	max_ranges = (req->hdr.size - sizeof(*req)) /
+		     sizeof(req->range_array[0]);
+
+	return req->range_count <= max_ranges;
+}
+
 static void balloon_down(struct hv_dynmem_device *dm,
-			 struct dm_unballoon_request *req)
+			 struct dm_unballoon_request *req, u32 msg_size)
 {
 	union dm_mem_page_range *range_array = req->range_array;
 	int range_count = req->range_count;
@@ -1346,6 +1361,12 @@ static void balloon_down(struct hv_dynmem_device *dm,
 	int i;
 	unsigned int prev_pages_ballooned = dm->num_pages_ballooned;
 
+	if (!unballoon_request_valid(req, msg_size)) {
+		pr_warn_ratelimited("Invalid unballoon request: size %u, header size %u, range count %u\n",
+				    msg_size, req->hdr.size, req->range_count);
+		return;
+	}
+
 	for (i = 0; i < range_count; i++) {
 		free_balloon_pages(dm, &range_array[i]);
 		complete(&dm_device.config_event);
@@ -1527,7 +1548,8 @@ static void balloon_onchannelcallback(void *context)
 
 			dm->state = DM_BALLOON_DOWN;
 			balloon_down(dm,
-				     (struct dm_unballoon_request *)recv_buffer);
+				     (struct dm_unballoon_request *)recv_buffer,
+				     recvlen);
 			break;
 
 		case DM_MEM_HOT_ADD_REQUEST:
-- 
2.53.0

^ permalink raw reply related	[flat|nested] 3+ messages in thread

* Re: [PATCH] hv: hv_balloon: validate unballoon range count
  2026-07-10  2:29 [PATCH] hv: hv_balloon: validate unballoon range count Michael Bommarito
@ 2026-07-10  2:42 ` sashiko-bot
  2026-07-11 18:09 ` Michael Kelley
  1 sibling, 0 replies; 3+ messages in thread
From: sashiko-bot @ 2026-07-10  2:42 UTC (permalink / raw)
  To: Michael Bommarito; +Cc: linux-hyperv

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

Pre-existing issues:
- [Critical] free_balloon_pages() blindly trusts untrusted host-provided PFNs, causing arbitrary memory free.
- [High] Teardown race in balloon_remove() leads to Use-After-Free.
- [High] Data race when updating worker arguments from VMBus softirq callbacks.
- [High] Concurrent lockless modification of dm->num_pages_ballooned.
- [High] Pending workqueues are leaked on the balloon_probe() error path.
--

--- Patch [1]: [PATCH] hv: hv_balloon: validate unballoon range count ---
commit cffe2d5855cc693f743d4cc5594c147a6fff1bdb
Author: Michael Bommarito <michael.bommarito@gmail.com>

hv: hv_balloon: validate unballoon range count

The Hyper-V dynamic memory host supplies DM_UNBALLOON_REQUEST messages
with a header size and a range_count field. balloon_down() trusts
range_count and walks req->range_array without checking that the received
message contains that many ranges. A malformed host or backend message can
therefore make the guest read past the received VMBus packet while freeing
balloon ranges.

> diff --git a/drivers/hv/hv_balloon.c b/drivers/hv/hv_balloon.c
> index 42ce27be344d5..0ebfc2524d630 100644
> --- a/drivers/hv/hv_balloon.c
> +++ b/drivers/hv/hv_balloon.c

[ ... ]

> @@ -1346,6 +1361,12 @@ static void balloon_down(struct hv_dynmem_device *dm,
>  	int i;
>  	unsigned int prev_pages_ballooned = dm->num_pages_ballooned;

[Severity: High]
This is a pre-existing issue, but is it safe to modify
dm->num_pages_ballooned without locking or atomic operations?

If a DM_UNBALLOON_REQUEST arrives while a DM_BALLOON_REQUEST is still being
processed, free_balloon_pages() executes synchronously in the VMBus tasklet
context and decrements the counter:

drivers/hv/hv_balloon.c:free_balloon_pages() {
	...
		dm->num_pages_ballooned--;
	...
}

At the same time, alloc_balloon_pages() executes asynchronously in the
workqueue and increments it:

drivers/hv/hv_balloon.c:alloc_balloon_pages() {
	...
		dm->num_pages_ballooned += alloc_unit;
	...
}

Could lost updates corrupt the guest's memory pressure statistics, leading
to incorrect accounting when reported back to the host via
get_pages_committed()?

>  
> +	if (!unballoon_request_valid(req, msg_size)) {
> +		pr_warn_ratelimited("Invalid unballoon request: size %u, header size %u, range count %u\n",
> +				    msg_size, req->hdr.size, req->range_count);
> +		return;
> +	}
> +
>  	for (i = 0; i < range_count; i++) {
>  		free_balloon_pages(dm, &range_array[i]);
>  		complete(&dm_device.config_event);

[Severity: Critical]
This is a pre-existing issue, but does free_balloon_pages() blindly trust
the untrusted host-provided PFNs?

If a malicious host sends a DM_UNBALLOON_REQUEST specifying arbitrary PFNs
(e.g., kernel text or page tables), the guest iterates over these PFNs and
unconditionally frees them without validating if the pages were actually
ballooned out:

drivers/hv/hv_balloon.c:free_balloon_pages() {
	...
	for (i = 0; i < num_pages; i++) {
		pg = pfn_to_page(i + start_frame);
		__ClearPageOffline(pg);
		__free_page(pg);
	...
}

Can this lead to an immediate guest crash or arbitrary memory corruption via
freeing active kernel or user pages to the buddy allocator?

[ ... ]

> @@ -1527,7 +1548,8 @@ static void balloon_onchannelcallback(void *context)
>  
>  			dm->state = DM_BALLOON_DOWN;
>  			balloon_down(dm,
> -				     (struct dm_unballoon_request *)recv_buffer);
> +				     (struct dm_unballoon_request *)recv_buffer,
> +				     recvlen);
>  			break;
>  
>  		case DM_MEM_HOT_ADD_REQUEST:

[Severity: High]
This is a pre-existing issue, but are worker arguments safely updated during
VMBus softirq callbacks?

The VMBus interrupt handler balloon_onchannelcallback() directly overwrites
shared worker arguments without any locking or synchronization:

drivers/hv/hv_balloon.c:balloon_onchannelcallback() {
	...
		case DM_BALLOON_REQUEST:
			...
			dm_device.balloon_wrk.num_pages = bal_msg->num_pages;
			schedule_work(&dm_device.balloon_wrk.wrk);
			break;
	...
		case DM_MEM_HOT_ADD_REQUEST:
			...
				dm->ha_wrk.ha_page_range = *ha_pg_range;
				dm->ha_wrk.ha_region_range.page_range = 0;
	...
}

If an untrusted host floods the guest with back-to-back request messages,
can worker threads operate on torn, mixed, or corrupted parameters, leading
to incorrect guest memory operations?


[Severity: High]
This isn't a bug introduced by this patch, but does balloon_remove() have a
teardown race that violates subsystem rules?

The function calls cancel_work_sync() before vmbus_close():

drivers/hv/hv_balloon.c:balloon_remove() {
	...
	cancel_work_sync(&dm->balloon_wrk.wrk);
	cancel_work_sync(&dm->ha_wrk.wrk);
	...
	if (dm_device.state != DM_INIT_ERROR) {
		disable_page_reporting();
		vmbus_close(dev->channel);
	...
}

Because the VMBus channel remains open and active, an incoming interrupt can
trigger balloon_onchannelcallback() and independently reschedule the work
item after it was already cancelled.

Could this lead to a use-after-free of a work struct and device state,
causing a kernel crash upon module unload or device unbind?


[Severity: High]
This is a pre-existing issue, but does the probe_error path leak pending
workqueues?

In balloon_probe(), balloon_connect_vsp() opens the channel, allowing the
host to immediately send messages that schedule balloon_wrk or ha_wrk. If
the subsequent kthread_run() step fails, the code jumps to probe_error,
closes the channel, and returns an error without cancelling the work items:

drivers/hv/hv_balloon.c:balloon_probe() {
	...
probe_error:
	dm_device.state = DM_INIT_ERROR;
	dm_device.thread  = NULL;
	disable_page_reporting();
	vmbus_close(dev->channel);
connect_error:
	...
}

Can this result in asynchronous execution of a leaked work item on
uninitialized or freed device state, causing a use-after-free?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260710022914.3740453-1-michael.bommarito@gmail.com?part=1

^ permalink raw reply	[flat|nested] 3+ messages in thread

* RE: [PATCH] hv: hv_balloon: validate unballoon range count
  2026-07-10  2:29 [PATCH] hv: hv_balloon: validate unballoon range count Michael Bommarito
  2026-07-10  2:42 ` sashiko-bot
@ 2026-07-11 18:09 ` Michael Kelley
  1 sibling, 0 replies; 3+ messages in thread
From: Michael Kelley @ 2026-07-11 18:09 UTC (permalink / raw)
  To: Michael Bommarito, kys@microsoft.com, Haiyang Zhang, Wei Liu,
	Dexuan Cui, Long Li
  Cc: Greg Kroah-Hartman, linux-hyperv@vger.kernel.org,
	linux-kernel@vger.kernel.org, stable@vger.kernel.org

From: Michael Bommarito <michael.bommarito@gmail.com> Sent: Thursday, July 9, 2026 7:29 PM
> 
> The Hyper-V dynamic memory host supplies DM_UNBALLOON_REQUEST messages
> with a header size and a range_count field. balloon_down() trusts
> range_count and walks req->range_array without checking that the received
> message contains that many ranges.
> 
> A malformed host or backend message can therefore make the guest read
> past the received VMBus packet while freeing balloon ranges. Validate the
> received message size and reject range_count values that exceed the
> present range array before walking it.

Same comment applies here as I wrote for your proposed validations for
the Hyper-V mouse driver. The balloon driver also has .allowed_in_isolated
set to false, so it isn't loaded in a CoCo VM and it hasn't been hardened
for the "untrusted host" threat model.

Michael

> 
> Impact: A malicious Hyper-V host or backend can crash a guest by sending
> a short unballoon request with an oversized range_count.
> 
> Fixes: 9aa8b50b2b3d ("Drivers: hv: Add Hyper-V balloon driver")
> Cc: stable@vger.kernel.org
> Assisted-by: Codex:gpt-5-5-xhigh
> Signed-off-by: Michael Bommarito <michael.bommarito@gmail.com>
> ---
>  drivers/hv/hv_balloon.c | 26 ++++++++++++++++++++++++--
>  1 file changed, 24 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/hv/hv_balloon.c b/drivers/hv/hv_balloon.c
> index a848400a59a2d..f5bc8c9fea7b9 100644
> --- a/drivers/hv/hv_balloon.c
> +++ b/drivers/hv/hv_balloon.c
> @@ -1337,8 +1337,23 @@ static void balloon_up(struct work_struct *dummy)
>  	}
>  }
> 
> +static bool unballoon_request_valid(struct dm_unballoon_request *req,
> +				    u32 msg_size)
> +{
> +	u32 max_ranges;
> +
> +	if (msg_size < sizeof(*req) || req->hdr.size < sizeof(*req) ||
> +	    req->hdr.size > msg_size)
> +		return false;
> +
> +	max_ranges = (req->hdr.size - sizeof(*req)) /
> +		     sizeof(req->range_array[0]);
> +
> +	return req->range_count <= max_ranges;
> +}
> +
>  static void balloon_down(struct hv_dynmem_device *dm,
> -			 struct dm_unballoon_request *req)
> +			 struct dm_unballoon_request *req, u32 msg_size)
>  {
>  	union dm_mem_page_range *range_array = req->range_array;
>  	int range_count = req->range_count;
> @@ -1346,6 +1361,12 @@ static void balloon_down(struct hv_dynmem_device *dm,
>  	int i;
>  	unsigned int prev_pages_ballooned = dm->num_pages_ballooned;
> 
> +	if (!unballoon_request_valid(req, msg_size)) {
> +		pr_warn_ratelimited("Invalid unballoon request: size %u, header size
> %u, range count %u\n",
> +				    msg_size, req->hdr.size, req->range_count);
> +		return;
> +	}
> +
>  	for (i = 0; i < range_count; i++) {
>  		free_balloon_pages(dm, &range_array[i]);
>  		complete(&dm_device.config_event);
> @@ -1527,7 +1548,8 @@ static void balloon_onchannelcallback(void *context)
> 
>  			dm->state = DM_BALLOON_DOWN;
>  			balloon_down(dm,
> -				     (struct dm_unballoon_request *)recv_buffer);
> +				     (struct dm_unballoon_request *)recv_buffer,
> +				     recvlen);
>  			break;
> 
>  		case DM_MEM_HOT_ADD_REQUEST:
> --
> 2.53.0


^ permalink raw reply	[flat|nested] 3+ messages in thread

end of thread, other threads:[~2026-07-11 18:09 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-07-10  2:29 [PATCH] hv: hv_balloon: validate unballoon range count Michael Bommarito
2026-07-10  2:42 ` sashiko-bot
2026-07-11 18:09 ` Michael Kelley

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox