virtio-balloon: do not attempt to release more than available pages

PATCH In review Marcelo Tosatti 4 participants 14 messages Mar 5, 2008 – Mar 11, 2008

Subsystems (3)

KVM virtio core virtio balloon

lore

Share & tools

lore

Marcelo Tosatti

author

2008-03-05 16:28 UTC #0 lore raw

PATCH 1/1 virtio-balloon: do not attempt to release more than available pages

Handle the case where the balloon target is larger than total ram size.

BUG: unable to handle kernel paging request at 0000000000100100
IP: [<ffffffff881970f9>] :virtio_balloon:leak__balloon+0x2e/0xbe

Signed-off-by Marcelo Tosatti <[email protected]>

drivers/virtio/virtio_balloon.c +17 -0

Index: virtio/virtio_balloon.c
===================================================================
@@ -122,10 +122,21 @@ static void release_pages_by_pfn(const u
 	}
 }
 
+static void update_target_size(struct virtio_balloon *vb)
+{
+	__le32 num_pages = cpu_to_le32(vb->num_pages);
+
+	vb->vdev->config->set(vb->vdev,
+			      offsetof(struct virtio_balloon_config, num_pages),
+			      &num_pages, sizeof(num_pages));
+}
+
+
 static void leak_balloon(struct virtio_balloon *vb, size_t num)
 {
 	struct page *page;
 
+	num = min_t(unsigned int, vb->num_pages, num);
 	/* We can only do one array worth at a time. */
 	num = min(num, ARRAY_SIZE(vb->pfns));

@@ -136,6 +147,12 @@ static void leak_balloon(struct virtio_b
 		vb->num_pages--;
 	}
 
+	/* Have nothing to release? update target accordingly. */
+	if (vb->num_pfns == 0) {
+		update_target_size(vb);
+		return;
+	}
+
 	if (vb->tell_host_first) {
 		tell_host(vb, vb->deflate_vq);
 		release_pages_by_pfn(vb->pfns, vb->num_pfns);

-------------------------------------------------------------------------
This SF.net email is sponsored by: Microsoft Defy all challenges. Microsoft(R) Visual Studio 2008.
http://clk.atdmt.com/MRT/go/vse0120000070mrt/direct/01/

Avi Kivity

2008-03-05 16:59 UTC #1 lore raw

Quoted 14 lines · Marcelo Tosatti

Handle the case where the balloon target is larger than total ram size.
BUG: unable to handle kernel paging request at 0000000000100100
IP: [<ffffffff881970f9>] :virtio_balloon:leak__balloon+0x2e/0xbe
Signed-off-by Marcelo Tosatti <[email protected]>
Index: virtio/virtio_balloon.c
===================================================================
drivers/virtio/virtio_balloon.c +0 -0
@@ -122,10 +122,21 @@ static void release_pages_by_pfn(const u
 	}
 }

drivers/virtio/virtio_balloon.c static void release_pages_by_pfn(const u · line 133

Quoted 10 lines

drivers/virtio/virtio_balloon.c +8 -0

 
+static void update_target_size(struct virtio_balloon *vb)
+{
+	__le32 num_pages = cpu_to_le32(vb->num_pages);
+
+	vb->vdev->config->set(vb->vdev,
+			      offsetof(struct virtio_balloon_config, num_pages),
+			      &num_pages, sizeof(num_pages));
+}

The target is host-owned; moreover the problem may be temporary, but you've changed the target permanently.

Suggest sending the host a message (like the page list) indicating it couldn't allocate any more.

Also, we may have driven the guest close to oom with this. We need to notify the host when the guest gets into a low-memory cannot swap condition.

Signature · 7 lines

Do not meddle in the internals of kernels, for they are subtle and quick to panic.


-------------------------------------------------------------------------
This SF.net email is sponsored by: Microsoft
Defy all challenges. Microsoft(R) Visual Studio 2008.
http://clk.atdmt.com/MRT/go/vse0120000070mrt/direct/01/

Marcelo Tosatti

author

2008-03-05 18:12 UTC #2 lore raw

Quoted 30 lines · Avi Kivity

Quoted 15 lines · Marcelo Tosatti
Handle the case where the balloon target is larger than total ram size.
BUG: unable to handle kernel paging request at 0000000000100100
IP: [<ffffffff881970f9>] :virtio_balloon:leak__balloon+0x2e/0xbe
Signed-off-by Marcelo Tosatti <[email protected]>
Index: virtio/virtio_balloon.c
===================================================================
drivers/virtio/virtio_balloon.c +0 -0
@@ -122,10 +122,21 @@ static void release_pages_by_pfn(const u
 	}
 }
 
drivers/virtio/virtio_balloon.c static void release_pages_by_pfn(const u · line 134
Quoted 10 lines
drivers/virtio/virtio_balloon.c +8 -0
+static void update_target_size(struct virtio_balloon *vb)
+{
+	__le32 num_pages = cpu_to_le32(vb->num_pages);
+
+	vb->vdev->config->set(vb->vdev,
+			      offsetof(struct virtio_balloon_config, 
 num_pages),
+			      &num_pages, sizeof(num_pages));
+}
 
The target is host-owned; moreover the problem may be temporary, but you've changed the target permanently.

Quoted 5 lines

Suggest sending the host a message (like the page list) indicating it couldn't allocate any more.
Also, we may have driven the guest close to oom with this. We need to notify the host when the guest gets into a low-memory cannot swap condition.

I guess the description was not clear, you understood the opposite.

The problem is when the target for total guest pages (not balloon target size) is set to be larger than the amount of total pages the guest has booted with. What happens then is that the driver tries to release pages from the balloon, without checking if there are any:

static void leak_balloon(struct virtio_balloon *vb, size_t num)

{
        struct page *page;

	/* We can only do one array worth at a time. */
	num = min(num, ARRAY_SIZE(vb->pfns));

        for (vb->num_pfns = 0; vb->num_pfns < num; vb->num_pfns++) {
                page = list_first_entry(&vb->pages, struct page, lru);
                list_del(&page->lru);
                vb->pfns[vb->num_pfns] = page_to_pfn(page);
                vb->num_pages--;
        }

vp->pages is empty here.

So the patch checks for the availability of ballooned pages before attempting to release any, and sets num_pages to match that.

The host should not allow that to condition to happen, but its still fragile code in the guest driver.

Avi Kivity

2008-03-05 18:13 UTC #3 lore raw

Quoted 61 lines · Marcelo Tosatti

Quoted 31 lines · Avi Kivity
Quoted 16 lines · Marcelo Tosatti
Handle the case where the balloon target is larger than total ram size.
BUG: unable to handle kernel paging request at 0000000000100100
IP: [<ffffffff881970f9>] :virtio_balloon:leak__balloon+0x2e/0xbe
Signed-off-by Marcelo Tosatti <[email protected]>
Index: virtio/virtio_balloon.c
===================================================================
drivers/virtio/virtio_balloon.c +1 -0
@@ -122,10 +122,21 @@ static void release_pages_by_pfn(const u
 	}
 }
 
+static void update_target_size(struct virtio_balloon *vb)
drivers/virtio/virtio_balloon.c static void release_pages_by_pfn(const u · line 135
Quoted 10 lines
drivers/virtio/virtio_balloon.c +7 -0
+{
+	__le32 num_pages = cpu_to_le32(vb->num_pages);
+
+	vb->vdev->config->set(vb->vdev,
+			      offsetof(struct virtio_balloon_config, 
 num_pages),
+			      &num_pages, sizeof(num_pages));
+}
 
 
The target is host-owned; moreover the problem may be temporary, but you've changed the target permanently.
Quoted 6 lines
Suggest sending the host a message (like the page list) indicating it couldn't allocate any more.
Also, we may have driven the guest close to oom with this. We need to notify the host when the guest gets into a low-memory cannot swap condition.
I guess the description was not clear, you understood the opposite.
The problem is when the target for total guest pages (not balloon target size) is set to be larger than the amount of total pages the guest has booted with. What happens then is that the driver tries to release pages from the balloon, without checking if there are any:
static void leak_balloon(struct virtio_balloon *vb, size_t num)
{
        struct page *page;

	/* We can only do one array worth at a time. */
	num = min(num, ARRAY_SIZE(vb->pfns));

        for (vb->num_pfns = 0; vb->num_pfns < num; vb->num_pfns++) {
                page = list_first_entry(&vb->pages, struct page, lru);
                list_del(&page->lru);
                vb->pfns[vb->num_pfns] = page_to_pfn(page);
                vb->num_pages--;
        }

Quoted 9 lines

vp->pages is empty here.
So the patch checks for the availability of ballooned pages before attempting to release any, and sets num_pages to match that.
The host should not allow that to condition to happen, but its still fragile code in the guest driver.

Ah, I see. We could simply ignore it, or adjust the target as you did. Not sure what is better.

Signature · 7 lines

Do not meddle in the internals of kernels, for they are subtle and quick to panic.


-------------------------------------------------------------------------
This SF.net email is sponsored by: Microsoft
Defy all challenges. Microsoft(R) Visual Studio 2008.
http://clk.atdmt.com/MRT/go/vse0120000070mrt/direct/01/

Anthony Liguori

2008-03-05 18:43 UTC #4 lore raw

In reply to Marcelo Tosatti

Quoted 40 lines · Marcelo Tosatti

Quoted 31 lines · Avi Kivity
Quoted 16 lines · Marcelo Tosatti
Handle the case where the balloon target is larger than total ram size.
BUG: unable to handle kernel paging request at 0000000000100100
IP: [<ffffffff881970f9>] :virtio_balloon:leak__balloon+0x2e/0xbe
Signed-off-by Marcelo Tosatti <[email protected]>
Index: virtio/virtio_balloon.c
===================================================================
drivers/virtio/virtio_balloon.c +1 -0
@@ -122,10 +122,21 @@ static void release_pages_by_pfn(const u
 	}
 }
 
+static void update_target_size(struct virtio_balloon *vb)
drivers/virtio/virtio_balloon.c static void release_pages_by_pfn(const u · line 135
Quoted 10 lines
drivers/virtio/virtio_balloon.c +7 -0
+{
+	__le32 num_pages = cpu_to_le32(vb->num_pages);
+
+	vb->vdev->config->set(vb->vdev,
+			      offsetof(struct virtio_balloon_config, 
 num_pages),
+			      &num_pages, sizeof(num_pages));
+}
 
 
The target is host-owned; moreover the problem may be temporary, but you've changed the target permanently.
Quoted 6 lines
Suggest sending the host a message (like the page list) indicating it couldn't allocate any more.
Also, we may have driven the guest close to oom with this. We need to notify the host when the guest gets into a low-memory cannot swap condition.

Quoted 7 lines

I guess the description was not clear, you understood the opposite.
The problem is when the target for total guest pages (not balloon target size) is set to be larger than the amount of total pages the guest has booted with. What happens then is that the driver tries to release pages from the balloon, without checking if there are any:

target in the config space is target balloon size, not target for total guest pages. So how is it ever possible for this condition to occur?

Regards,

Anthony Liguori

Quoted 23 lines

static void leak_balloon(struct virtio_balloon *vb, size_t num)
{
        struct page *page;

	/* We can only do one array worth at a time. */
	num = min(num, ARRAY_SIZE(vb->pfns));

        for (vb->num_pfns = 0; vb->num_pfns < num; vb->num_pfns++) {
                page = list_first_entry(&vb->pages, struct page, lru);
                list_del(&page->lru);
                vb->pfns[vb->num_pfns] = page_to_pfn(page);
                vb->num_pages--;
        }
vp->pages is empty here.
So the patch checks for the availability of ballooned pages before attempting to release any, and sets num_pages to match that.
The host should not allow that to condition to happen, but its still fragile code in the guest driver.

Quoted 9 lines

-------------------------------------------------------------------------
This SF.net email is sponsored by: Microsoft Defy all challenges. Microsoft(R) Visual Studio 2008.
http://clk.atdmt.com/MRT/go/vse0120000070mrt/direct/01/
_______________________________________________
kvm-devel mailing list [email protected]
https://lists.sourceforge.net/lists/listinfo/kvm-devel

Marcelo Tosatti

author

2008-03-05 19:39 UTC #5 lore raw

Quoted 11 lines

Quoted 8 lines
I guess the description was not clear, you understood the opposite.
The problem is when the target for total guest pages (not balloon target size) is set to be larger than the amount of total pages the guest has booted with. What happens then is that the driver tries to release pages from the balloon, without checking if there are any:
target in the config space is target balloon size, not target for total guest pages. So how is it ever possible for this condition to occur?

Set the target in QEMU to be larger than guest ram size. The config space variable will be set negatively, so guest attempts to release pages from the balloon.

Anthony Liguori

2008-03-05 19:42 UTC #6 lore raw

Quoted 10 lines · Marcelo Tosatti

Quoted 10 lines
Quoted 10 lines
I guess the description was not clear, you understood the opposite.
The problem is when the target for total guest pages (not balloon target size) is set to be larger than the amount of total pages the guest has booted with. What happens then is that the driver tries to release pages
from the balloon, without checking if there are any:

Quoted 9 lines

Quoted 3 lines
target in the config space is target balloon size, not target for total guest pages. So how is it ever possible for this condition to occur?
Set the target in QEMU to be larger than guest ram size. The config space variable will be set negatively, so guest attempts to release pages from the balloon.

Is an __le32 signed? If so, we should just use an unsigned type.

Regards,

Anthony Liguori

Avi Kivity

2008-03-06 07:06 UTC #7 lore raw

Quoted 11 lines · Anthony Liguori

Quoted 7 lines
Set the target in QEMU to be larger than guest ram size. The config space variable will be set negatively, so guest attempts to release pages from the balloon.
Is an __le32 signed? If so, we should just use an unsigned type.

That would balloon the guest into oblivion if the same condition happened.

Signature · 7 lines

Do not meddle in the internals of kernels, for they are subtle and quick to panic.


-------------------------------------------------------------------------
This SF.net email is sponsored by: Microsoft
Defy all challenges. Microsoft(R) Visual Studio 2008.
http://clk.atdmt.com/MRT/go/vse0120000070mrt/direct/01/

Anthony Liguori

2008-03-05 18:42 UTC #8 lore raw

In reply to Avi Kivity

Quoted 31 lines · Avi Kivity

Quoted 15 lines · Marcelo Tosatti
Handle the case where the balloon target is larger than total ram size.
BUG: unable to handle kernel paging request at 0000000000100100
IP: [<ffffffff881970f9>] :virtio_balloon:leak__balloon+0x2e/0xbe
Signed-off-by Marcelo Tosatti <[email protected]>
Index: virtio/virtio_balloon.c
===================================================================
drivers/virtio/virtio_balloon.c +0 -0
@@ -122,10 +122,21 @@ static void release_pages_by_pfn(const u
 	}
 }
 
drivers/virtio/virtio_balloon.c static void release_pages_by_pfn(const u · line 134
Quoted 10 lines
drivers/virtio/virtio_balloon.c +8 -0
+static void update_target_size(struct virtio_balloon *vb)
+{
+	__le32 num_pages = cpu_to_le32(vb->num_pages);
+
+	vb->vdev->config->set(vb->vdev,
+			      offsetof(struct virtio_balloon_config, num_pages),
+			      &num_pages, sizeof(num_pages));
+}
 
 
The target is host-owned; moreover the problem may be temporary, but you've changed the target permanently.

Quoted 3 lines

Suggest sending the host a message (like the page list) indicating it couldn't allocate any more.

This is what the actual field in the config space is meant for.

Quoted 3 lines

Also, we may have driven the guest close to oom with this. We need to notify the host when the guest gets into a low-memory cannot swap condition.

We need an OOM handler and a high seek-cost shrinker to ensure we do not go too low I imagine. We can notify the host of when we refuse to go lower with actual in the config space.

Regards,

Anthony Liguori

Rusty Russell

2008-03-05 22:39 UTC #9 lore raw

In reply to Marcelo Tosatti

Quoted 6 lines · On Thursday 06 March 2008 03:28:32 Marcelo Tosatti wrote:

Handle the case where the balloon target is larger than total ram size.
BUG: unable to handle kernel paging request at 0000000000100100
IP: [<ffffffff881970f9>] :virtio_balloon:leak__balloon+0x2e/0xbe
Signed-off-by Marcelo Tosatti <[email protected]>

Hi Marcelo,

My deepest concern is that we're papering over some real bug, if you
tripped this. Zeroing num_pages makes sense as a hack, but we're clearly in trouble at this point and there should be some indication of that.

If I may ask, how did this happen?

Thanks,
Rusty.

Marcelo Tosatti

author

2008-03-08 19:06 UTC #10 lore raw

Quoted 8 lines · Rusty Russell

Quoted 6 lines · On Thursday 06 March 2008 03:28:32 Marcelo Tosatti wrote:
Handle the case where the balloon target is larger than total ram size.
BUG: unable to handle kernel paging request at 0000000000100100
IP: [<ffffffff881970f9>] :virtio_balloon:leak__balloon+0x2e/0xbe
Signed-off-by Marcelo Tosatti <[email protected]>

Quoted 7 lines

Hi Marcelo,
My deepest concern is that we're papering over some real bug, if you
tripped this. Zeroing num_pages makes sense as a hack, but we're clearly in trouble at this point and there should be some indication of that.
If I may ask, how did this happen?

Rusty,

QEMU allows the user to set a target larger than the memory size which the guest was booted with:

        dev->num_pages = (ram_size - target) >> TARGET_PAGE_BITS;
        virtio_notify_config(&dev->vdev);

num_pages goes negative, so driver attempts deflate back -num_pages from its balloon.

There is no other bug here, its just a matter of guest driver being fragile. You can avoid that condition in the host backend:

virtio-balloon.c 2008-03-08 14:41:08.000000000 -0300 renamed from virtio-balloon.c.orig 2008-03-08 14:39:51.000000000 -0300 +2 -0

@@ -113,6 +113,8 @@
     VirtIOBalloon *dev = opaque;
 
     if (target) {
+       if (target > ram_size)
+               target = ram_size;
        dev->num_pages = (ram_size - target) >> TARGET_PAGE_BITS;
        virtio_notify_config(&dev->vdev);
     }

But making the driver robust against it seems sensate. I agree that zeroing num_pages is hackish. What do you suggest?

Thanks

Rusty Russell

2008-03-11 00:26 UTC #11 lore raw

Quoted 2 lines · On Sunday 09 March 2008 06:06:38 Marcelo Tosatti wrote:

But making the driver robust against it seems sensate. I agree that zeroing num_pages is hackish. What do you suggest?

OK, after more discussion on IRC, this seems like the correct thing to do.

1) Allow more than 2G of pages. It doesn't cost us much to break the 8TB barrier. This is an ABI change, so best to do this now.
2) Handle the case where we get nonsense from the host, which causes us to wrap around.

How's this (compile-tested only!):

drivers/virtio/virtio_balloon.c +18 ~~-11~~

diff -r fd0c80dbbd95 drivers/virtio/virtio_balloon.c
@@ -43,12 +43,12 @@ struct virtio_balloon
 	bool tell_host_first;
 
 	/* The pages we've told the Host we're not using. */
-	unsigned int num_pages;
+	unsigned long num_pages;
 	struct list_head pages;
 
 	/* The array of pfns we tell the Host about. */
 	unsigned int num_pfns;
-	u32 pfns[256];
+	u64 pfns[256];
 };
 
 static struct virtio_device_id id_table[] = {

@@ -83,17 +83,17 @@ static void tell_host(struct virtio_ball
 	wait_for_completion(&vb->acked);
 }
 
-static void fill_balloon(struct virtio_balloon *vb, size_t num)
+static void fill_balloon(struct virtio_balloon *vb, u64 num)
 {
 	/* We can only do one array worth at a time. */
-	num = min(num, ARRAY_SIZE(vb->pfns));
+	num = min_t(u64, num, ARRAY_SIZE(vb->pfns));
 
 	for (vb->num_pfns = 0; vb->num_pfns < num; vb->num_pfns++) {
 		struct page *page = alloc_page(GFP_HIGHUSER | __GFP_NORETRY);
 		if (!page) {
 			if (printk_ratelimit())
 				dev_printk(KERN_INFO, &vb->vdev->dev,
-					   "Out of puff! Can't get %zu pages\n",
+					   "Out of puff! Can't get %llu pages\n",
 					   num);
 			/* Sleep for at least 1/5 of a second before retry. */
 			msleep(200);

@@ -112,7 +112,7 @@ static void fill_balloon(struct virtio_b
 	tell_host(vb, vb->inflate_vq);
 }
 
-static void release_pages_by_pfn(const u32 pfns[], unsigned int num)
+static void release_pages_by_pfn(const u64 pfns[], unsigned int num)
 {
 	unsigned int i;

@@ -122,12 +122,12 @@ static void release_pages_by_pfn(const u
 	}
 }
 
-static void leak_balloon(struct virtio_balloon *vb, size_t num)
+static void leak_balloon(struct virtio_balloon *vb, u64 num)
 {
 	struct page *page;
 
 	/* We can only do one array worth at a time. */
-	num = min(num, ARRAY_SIZE(vb->pfns));
+	num = min_t(u64, num, ARRAY_SIZE(vb->pfns));
 
 	for (vb->num_pfns = 0; vb->num_pfns < num; vb->num_pfns++) {
 		page = list_first_entry(&vb->pages, struct page, lru);

@@ -152,12 +152,19 @@ static void virtballoon_changed(struct v
 	wake_up(&vb->config_change);
 }
 
-static inline int towards_target(struct virtio_balloon *vb)
+static inline s64 towards_target(struct virtio_balloon *vb)
 {
-	u32 v;
+	u64 v;
 	__virtio_config_val(vb->vdev,
 			    offsetof(struct virtio_balloon_config, num_pages),
 			    &v);
+
+	/* If they ask for something which will wrap, we ignore it. */
+	if (v & 0x8000000000000000ULL) {
+		dev_warn(&vb->vdev->dev, "Insane target 0x%llx\n", v);
+		return 0;
+	}
+
 	return v - vb->num_pages;
 }

@@ -176,7 +183,7 @@ static int balloon(void *_vballoon)
 
 	set_freezable();
 	while (!kthread_should_stop()) {
-		int diff;
+		s64 diff;
 
 		try_to_freeze();
 		wait_event_interruptible(vb->config_change,

include/linux/virtio_balloon.h +2 -2

diff -r fd0c80dbbd95 include/linux/virtio_balloon.h
@@ -11,8 +11,8 @@ struct virtio_balloon_config
 struct virtio_balloon_config
 {
 	/* Number of pages host wants Guest to give up. */
-	__le32 num_pages;
+	__le64 num_pages;
 	/* Number of pages we've actually got in balloon. */
-	__le32 actual;
+	__le64 actual;
 };
 #endif /* _LINUX_VIRTIO_BALLOON_H */

Anthony Liguori

2008-03-11 00:52 UTC #12 lore raw

Quoted 11 lines · Rusty Russell

Quoted 3 lines · On Sunday 09 March 2008 06:06:38 Marcelo Tosatti wrote:
But making the driver robust against it seems sensate. I agree that zeroing num_pages is hackish. What do you suggest?
OK, after more discussion on IRC, this seems like the correct thing to do.
1) Allow more than 2G of pages. It doesn't cost us much to break the 8TB barrier. This is an ABI change, so best to do this now.

I don't think there's a compelling need to break the ABI. If we truly ever needed to break the 8TB barrier, we could simply introduce another ballooning device. virtio_pci relies on PFNs fitting into a 32-bit value so breaking the 8TB barrier here doesn't given it to us for free anyway.

Plus, we've released an external drivers package. Breaking the ABI at this stage is going to cause user confusion. Sure, the ABI isn't stable until 2.6.25 ships but I do think we should have a more compelling reason to do it if we truly have to.

Quoted 3 lines

2) Handle the case where we get nonsense from the host, which causes us to wrap around.

Here's my neither compiled nor tested version which should fix this problem without breaking the ABI.

Regards,

Anthony Liguori

drivers/virtio/virtio_balloon.c +2 -2

index c8a4332..f1d71c4 100644
@@ -152,7 +152,7 @@ static void virtballoon_changed(struct virtio_device *vdev)
 	wake_up(&vb->config_change);
 }
 
-static inline int towards_target(struct virtio_balloon *vb)
+static inline int64_t towards_target(struct virtio_balloon *vb)
 {
 	u32 v;
 	__virtio_config_val(vb->vdev,

@@ -176,7 +176,7 @@ static int balloon(void *_vballoon)
 
 	set_freezable();
 	while (!kthread_should_stop()) {
-		int diff;
+		int64_t diff;
 
 		try_to_freeze();
 		wait_event_interruptible(vb->config_change,

_______________________________________________
kvm-devel mailing list
[email protected]
https://lists.sourceforge.net/lists/listinfo/kvm-devel

Rusty Russell

2008-03-11 11:54 UTC #13 lore raw

Quoted 6 lines · On Tuesday 11 March 2008 11:52:46 Anthony Liguori wrote:

Quoted 2 lines · Rusty Russell
2) Handle the case where we get nonsense from the host, which causes us to wrap around.
Here's my neither compiled nor tested version which should fix this problem without breaking the ABI.

Hmm, we could just refuse to grab >= 2G pages. It is not only reasonable, but also gives us an opportunity to make another Sophie Blanchard reference![1]

Patch below.

Cheers,
Rusty.
[1] Wow, I actually learned something from a Wikipedia Featured Article.

virtio: handle > 2 billion page balloon targets

If the host asks for a huge target towards_target() can overflow, and we up oops as we try to release more pages than we have. The simple fix is to use a 64-bit value.

Signed-off-by Rusty Russell <[email protected]>

drivers/virtio/virtio_balloon.c +2 -2

diff -r fd0c80dbbd95 drivers/virtio/virtio_balloon.c
@@ -152,7 +152,7 @@ static void virtballoon_changed(struct v
 	wake_up(&vb->config_change);
 }
 
-static inline int towards_target(struct virtio_balloon *vb)
+static inline s64 towards_target(struct virtio_balloon *vb)
 {
 	u32 v;
 	__virtio_config_val(vb->vdev,

@@ -176,7 +183,7 @@ static int balloon(void *_vballoon)
 
 	set_freezable();
 	while (!kthread_should_stop()) {
-		int diff;
+		s64 diff;
 
 		try_to_freeze();
 		wait_event_interruptible(vb->config_change,