On Mon, Jul 13, 2026 at 12:41:07PM -0700, David Rientjes wrote:
> On Thu, 9 Jul 2026, Link Lin wrote:
> 
> > > > 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.
> > >
> > > AI review thinks the patch didn't do the above:
> > >         
> > > https://sashiko.dev/#/patchset/[email protected]
> > 
> > The AI reviewer might not have parsed the entirety of the fix I proposed.
> > The patch submitted definitely includes the changes to virtballoon_restore()
> > for steps 2 and 3 (re-registering page reporting and unwinding init_vqs
> > on failure). It seems the AI failed to parse the diff correctly. See:
> > 
> > +       if (virtio_has_feature(vb->vdev, VIRTIO_BALLOON_F_REPORTING)) {
> > +               ret = page_reporting_register(&vb->pr_dev_info);
> > +               if (ret)
> > +                       goto out_remove_vqs;
> > +       }
> > +
> >         if (towards_target(vb))
> >                 virtballoon_changed(vdev);
> >         update_balloon_size(vb);
> >         return 0;
> > +
> > +out_remove_vqs:
> > +       remove_common(vb);
> > +       return ret;
> >  }
> > 
> > > It also might have found a couple of pre-existing bugs in there.
> > 
> > Indeed. Regarding the first pre-existing bug found by the AI (leaving the
> > OOM notifier registered during suspend, leading to a UAF if memory pressure
> > spikes during S4 hibernation):
> > 
> > I actually addressed this in the commit message:
> > 
> >   "(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)."
> > 
> > Regarding the second pre-existing bug the AI flagged (leaving uncancelled
> > works on system_freezable_wq if virtballoon_restore fails on the cold path):
> > the AI is correct that this asynchronous work cancellation failure exists.
> > 
> > Since these are separate, pre-existing lifecycle bugs, would you prefer I
> > roll fixes for the OOM notifier, shrinker/free page hinting, and work
> > cancellations into a v2 of this patch, or submit them as a separate patch
> > series to keep the stable backports clean?
> > 
> 
> I think it would be best to have separate patches for each fix; Andrew, 
> please correct me if you'd prefer one patch to address everything.

It does not matter much but yes separate ones are a bit better if
each can be applied independently.


Reply via email to