Re: [PATCH 2/4] tracing/osnoise: timerlat_main: Disable migration before per-cpu access

Thank you for your contribution! Sashiko AI review found 5 potential issue(s) to consider:

New issues:
- [Critical] Early exit in `timerlat_main` leaves a dangling task_struct pointer, causing a Use-After-Free when the tracer is stopped.
- [High] `osn->cpu` is only initialized for online CPUs, permanently breaking hotplug functionality and triggering the aforementioned Use-After-Free.

Pre-existing issues:
- [High] Uninitialized mutex `osn->lock` used on CPUs hotplugged after boot.
- [High] `osnoise_main` ignores its `data` argument and lacks the CPU migration validation, leading to per-CPU data corruption if migrated.
- [High] ABBA deadlock between `stop_kthread()` and `osnoise_migration_pending()`.
--

--- Patch [2]: [PATCH 2/4] tracing/osnoise: timerlat_main: Disable migration before per-cpu access ---

commit 1b15cf4d4c5a1c36335ab647f7c93d428b13f4fc
Author: Crystal Wood [off-list ref]

tracing/osnoise: timerlat_main: Disable migration before per-cpu access

This commit attempts to fix a preemptible-context splat in timerlat_main
by disabling migration before accessing per-CPU variables. It verifies the
CPU assignment after calling migrate_disable and exits early if a migration
occurred. It also initializes the CPU ID for each online CPU in the tracer
setup.

[ ... ]

[Severity: Critical]
Does this early return leave a dangling task_struct pointer?

If timerlat_main exits here, the thread terminates without clearing
osn_var->kthread. Because the creator in start_kthread does not hold an
extra reference, the task_struct is freed upon exit.

When the tracer is later disabled or the CPU goes offline, stop_kthread
will unconditionally call kthread_stop(osn_var->kthread) on this freed
pointer. Can this cause a use-after-free crash?

[Severity: High]
This is a pre-existing issue, but does a deadlock exist between
stopping the kthread and handling pending migrations?

If a user disables the tracer, stop_kthread successfully takes
osn_var->lock and calls kthread_stop.

Concurrently, if userspace alters the kthread's CPU affinity, the scheduler
sets current->migration_pending = 1 because the kthread is pinned by
migrate_disable.

The kthread loop calls osnoise_migration_pending, detects this flag, and
attempts to acquire mutex_lock(&osn->lock).

Will kthread_stop block indefinitely waiting for the kthread to exit,
while the kthread blocks indefinitely on the mutex held by stop_kthread?

+
+	tlat = this_cpu_tmr_var();
+

[ ... ]

[Severity: High]
This is a pre-existing issue, but does osnoise_main have the same
migration vulnerability that was just addressed in timerlat_main?

start_kthread now passes cpu_osn_var(cpu) to both timerlat_main and
osnoise_main via the data parameter. However, osnoise_main ignores its
data argument and blindly pins itself without validating the CPU:

kernel/trace/trace_osnoise.c:osnoise_main() {
    unsigned long flags;
    ...
    migrate_disable();
    ...
}

If osnoise_main is migrated by hotplug activity before migrate_disable,
does it pin itself to the wrong CPU, causing two tracing threads to
concurrently overwrite the same tracking data without synchronization?

[ ... ]

[Severity: High]
Is osn->cpu correctly initialized for CPUs that are hotplugged after boot?

Because this loop uses for_each_online_cpu, CPUs that are offline during
tracer initialization will have osn->cpu left as 0.

When timerlat_main later starts on a newly hotplugged CPU, the migration
check (osn_var->cpu != smp_processor_id()) will incorrectly evaluate to
true.

Will this cause the kthread to immediately abort upon creation, breaking
hotplug functionality and triggering the dangling kthread regression?

[Severity: High]
This is a pre-existing issue, but is the mutex initialization skipping
offline CPUs?

Because mutex_init(&osn->lock) is strictly within the for_each_online_cpu
loop, CPUs brought online after boot will have a zero-initialized lock.

When start_kthread runs on a hotplugged CPU, it attempts to lock this
uninitialized mutex. Could this trigger a lockdep splat or a null pointer
dereference?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260824211544.3984835-1-crwood@redhat.com?part=2