wdt_start() registers a cyclic_info that lives inside the watchdog
device's uclass private data, and nothing ever takes it off
gd->cyclic_list again. device_remove() frees that private data
(per_device_auto), so from the moment a watchdog device is removed the
cyclic list holds a node in freed memory. The next schedule() walks it
and calls through cyclic->func, which by then contains whatever the
allocator has since put there.

efi_exit_boot_services() makes that reliable rather than rare: it calls
dm_remove_devices_active(), and U-Boot keeps running for a good while
afterwards, so on a board with a watchdog every EFI payload hand-off
leaves a dangling cyclic behind. Whether it is fatal depends only on
whether something reuses the chunk, which is what lets it hide on a
board that is otherwise perfectly healthy.

On a SpacemiT K3 it stayed invisible until an unrelated controller was
switched on in the firmware's setup. That changed the heap enough for
the ExitBootServices teardown to reuse the freed wdt_priv and zero
cyclic->func, and the next schedule() jumped to address 0:

  Unhandled exception: Instruction access fault
  EPC: 0000000000000000 RA: 00000004fdf4c242 TVAL: 0000000000000000
  EPC: fffffffc040d3000 RA: 000000010201f242 reloc adjusted

with the return address inside cyclic_run(). Switching that unrelated
controller back off made the boot work again, which is most of why it
looked like anything but a use-after-free.

Add a wdt uclass .pre_remove that unregisters the cyclic before the
memory goes, mirroring what the mmc uclass already does through
mmc_remove() -> mmc_deinit(). The hardware watchdog is deliberately left
running: whether an OS should inherit an armed watchdog is a separate
policy question, not part of fixing a dangling pointer.

Signed-off-by: Yuri Zaporozhets <[email protected]>
---
 drivers/watchdog/wdt-uclass.c | 20 ++++++++++++++++++++
 1 file changed, 20 insertions(+)

diff --git a/drivers/watchdog/wdt-uclass.c b/drivers/watchdog/wdt-uclass.c
index 438833b2245..ae755cb6a88 100644
--- a/drivers/watchdog/wdt-uclass.c
+++ b/drivers/watchdog/wdt-uclass.c
@@ -262,10 +262,30 @@ static int wdt_pre_probe(struct udevice *dev)
        return 0;
 }
 
+/*
+ * The cyclic_info that wdt_start() registered lives inside this device's 
uclass
+ * private data, which device_free() releases as soon as the device is removed.
+ * Take it off the cyclic list first, or the next schedule() walks a freed node
+ * and calls through whatever has since been allocated over it.
+ */
+static int wdt_pre_remove(struct udevice *dev)
+{
+       struct wdt_priv *priv = dev_get_uclass_priv(dev);
+
+       if (!IS_ENABLED(CONFIG_WATCHDOG) || !priv || !priv->running)
+               return 0;
+
+       cyclic_unregister(&priv->cyclic);
+       priv->running = false;
+
+       return 0;
+}
+
 UCLASS_DRIVER(wdt) = {
        .id                     = UCLASS_WDT,
        .name                   = "watchdog",
        .flags                  = DM_UC_FLAG_SEQ_ALIAS,
        .pre_probe              = wdt_pre_probe,
+       .pre_remove             = wdt_pre_remove,
        .per_device_auto        = sizeof(struct wdt_priv),
 };
-- 
2.47.3

Reply via email to