[PATCH] virt: vmgenid: set driver_data before registering notification handlers

[Zhichen Wang]

Both probe paths register their notification handler before assigning
driver_data, which the handler dereferences.

In the devicetree path, the notification IRQ can fire as soon as
devm_request_irq() registers the handler: the interrupt may already be
pending at probe time, for example when a VMM injects the
generation-changed notification while restoring a guest from a snapshot
that was taken before the driver had probed (Firecracker does exactly
this on snapshot restore). The IRQ is also requested with IRQF_SHARED,
so another device sharing the line can trigger the handler just as
early. The handler then calls vmgenid_notify(), which dereferences the
still-NULL driver_data and panics:

  Unable to handle kernel NULL pointer dereference at virtual address 0000000000000010
  CPU: 0 UID: 0 PID: 1 Comm: swapper/0 Not tainted 6.18.38+ #1 PREEMPT(none)
  Hardware name: linux,dummy-virt (DT)
  pc : vmgenid_notify.isra.0+0x24/0x8c
  lr : vmgenid_of_irq_handler+0x14/0x34
  Call trace:
   vmgenid_notify.isra.0+0x24/0x8c (P)
   vmgenid_of_irq_handler+0x14/0x34
   __handle_irq_event_percpu+0x44/0x1bc
   handle_irq_event+0x4c/0xb4
   handle_fasteoi_irq+0xf8/0x1f8

The ACPI path has the same ordering problem: the handler is installed
with acpi_install_notify_handler() before driver_data is assigned.
ACPI notifications are dispatched asynchronously from a workqueue, so
the window is narrow, but a notification arriving between the two calls
hits the same NULL dereference.

Assign driver_data before registering the handlers. The state is fully
initialized at that point, so the handlers are safe to run. Should
registration fail, the probe error path leaves no dangling pointer
behind: the driver core clears driver_data in device_unbind_cleanup().

Fixes: 7b1bcd6b50a6 ("virt: vmgenid: add support for devicetree bindings")
Fixes: e07606713a90 ("virt: vmgenid: change implementation to use a platform driver")
Cc: stable@xxxxxxxxxxxxxxx
Signed-off-by: Zhichen Wang <wangzhichen@xxxxxxxx>
---
Verified on Firecracker v1.14 (aarch64, devicetree) with a snapshot
taken before the vmgenid driver had probed, so the injected
notification IRQ is pending when the restored guest reaches probe:
the unpatched kernel panics with the trace above, while the patched
kernel handles the pending IRQ as soon as devm_request_irq()
registers the handler (/proc/interrupts shows the vmgenid IRQ count
at 1) and boot completes normally. The ACPI path change is
compile-tested; the window there is analogous but much harder to hit.

 drivers/virt/vmgenid.c | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/drivers/virt/vmgenid.c b/drivers/virt/vmgenid.c
index 66135eac3..92d9779c9 100644
--- a/drivers/virt/vmgenid.c
+++ b/drivers/virt/vmgenid.c
@@ -82,6 +82,8 @@ static int vmgenid_add_acpi(struct device *dev, struct vmgenid_state *state)
 	}
 	setup_vmgenid_state(state, virt_addr);
 
+	dev->driver_data = state;
+
 	status = acpi_install_notify_handler(device->handle, ACPI_DEVICE_NOTIFY,
 					     vmgenid_acpi_handler, dev);
 	if (ACPI_FAILURE(status)) {
@@ -89,7 +91,6 @@ static int vmgenid_add_acpi(struct device *dev, struct vmgenid_state *state)
 		goto out;
 	}
 
-	dev->driver_data = state;
 out:
 	ACPI_FREE(parsed.pointer);
 	return ret;
@@ -123,12 +124,13 @@ static int vmgenid_add_of(struct platform_device *pdev,
 	if (ret < 0)
 		return ret;
 
+	pdev->dev.driver_data = state;
+
 	ret = devm_request_irq(&pdev->dev, ret, vmgenid_of_irq_handler,
 			       IRQF_SHARED, "vmgenid", &pdev->dev);
 	if (ret < 0)
 		return ret;
 
-	pdev->dev.driver_data = state;
 	return 0;
 }
 

base-commit: 66498c75b4f8017f62d720d9b59675bdf3abce91
-- 
2.47.3