Linux virtualization list
 help / color / mirror / Atom feed
From: "Michael S. Tsirkin" <mst@redhat.com>
To: Link Lin <linkl@google.com>
Cc: Jason Wang <jasowang@redhat.com>,
	Xuan Zhuo <xuanzhuo@linux.alibaba.com>,
	Andrew Morton <akpm@linux-foundation.org>,
	David Hildenbrand <david@kernel.org>,
	Vlastimil Babka <vbabka@kernel.org>,
	virtualization@lists.linux.dev, linux-mm@kvack.org,
	linux-kernel@vger.kernel.org, prasin@google.com,
	rientjes@google.com, duenwen@google.com, jiaqiyan@google.com,
	ahwilkins@google.com, Greg Thelen <gthelen@google.com>,
	Alexander Duyck <alexander.duyck@gmail.com>,
	jthoughton@google.com, stable@vger.kernel.org,
	Cory Maccarrone <maccarro@google.com>,
	Taylor Scanlon <tayy@google.com>
Subject: Re: [RFC PATCH] virtio_balloon: add VIRTIO_BALLOON_F_REPORTING_PM_SAFE feature bit
Date: Thu, 6 Aug 2026 19:00:01 -0400	[thread overview]
Message-ID: <20260806185414-mutt-send-email-mst@kernel.org> (raw)
In-Reply-To: <20260724014605.3377283-1-linkl@google.com>

