Re: [RFC] virtio_balloon: fix Use-After-Free in page reporting during PM freeze

["David Hildenbrand (Arm)" <david@kernel.org>]

On 7/10/26 00:43, Link Lin wrote:
> During system power management freeze (e.g. ACPI S3 suspend or S4
> hibernation), virtballoon_freeze() calls remove_common() to reset the
> virtio device and delete all virtqueues via vdev->config->del_vqs().
> However, unlike virtballoon_remove(), virtballoon_freeze() fails to call
> page_reporting_unregister(&vb->pr_dev_info).
> 
> The comment in virtballoon_freeze() states:
>     /*
>      * The workqueue is already frozen by the PM core before this
>      * function is called.
>      */
> 
> While this comment was accurate in 2011 for balloon-internal workqueues
> (such as balloon_wq, which was created with WQ_FREEZABLE and is paused
> by the PM freezer), it is invalid for Free Page Reporting.
> 
> Free Page Reporting (mm/page_reporting.c) schedules its delayed work
> (prdev->work) on the global system_wq. Because system_wq lacks the
> WQ_FREEZABLE flag, the PM freezer (freeze_workqueues_busy()) explicitly
> skips it. Consequently, page_reporting_process() on system_wq remains
> active and unfrozen throughout device suspend.
> 
> If memory is freed into the buddy allocator or a delayed work timer
> expires while the device is being frozen, page_reporting_process() fires
> on system_wq and calls virtballoon_free_page_report(). This function
> passes vb->reporting_vq into virtqueue_add_inbuf() / virtqueue_add_split().
> Because the virtqueues were already destroyed by del_vqs(), this results
> in a Use-After-Free / General Protection Fault:
> 
>     [  250.709271] general protection fault, probably for non-canonical address 0x7f728084daf08d5e: 0000 [#1] SMP PTI
>     [  250.732967] CPU: 2 PID: 38 Comm: kworker/2:1 Not tainted 5.10.0-44-cloud-amd64 #1 Debian 5.10.257-1
>     [  250.751575] Workqueue: events page_reporting_process
>     [  250.756665] RIP: 0010:virtqueue_add_split+0x233/0x4c0 [virtio_ring]
>     ...
>     [  250.867678] virtballoon_free_page_report+0x3a/0xe0 [virtio_balloon]
>     [  250.883446] page_reporting_process+0x225/0x4f0
> 
> (Note: The OOM Notifier and Shrinker/Free Page Hinting features suffer
> from an identical lifecycle flaw and are also vulnerable to UAFs during
> S4 hibernation when memory pressure spikes. This patch focuses on Free
> Page Reporting, which runs periodically, to ensure clean backports to
> stable kernels).
> 
> Fix this by:
> 1. Unregistering page reporting in virtballoon_freeze() prior to calling
>    remove_common(). This clears the RCU pr_dev_info pointer and flushes/
>    cancels prdev->work on system_wq via cancel_delayed_work_sync().
> 2. Re-registering page reporting in virtballoon_restore() after the
>    virtqueues are re-initialized and virtio_device_ready() has been called.
> 3. Unwinding virtqueue initialization via remove_common() in 
>    virtballoon_restore() if page_reporting_register() fails.
> 
> Fixes: 924a663f75e2 ("virtio-balloon: Reporting free page reservations")
> Cc: stable@vger.kernel.org
> Cc: jasowang@redhat.com
> Cc: xuanzhuo@linux.alibaba.com
> Cc: Ammar Faizi <ammarfaizi2@openresty.com>
> Cc: jiaqiyan@google.com
> Cc: ahwilkins@google.com
> Cc: Greg Thelen <gthelen@google.com>
> Cc: Alexander Duyck <alexander.duyck@gmail.com>
> Signed-off-by: Link Lin <linkl@google.com>
> ---
>  drivers/virtio/virtio_balloon.c | 11 +++++++++++
>  1 file changed, 11 insertions(+)
> 
> diff --git a/drivers/virtio/virtio_balloon.c b/drivers/virtio/virtio_balloon.c
> index a1b2c3d4e5f6..45a90fb3abf8 100640
> --- a/drivers/virtio/virtio_balloon.c
> +++ b/drivers/virtio/virtio_balloon.c
> @@ -1055,6 +1055,9 @@ static int virtballoon_freeze(struct virtio_device *vdev)
>  	 * The workqueue is already frozen by the PM core before this
>  	 * function is called.
>  	 */
> +	if (virtio_has_feature(vb->vdev, VIRTIO_BALLOON_F_REPORTING))
> +		page_reporting_unregister(&vb->pr_dev_info);
> +
>  	remove_common(vb);
>  	return 0;
>  }
>  
>  static int virtballoon_restore(struct virtio_device *vdev)
>  {
>  	struct virtio_balloon *vb = vdev->priv;
>  	int ret;
>  
>  	ret = init_vqs(vdev->priv);
>  	if (ret)
>  		return ret;
>  
>  	virtio_device_ready(vdev);
>  
> +	if (virtio_has_feature(vb->vdev, VIRTIO_BALLOON_F_REPORTING)) {
> +		ret = page_reporting_register(&vb->pr_dev_info);
> +		if (ret)
> +			goto out_remove_vqs;
> +	}

Hm, that failure handling is rather nasty.


In virtballoon_freeze() we document:

"The workqueue is already frozen by the PM core before this function is called"

Your report states:

"Workqueue: events page_reporting_process"


I assume that workqueue is not frozen yet because ... it's not freezable :)

So could we queue to system_freezable_wq instead, or define our own freezable
workqueue there? Then a driver doesn't have to worry about that.

-- 
Cheers,

David