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
> --
next prev 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