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: <20231120221524.GD8262@noisy.programming.kicks-ass.net>
Date:   Mon, 20 Nov 2023 23:15:24 +0100
From:   Peter Zijlstra <peterz@...radead.org>
To:     Mathieu Desnoyers <mathieu.desnoyers@...icios.com>
Cc:     Steven Rostedt <rostedt@...dmis.org>,
        Masami Hiramatsu <mhiramat@...nel.org>,
        linux-kernel@...r.kernel.org,
        Michael Jeanson <mjeanson@...icios.com>,
        Alexei Starovoitov <ast@...nel.org>,
        Yonghong Song <yhs@...com>,
        "Paul E . McKenney" <paulmck@...nel.org>,
        Ingo Molnar <mingo@...hat.com>,
        Arnaldo Carvalho de Melo <acme@...nel.org>,
        Mark Rutland <mark.rutland@....com>,
        Alexander Shishkin <alexander.shishkin@...ux.intel.com>,
        Jiri Olsa <jolsa@...hat.com>,
        Namhyung Kim <namhyung@...nel.org>, bpf@...r.kernel.org,
        Joel Fernandes <joel@...lfernandes.org>
Subject: Re: [PATCH v4 2/5] tracing/ftrace: Add support for faultable
 tracepoints

On Mon, Nov 20, 2023 at 03:54:15PM -0500, Mathieu Desnoyers wrote:
> @@ -380,8 +415,8 @@ static inline notrace int trace_event_get_offsets_##call(		\
>  
>  #include "stages/stage6_event_callback.h"
>  
> -#undef DECLARE_EVENT_CLASS
> -#define DECLARE_EVENT_CLASS(call, proto, args, tstruct, assign, print)	\
> +#undef _DECLARE_EVENT_CLASS
> +#define _DECLARE_EVENT_CLASS(call, proto, args, tstruct, assign, print, tp_flags) \
>  									\
>  static notrace void							\
>  trace_event_raw_event_##call(void *__data, proto)			\
> @@ -392,8 +427,13 @@ trace_event_raw_event_##call(void *__data, proto)			\
>  	struct trace_event_raw_##call *entry;				\
>  	int __data_size;						\
>  									\
> +	if ((tp_flags) & TRACEPOINT_MAY_FAULT) {			\
> +		might_fault();						\
> +		preempt_disable_notrace();				\
> +	}								\
> +									\
>  	if (trace_trigger_soft_disabled(trace_file))			\
> -		return;							\
> +		goto end;						\
>  									\
>  	__data_size = trace_event_get_offsets_##call(&__data_offsets, args); \
>  									\
> @@ -401,14 +441,28 @@ trace_event_raw_event_##call(void *__data, proto)			\
>  				 sizeof(*entry) + __data_size);		\
>  									\
>  	if (!entry)							\
> -		return;							\
> +		goto end;						\
>  									\
>  	tstruct								\
>  									\
>  	{ assign; }							\
>  									\
>  	trace_event_buffer_commit(&fbuffer);				\
> +end:									\
> +	if ((tp_flags) & TRACEPOINT_MAY_FAULT)				\
> +		preempt_enable_notrace();				\
>  }

