All of lore.kernel.org
 help / color / mirror / Atom feed
From: sashiko-bot@kernel.org
To: "Crystal Wood" <crwood@redhat.com>
Cc: linux-trace-kernel@vger.kernel.org
Subject: Re: [PATCH v3 9/9] tracing/osnoise: Refuse timerlat fd open if tracer inactive
Date: Wed, 14 Oct 2026 02:39:30 +0000	[thread overview]
Message-ID: <sashiko-outbox-170742@kernel.org> (raw)
In-Reply-To: <20261014022640.695786-10-crwood@redhat.com>

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] trace: timerlat_fd_release unconditionally resets cpu state
--

commit 3a852123b0f7d3294fb2be0fe904429d8ee1fccc
Author: Crystal Wood <crwood@redhat.com>

tracing/osnoise: Refuse timerlat fd open if tracer inactive

This commit refuses to open a timerlat file descriptor when there are no
active instances. This prevents the file descriptor from remaining active
during reconfiguration, which could cause WARN splats.

> diff --git a/kernel/trace/trace_osnoise.c b/kernel/trace/trace_osnoise.c
> index 9ca45b0b8cb39..2511a9b763b98 100644
> --- a/kernel/trace/trace_osnoise.c
> +++ b/kernel/trace/trace_osnoise.c
> @@ -2506,7 +2506,8 @@ static int timerlat_fd_open(struct inode *inode, struct file *file)
>  	 * This file is accessible only if timerlat is enabled, and
>  	 * NO_OSNOISE_WORKLOAD is set.
>  	 */
> -	if (!timerlat_enabled() || test_bit(OSN_WORKLOAD, &osnoise_options))
> +	if (!osnoise_has_registered_instances() || !timerlat_enabled() ||
> +	    test_bit(OSN_WORKLOAD, &osnoise_options))
>  		return -EINVAL;
>  
>  	guard(migrate)();

[Severity: High]
Does this code still allow state corruption during the fd release path?

Looking at timerlat_fd_release() in kernel/trace/trace_osnoise.c,
osn_reset_cpu() is called unconditionally, bypassing the active session check:

kernel/trace/trace_osnoise.c:timerlat_fd_release() {
    ...
	if (fdd == osn_var->fdd)
		timerlat_fd_detach(fdd);
	osn_reset_cpu(fdd->cpu);
    ...
}

Can the following sequence occur?

1. Userspace opens a timerlat fd, creating an active tracing session (fdd1).

2. Tracing is stopped or reconfigured (e.g. via
   /sys/kernel/tracing/osnoise/cpus), calling stop_kthread(). This detaches
   fdd1 (osn_var->fdd = NULL) but leaves the fd open in userspace.

3. Tracing is restarted, and userspace opens a new timerlat fd on the same
   CPU (fdd2). This becomes the active session (osn_var->fdd = fdd2) and
   tlat_var->timer is initialized and armed.

4. Userspace closes the first fd (fdd1).

5. timerlat_fd_release() is called. Because fdd1 != osn_var->fdd, it skips
   detachment, but unconditionally calls osn_reset_cpu(fdd1->cpu).

Would this unconditional reset wipe the active tracing session's state,
including zero-filling the armed hrtimer of the active fdd2 session,
potentially leading to timer tree corruption and a kernel panic?

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

      reply	other threads:[~2026-10-14  2:39 UTC|newest]

Thread overview: 12+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-10-14  2:26 [PATCH v3 0/9] tracing/osnoise: Synchronization fixes Crystal Wood
2026-10-14  2:26 ` [PATCH v3 1/9] tracing/osnoise: Variable cleanup Crystal Wood
2026-10-14  2:26 ` [PATCH v3 2/9] tracing/osnoise: Hold a ref on osn->kthread Crystal Wood
2026-10-14  2:26 ` [PATCH v3 3/9] tracing/osnoise: Per-cpu mutexes and fd detachment Crystal Wood
2026-10-14  2:26 ` [PATCH v3 4/9] tracing/osnoise: kthreads: Verify CPU after migrate_disable() Crystal Wood
2026-10-14  2:26 ` [PATCH v3 5/9] tracing/osnoise: start_kthread: Always check OSN_WORKLOAD Crystal Wood
2026-10-14  2:26 ` [PATCH v3 6/9] tracing/osnoise: Take trace_types_lock in timerlat_fd_open Crystal Wood
2026-10-14  2:26 ` [PATCH v3 7/9] tracing/osnoise: Split out per-cpu variable resets Crystal Wood
2026-10-14  2:39   ` sashiko-bot
2026-10-14  2:26 ` [PATCH v3 8/9] tracing/osnoise: Print taint message for timerlat Crystal Wood
2026-10-14  2:26 ` [PATCH v3 9/9] tracing/osnoise: Refuse timerlat fd open if tracer inactive Crystal Wood
2026-10-14  2:39   ` sashiko-bot [this message]

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=sashiko-outbox-170742@kernel.org \
    --to=sashiko-bot@kernel.org \
    --cc=crwood@redhat.com \
    --cc=linux-trace-kernel@vger.kernel.org \
    --cc=sashiko-reviews@lists.linux.dev \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.