On Fri, Jul 24, 2026 at 01:46:05AM +0000, Link Lin wrote:
> Following up on the fix for the PM suspend Use-After-Free race condition in
> mm/page_reporting (merged in mm-hotfixes-unstable: 
> https://lore.kernel.org/all/20260723003650.CAAF01F000E9@smtp.kernel.org/), 
> we face a hypervisor-side deployment dilemma.
> 
> Cloud hypervisors want to safely enable the Free Page Reporting (FPR) 
> virtqueue across their fleets, but enabling it indiscriminately on guests 
> without the recent suspend fix exposes them to UAF crashes. Relying on 
> out-of-band metadata (e.g., OS image tags) to selectively enable the 
> feature is fragile for custom user images or live-patched kernels.
> 
> To address this at the protocol level, we propose adding a new feature bit 
> to the Virtio Specification: 
> VIRTIO_BALLOON_F_REPORTING_PM_SAFE (Bit 6)
> 
> This establishes a formal device lifecycle contract for power management:
> If negotiated, the driver MUST guarantee that all page reporting operations
> are halted and pending requests are flushed before the device/system
> transitions into a suspended state (e.g., ACPI S3/S4).


> Deployment semantics:
> - Hypervisors operating in a strict "safe mode" can offer Bit 6 exclusively 
>   (suppressing Bit 5 / VIRTIO_BALLOON_F_REPORTING).
> - Older, unpatched Linux guests will see Bit 5 is absent, ignore Bit 6, and 
>   safely skip FPR initialization, preventing the suspend crash. Standard
>   ballooning remains 100% functional.
> - Patched Linux guests will recognize Bit 6 and safely initialize FPR.
> - Note for fleet deployments: Non-Linux guests (e.g., Windows, FreeBSD) 
>   that rely on Bit 5 will temporarily lose FPR if the hypervisor exclusively 
>   offers Bit 6. This is considered an acceptable trade-off to globally 
>   protect unpatched guests without relying on OS image tags, until those 
>   respective virtio drivers adopt Bit 6.

This makes no sense to me. So there's a bug in the guest and it crashes.
Patch the guest.

There's just no chance we'll add flags every time some guest drivers
on some OSes have a UAF.

What makes this specific bug special?

Trust me when I say UAF issues e.g. around hotplug are a dime a dozen.
Now what, let's add another one around hotplug? And so on.

> 
> Implementation Note on Upstream/Downstream Dependencies:
> --------------------------------------------------------
> Because it is critical that downstream Linux distros do not accidentally 
> backport Bit 6 without the core MM UAF fix, the final upstream 
> implementation of this patch will enforce a strict compile-time dependency. 
> We plan to export a macro (e.g., PAGE_REPORTING_HAS_FREEZABLE_WQ) from the 
> core MM fix, and wrap Bit 6 behind an #ifdef of that macro in 
> virtio_balloon.c. This guarantees that compiler backports must consume the 
> entire dependency chain to advertise the feature.
> 
> Below is the proposed Linux proof-of-concept based on upstream master. We 
> introduce a helper virtio_balloon_has_reporting() to ensure virtqueues are 
> properly allocated, torn down, and validated if either bit is negotiated.
> 
> If this architectural approach is acceptable for cloud deployments, we will 
> formally submit this patch and open a corresponding issue for the OASIS 
> Virtio specification.
> 
> Depends-on: <Message-ID: 20260723003650.CAAF01F000E9@smtp.kernel.org>
> Signed-off-by: Link Lin <linkl@google.com>
> ---
>  drivers/virtio/virtio_balloon.c     | 15 +++++++++++----
>  include/uapi/linux/virtio_balloon.h |  1 +
>  2 files changed, 12 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/virtio/virtio_balloon.c b/drivers/virtio/virtio_balloon.c
> index 581ac799d9..00e8273dc3 100644
> --- a/drivers/virtio/virtio_balloon.c
> +++ b/drivers/virtio/virtio_balloon.c
> @@ -39,6 +39,12 @@
>  	(1 << (VIRTIO_BALLOON_HINT_BLOCK_ORDER + PAGE_SHIFT))
>  #define VIRTIO_BALLOON_HINT_BLOCK_PAGES (1 << VIRTIO_BALLOON_HINT_BLOCK_ORDER)
>  
> +static inline bool virtio_balloon_has_reporting(struct virtio_device *vdev)
> +{
> +	return virtio_has_feature(vdev, VIRTIO_BALLOON_F_REPORTING) ||
> +	       virtio_has_feature(vdev, VIRTIO_BALLOON_F_REPORTING_PM_SAFE);
> +}
> +
>  enum virtio_balloon_vq {
>  	VIRTIO_BALLOON_VQ_INFLATE,
>  	VIRTIO_BALLOON_VQ_DEFLATE,
> @@ -598,7 +604,7 @@ static int init_vqs(struct virtio_balloon *vb)
>  	if (virtio_has_feature(vb->vdev, VIRTIO_BALLOON_F_FREE_PAGE_HINT))
>  		vqs_info[VIRTIO_BALLOON_VQ_FREE_PAGE].name = "free_page_vq";
>  
> -	if (virtio_has_feature(vb->vdev, VIRTIO_BALLOON_F_REPORTING)) {
> +	if (virtio_balloon_has_reporting(vb->vdev)) {
>  		vqs_info[VIRTIO_BALLOON_VQ_REPORTING].name = "reporting_vq";
>  		vqs_info[VIRTIO_BALLOON_VQ_REPORTING].callback = balloon_ack;
>  	}
> @@ -635,7 +641,7 @@ static int init_vqs(struct virtio_balloon *vb)
>  	if (virtio_has_feature(vb->vdev, VIRTIO_BALLOON_F_FREE_PAGE_HINT))
>  		vb->free_page_vq = vqs[VIRTIO_BALLOON_VQ_FREE_PAGE];
>  
> -	if (virtio_has_feature(vb->vdev, VIRTIO_BALLOON_F_REPORTING))
> +	if (virtio_balloon_has_reporting(vb->vdev))
>  		vb->reporting_vq = vqs[VIRTIO_BALLOON_VQ_REPORTING];
>  
>  	return 0;
> @@ -1013,7 +1019,7 @@ static int virtballoon_probe(struct virtio_device *vdev)
>  	}
>  
>  	vb->pr_dev_info.report = virtballoon_free_page_report;
> -	if (virtio_has_feature(vb->vdev, VIRTIO_BALLOON_F_REPORTING)) {
> +	if (virtio_balloon_has_reporting(vb->vdev)) {
>  		unsigned int capacity;
>  
>  		capacity = virtqueue_get_vring_size(vb->reporting_vq);
> @@ -1099,7 +1105,7 @@ static void virtballoon_remove(struct virtio_device *vdev)
>  {
>  	struct virtio_balloon *vb = vdev->priv;
>  
> -	if (virtio_has_feature(vb->vdev, VIRTIO_BALLOON_F_REPORTING))
> +	if (virtio_balloon_has_reporting(vb->vdev))
>  		page_reporting_unregister(&vb->pr_dev_info);
>  	if (virtio_has_feature(vb->vdev, VIRTIO_BALLOON_F_DEFLATE_ON_OOM))
>  		unregister_oom_notifier(&vb->oom_nb);
> @@ -1162,8 +1168,10 @@ static int virtballoon_validate(struct virtio_device *vdev)
>  	 */
>  	if (!want_init_on_free() && !page_poisoning_enabled_static())
>  		__virtio_clear_bit(vdev, VIRTIO_BALLOON_F_PAGE_POISON);
> -	else if (!virtio_has_feature(vdev, VIRTIO_BALLOON_F_PAGE_POISON))
> +	else if (!virtio_has_feature(vdev, VIRTIO_BALLOON_F_PAGE_POISON)) {
>  		__virtio_clear_bit(vdev, VIRTIO_BALLOON_F_REPORTING);
> +		__virtio_clear_bit(vdev, VIRTIO_BALLOON_F_REPORTING_PM_SAFE);
> +	}
>  
>  	__virtio_clear_bit(vdev, VIRTIO_F_ACCESS_PLATFORM);
>  	return 0;
> @@ -1176,6 +1184,7 @@ static unsigned int features[] = {
>  	VIRTIO_BALLOON_F_FREE_PAGE_HINT,
>  	VIRTIO_BALLOON_F_PAGE_POISON,
>  	VIRTIO_BALLOON_F_REPORTING,
> +	VIRTIO_BALLOON_F_REPORTING_PM_SAFE,
>  };
>  
>  static struct virtio_driver virtio_balloon_driver = {
> diff --git a/include/uapi/linux/virtio_balloon.h b/include/uapi/linux/virtio_balloon.h
> index ee35a37280..d206f156d6 100644
> --- a/include/uapi/linux/virtio_balloon.h
> +++ b/include/uapi/linux/virtio_balloon.h
> @@ -37,6 +37,7 @@
>  #define VIRTIO_BALLOON_F_FREE_PAGE_HINT	3 /* VQ to report free pages */
>  #define VIRTIO_BALLOON_F_PAGE_POISON	4 /* Guest is using page poisoning */
>  #define VIRTIO_BALLOON_F_REPORTING	5 /* Page reporting virtqueue */
> +#define VIRTIO_BALLOON_F_REPORTING_PM_SAFE	6 /* PM-safe page reporting */
>  
>  /* Size of a PFN in the balloon interface. */
>  #define VIRTIO_BALLOON_PFN_SHIFT 12
> --


  parent reply	other threads:[~2026-08-06 23:00 UTC|newest]

Thread overview: 5+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-07-24  1:46 [RFC PATCH] virtio_balloon: add VIRTIO_BALLOON_F_REPORTING_PM_SAFE feature bit Link Lin
2026-07-30  2:52 ` Link Lin
2026-08-06 22:45   ` Link Lin
2026-08-06 23:00 ` Michael S. Tsirkin [this message]
2026-08-07  0:37   ` Link Lin

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20260806185414-mutt-send-email-mst@kernel.org \
    --to=mst@redhat.com \
    --cc=ahwilkins@google.com \
    --cc=akpm@linux-foundation.org \
    --cc=alexander.duyck@gmail.com \
    --cc=david@kernel.org \
    --cc=duenwen@google.com \
    --cc=gthelen@google.com \
    --cc=jasowang@redhat.com \
    --cc=jiaqiyan@google.com \
    --cc=jthoughton@google.com \
    --cc=linkl@google.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-mm@kvack.org \
    --cc=maccarro@google.com \
    --cc=prasin@google.com \
    --cc=rientjes@google.com \
    --cc=stable@vger.kernel.org \
    --cc=tayy@google.com \
    --cc=vbabka@kernel.org \
    --cc=virtualization@lists.linux.dev \
    --cc=xuanzhuo@linux.alibaba.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox