lists.openwall.net   lists  /  announce  owl-users  owl-dev  john-users  john-dev  passwdqc-users  yescrypt  popa3d-users  /  oss-security  kernel-hardening  musl  sabotage  tlsify  passwords  /  crypt-dev  xvendor  /  Bugtraq  Full-Disclosure  linux-kernel  linux-netdev  linux-ext4  linux-hardening  linux-cve-announce  PHC 
Open Source and information security mailing list archives
 
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <20250930084645.GJ3245006@noisy.programming.kicks-ass.net>
Date: Tue, 30 Sep 2025 10:46:45 +0200
From: Peter Zijlstra <peterz@...radead.org>
To: chenyuan_fl@....com
Cc: john.ogness@...utronix.de, rostedt@...dmis.org, bigeasy@...utronix.de,
	chenyuan@...inos.cn, linux-kernel@...r.kernel.org,
	linux-trace-kernel@...r.kernel.org, mathieu.desnoyers@...icios.com,
	mhiramat@...nel.org
Subject: Re: [PATH v3] tracing: Fix race condition in kprobe initialization
 causing NULL pointer dereference

On Tue, Sep 30, 2025 at 09:18:48AM +0100, chenyuan_fl@....com wrote:

> diff --git a/kernel/trace/trace_probe.h b/kernel/trace/trace_probe.h
> index 842383fbc03b..98b838591edc 100644
> --- a/kernel/trace/trace_probe.h
> +++ b/kernel/trace/trace_probe.h
> @@ -274,19 +274,19 @@ struct event_file_link {
>  static inline bool trace_probe_test_flag(struct trace_probe *tp,
>  					 unsigned int flag)
>  {
> -	return !!(tp->event->flags & flag);
> +	return !!(smp_load_acquire(&tp->event->flags) & flag);
>  }
>  
>  static inline void trace_probe_set_flag(struct trace_probe *tp,
>  					unsigned int flag)
>  {
> -	tp->event->flags |= flag;
> +	smp_store_release(&tp->event->flags, tp->event->flags | flag);
>  }
>  
>  static inline void trace_probe_clear_flag(struct trace_probe *tp,
>  					  unsigned int flag)
>  {
> -	tp->event->flags &= ~flag;
> +	smp_store_release(&tp->event->flags, tp->event->flags & ~flag);
>  }


I _think_ the clear one is superfluous. Is there anything that cares
about stores done before the clear when the flag is found not set?

Also, code like:

static int fentry_dispatcher(struct fprobe *fp, unsigned long entry_ip,
			     unsigned long ret_ip, struct ftrace_regs *fregs,
			     void *entry_data)
{
	struct trace_fprobe *tf = container_of(fp, struct trace_fprobe, fp);
	int ret = 0;

	if (trace_probe_test_flag(&tf->tp, TP_FLAG_TRACE))
		fentry_trace_func(tf, entry_ip, fregs);

#ifdef CONFIG_PERF_EVENTS
	if (trace_probe_test_flag(&tf->tp, TP_FLAG_PROFILE))
		ret = fentry_perf_func(tf, entry_ip, fregs);
#endif
	return ret;
}


Will now have two barriers; where one would suffice, eg.

	flags = smp_load_acquire(&tp->event->flags);

	if (flags & TP_FLAG_TRACE)
		fentry_trace_func(...);

	if (flags & TP_FLAG_PROFILE)
		fentry_perf_func(...);

Should be just fine afaict.


Is this something anybody cares about?

Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