> diff --git a/kernel/trace/trace_events.c b/kernel/trace/trace_events.c
> index 82cb22ad6d61..954f87515668 100644
> --- a/kernel/trace/trace_events.c
> +++ b/kernel/trace/trace_events.c
> @@ -532,9 +532,16 @@ int trace_event_reg(struct trace_event_call *call,
>  	WARN_ON(!(call->flags & TRACE_EVENT_FL_TRACEPOINT));
>  	switch (type) {
>  	case TRACE_REG_REGISTER:
> -		return tracepoint_probe_register(call->tp,
> -						 call->class->probe,
> -						 file);
> +		if (call->tp->flags & TRACEPOINT_MAY_FAULT)
> +			return tracepoint_probe_register_prio_flags(call->tp,
> +								    call->class->probe,
> +								    file,
> +								    TRACEPOINT_DEFAULT_PRIO,
> +								    TRACEPOINT_MAY_FAULT);
> +		else
> +			return tracepoint_probe_register(call->tp,
> +							 call->class->probe,
> +							 file);
>  	case TRACE_REG_UNREGISTER:
>  		tracepoint_probe_unregister(call->tp,
>  					    call->class->probe,
> @@ -543,9 +550,16 @@ int trace_event_reg(struct trace_event_call *call,
>  
>  #ifdef CONFIG_PERF_EVENTS
>  	case TRACE_REG_PERF_REGISTER:
> -		return tracepoint_probe_register(call->tp,
> -						 call->class->perf_probe,
> -						 call);
> +		if (call->tp->flags & TRACEPOINT_MAY_FAULT)
> +			return tracepoint_probe_register_prio_flags(call->tp,
> +								    call->class->perf_probe,
> +								    call,
> +								    TRACEPOINT_DEFAULT_PRIO,
> +								    TRACEPOINT_MAY_FAULT);
> +		else
> +			return tracepoint_probe_register(call->tp,
> +							 call->class->perf_probe,
> +							 call);
>  	case TRACE_REG_PERF_UNREGISTER:
>  		tracepoint_probe_unregister(call->tp,
>  					    call->class->perf_probe,

I think something like the below (which is on top of the cleanup patch
in tip/locking/core) might just do...

---
diff --git a/include/linux/cleanup.h b/include/linux/cleanup.h
index c2d09bc4f976..37cbdb19d81d 100644
--- a/include/linux/cleanup.h
+++ b/include/linux/cleanup.h
@@ -151,7 +151,9 @@ static inline class_##_name##_t class_##_name##ext##_constructor(_init_args) \
 #define DEFINE_GUARD(_name, _type, _lock, _unlock) \
 	DEFINE_CLASS(_name, _type, if (_T) { _unlock; }, ({ _lock; _T; }), _type _T); \
 	static inline void * class_##_name##_lock_ptr(class_##_name##_t *_T) \
-	{ return *_T; }
+	{ return *_T; } \
+	static inline class_##_name##_t class_##_name##_null(void) \
+	{ return NULL; }
 
 #define DEFINE_GUARD_COND(_name, _ext, _condlock) \
 	EXTEND_CLASS(_name, _ext, \
@@ -175,6 +177,17 @@ static inline class_##_name##_t class_##_name##ext##_constructor(_init_args) \
 		if (!__guard_ptr(_name)(&scope)) _fail; \
 		else
 
+
+#define __guard_if(_cond, _name, var) \
+	class_##_name##_t var __cleanup(class_##_name##_destructor) = \
+		class_##_name##_null();	\
+	if (_cond) \
+		var = class_##_name##_constructor
+
+#define guard_if(_cond, _name) \
+	__guard_if(_cond, _name, __UNIQUE_ID(guard))
+
+
 /*
  * Additional helper macros for generating lock guards with types, either for
  * locks that don't have a native type (eg. RCU, preempt) or those that need a
@@ -209,6 +222,12 @@ static inline void class_##_name##_destructor(class_##_name##_t *_T)	\
 static inline void *class_##_name##_lock_ptr(class_##_name##_t *_T)	\
 {									\
 	return _T->lock;						\
+}									\
+									\
+static inline class_##_name##_t class_##_name##_null(void)		\
+{									\
+	class_##_name##_t _t = { };					\
+	return _t;							\
 }
 
 
diff --git a/include/trace/trace_events.h b/include/trace/trace_events.h
index 558af4960157..8e063c9846e0 100644
--- a/include/trace/trace_events.h
+++ b/include/trace/trace_events.h
@@ -427,13 +427,10 @@ trace_event_raw_event_##call(void *__data, proto)			\
 	struct trace_event_raw_##call *entry;				\
 	int __data_size;						\
 									\
-	if ((tp_flags) & TRACEPOINT_MAY_FAULT) {			\
-		might_fault();						\
-		preempt_disable_notrace();				\
-	}								\
+	guard_if((tp_flags) & TRACEPOINT_MAY_FAULT, preempt_notrace)();	\
 									\
 	if (trace_trigger_soft_disabled(trace_file))			\
-		goto end;						\
+		return;							\
 									\
 	__data_size = trace_event_get_offsets_##call(&__data_offsets, args); \
 									\
@@ -441,16 +438,13 @@ trace_event_raw_event_##call(void *__data, proto)			\
 				 sizeof(*entry) + __data_size);		\
 									\
 	if (!entry)							\
-		goto end;						\
+		return;							\
 									\
 	tstruct								\
 									\
 	{ assign; }							\
 									\
 	trace_event_buffer_commit(&fbuffer);				\
-end:									\
-	if ((tp_flags) & TRACEPOINT_MAY_FAULT)				\
-		preempt_enable_notrace();				\
 }
 
 #undef DECLARE_EVENT_CLASS
diff --git a/kernel/trace/trace_events.c b/kernel/trace/trace_events.c
index bf53d0d3eef9..09aec5db2e74 100644
--- a/kernel/trace/trace_events.c
+++ b/kernel/trace/trace_events.c
@@ -532,16 +532,11 @@ int trace_event_reg(struct trace_event_call *call,
 	WARN_ON(!(call->flags & TRACE_EVENT_FL_TRACEPOINT));
 	switch (type) {
 	case TRACE_REG_REGISTER:
-		if (call->tp->flags & TRACEPOINT_MAY_FAULT)
-			return tracepoint_probe_register_prio_flags(call->tp,
-								    call->class->probe,
-								    file,
-								    TRACEPOINT_DEFAULT_PRIO,
-								    TRACEPOINT_MAY_FAULT);
-		else
-			return tracepoint_probe_register(call->tp,
-							 call->class->probe,
-							 file);
+		return tracepoint_probe_register_prio_flags(call->tp,
+							    call->class->probe,
+							    file,
+							    TRACEPOINT_DEFAULT_PRIO,
+							    call->tp->flags & TRACEPOINT_MAY_FAULT);
 	case TRACE_REG_UNREGISTER:
 		tracepoint_probe_unregister(call->tp,
 					    call->class->probe,
@@ -550,16 +545,11 @@ int trace_event_reg(struct trace_event_call *call,
 
 #ifdef CONFIG_PERF_EVENTS
 	case TRACE_REG_PERF_REGISTER:
-		if (call->tp->flags & TRACEPOINT_MAY_FAULT)
-			return tracepoint_probe_register_prio_flags(call->tp,
-								    call->class->perf_probe,
-								    call,
-								    TRACEPOINT_DEFAULT_PRIO,
-								    TRACEPOINT_MAY_FAULT);
-		else
-			return tracepoint_probe_register(call->tp,
-							 call->class->perf_probe,
-							 call);
+		return tracepoint_probe_register_prio_flags(call->tp,
+							    call->class->perf_probe,
+							    call,
+							    TRACEPOINT_DEFAULT_PRIO,
+							    call->tp->flags & TRACEPOINT_MAY_FAULT);
 	case TRACE_REG_PERF_UNREGISTER:
 		tracepoint_probe_unregister(call->tp,
 					    call->class->perf_probe,

Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