[<prev] [next>] [<thread-prev] [day] [month] [year] [list]
Message-ID: <CAD=FV=WHWrKS_LVjod6nhnPdEk9_ZqeubGpft3PJOUJNMbBxfg@mail.gmail.com>
Date: Fri, 23 Jan 2026 16:01:55 -0800
From: Doug Anderson <dianders@...omium.org>
To: Qiliang Yuan <realwujing@...il.com>
Cc: akpm@...ux-foundation.org, lihuafei1@...wei.com,
linux-kernel@...r.kernel.org, mingo@...nel.org, song@...nel.org,
stable@...r.kernel.org, sunshx@...natelecom.cn, thorsten.blum@...ux.dev,
wangjinchao600@...il.com, yangyicong@...ilicon.com, yuanql9@...natelecom.cn,
zhangjn11@...natelecom.cn, mm-commits@...r.kernel.org
Subject: Re: [PATCH v3] watchdog/hardlockup: Fix UAF in perf event cleanup due
to migration race
Hi,
On Thu, Jan 22, 2026 at 10:34 PM Qiliang Yuan <realwujing@...il.com> wrote:
>
> During the early initialization of the hardlockup detector, the
> hardlockup_detector_perf_init() function probes for PMU hardware availability.
> It originally used hardlockup_detector_event_create(), which interacts with
> the per-cpu 'watchdog_ev' variable.
>
> If the initializing task migrates to another CPU during this probe phase,
> two issues arise:
> 1. The 'watchdog_ev' pointer on the original CPU is set but not cleared,
> leaving a stale pointer to a freed perf event.
> 2. The 'watchdog_ev' pointer on the new CPU might be incorrectly cleared.
>
> This race condition was observed in console logs (captured by adding debug printks):
>
> [23.038376] hardlockup_detector_perf_init 313 cur_cpu=2
Wait a second... The above function hasn't existed for 2.5 years. It
was removed in commit d9b3629ade8e ("watchdog/hardlockup: have the
perf hardlockup use __weak functions more cleanly"). All that's left
in the ToT kernel referencing that function is an old comment...
Oh, and I guess I can see below that your stack traces are on 4.19,
which is ancient! Things have changed a bit in the meantime. Are you
certain that the problem still reproduces on ToT?
> Signed-off-by: Shouxin Sun <sunshx@...natelecom.cn>
> Signed-off-by: Junnan Zhang <zhangjn11@...natelecom.cn>
> Signed-off-by: Qiliang Yuan <realwujing@...il.com>
> Signed-off-by: Qiliang Yuan <yuanql9@...natelecom.cn>
> Cc: Song Liu <song@...nel.org>
> Cc: Douglas Anderson <dianders@...omium.org>
> Cc: Jinchao Wang <wangjinchao600@...il.com>
> Cc: Wang Jinchao <wangjinchao600@...il.com>
> Cc: <stable@...r.kernel.org>
Probably want a "Fixes" tag? If I had to guess, maybe?
Fixes: 930d8f8dbab9 ("watchdog/perf: adapt the watchdog_perf interface
for async model")
Why? I think before that the init function could only be called
directly from the kernel init code and before smp_init(). After that,
a worker could call it, which is the case where preemption could have
been enabled. Does my logic sound correct?
Can you confirm that you're only seeing the problem when the retry
hits? In other words when called from lockup_detector_delay_init()?
Oh, though if you're on 4.19 then I'm not sure what to think...
> @@ -118,18 +118,11 @@ static void watchdog_overflow_callback(struct perf_event *event,
> watchdog_hardlockup_check(smp_processor_id(), regs);
> }
>
> -static int hardlockup_detector_event_create(void)
> +static struct perf_event *hardlockup_detector_event_create(unsigned int cpu)
> {
> - unsigned int cpu;
> struct perf_event_attr *wd_attr;
> struct perf_event *evt;
>
> - /*
> - * Preemption is not disabled because memory will be allocated.
> - * Ensure CPU-locality by calling this in per-CPU kthread.
> - */
> - WARN_ON(!is_percpu_thread());
I'm still a bit confused why this warning didn't trigger previously.
Do you know why?
> @@ -263,19 +258,31 @@ bool __weak __init arch_perf_nmi_is_available(void)
> */
> int __init watchdog_hardlockup_probe(void)
> {
> + struct perf_event *evt;
> + unsigned int cpu;
> int ret;
>
> if (!arch_perf_nmi_is_available())
> return -ENODEV;
>
> - ret = hardlockup_detector_event_create();
> + if (!hw_nmi_get_sample_period(watchdog_thresh))
> + return -EINVAL;
>
> - if (ret) {
> + /*
> + * Test hardware PMU availability by creating a temporary perf event.
> + * Allow migration during the check as any successfully created per-cpu
> + * event validates PMU support. The event is released immediately.
I guess it's implied by the "Allow migration during the check", but I
might even word it more strongly and say something like "The cpu we
use here is arbitrary, so we don't disable preemption and use
raw_smp_processor_id() to get a CPU."
I guess that should be OK. Hopefully the arbitrary CPU that you pick
doesn't go offline during this function. I don't know "perf" well, but
I could imagine that it might be upset if you tried to create a perf
event for a CPU that has gone offline. I guess you could be paranoid
and surround this with cpu_hotplug_disable() / cpu_hotplug_enable()?
I guess overall thoughts: the problem you're describing does seem
real, but the fact that your reports are from an ancient 4.19 kernel
make me concerned about whether you really tested all the cases on a
new kernel...
-Doug
Powered by blists - more mailing lists