linux-kernel - Re: [PATCH v2] tracing: Verify event formats that have "%*p.."

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] [day] [month] [year] [list]

Message-ID: <ffeb5e0a-6968-42d4-bcbb-bd8fc5341a50@oracle.com>
Date: Fri, 28 Mar 2025 16:22:27 -0700
From: Libo Chen <libo.chen@...cle.com>
To: Steven Rostedt <rostedt@...dmis.org>, LKML
 <linux-kernel@...r.kernel.org>,
        Linux Trace Kernel <linux-trace-kernel@...r.kernel.org>
Cc: Masami Hiramatsu <mhiramat@...nel.org>,
        Mathieu Desnoyers <mathieu.desnoyers@...icios.com>
Subject: Re: [PATCH v2] tracing: Verify event formats that have "%*p.."



On 3/27/25 16:53, Steven Rostedt wrote:
> From: Steven Rostedt <rostedt@...dmis.org>
> 
> The trace event verifier checks the formats of trace events to make sure
> that they do not point at memory that is not in the trace event itself or
> in data that will never be freed. If an event references data that was
> allocated when the event triggered and that same data is freed before the
> event is read, then the kernel can crash by reading freed memory.
> 
> The verifier runs at boot up (or module load) and scans the print formats
> of the events and checks their arguments to make sure that dereferenced
> pointers are safe. If the format uses "%*p.." the verifier will ignore it,
> and that could be dangerous. Cover this case as well.
> 
> Also add to the sample code a use case of "%*pbl".
> 
> Link: https://urldefense.com/v3/__https://lore.kernel.org/all/bcba4d76-2c3f-4d11-baf0-02905db953dd@oracle.com/__;!!ACWV5N9M2RV99hQ!L9dryF2Ft0p8H1X2Q6EPsm0j03lJOixhB2V-_zkgCOBUoIS5V7B8hmOZxCpHjzNnCodkqNmxUulCC5wPbg$ 
> 

this is no longer a blocker for me, as I mentioned in a different thread:

https://lore.kernel.org/all/4f5453e8-61b6-45d1-8c6c-8b4fb99a6448@oracle.com/


Thanks,
Libo

> Reported-by: Libo Chen <libo.chen@...cle.com>
> Reviewed-by: Libo Chen <libo.chen@...cle.com>
> Tested-by: Libo Chen <libo.chen@...cle.com>
> Signed-off-by: Steven Rostedt (Google) <rostedt@...dmis.org>
> ---
> Changes since v1: https://urldefense.com/v3/__https://lore.kernel.org/20250327114911.2c713511@gandalf.local.home__;!!ACWV5N9M2RV99hQ!L9dryF2Ft0p8H1X2Q6EPsm0j03lJOixhB2V-_zkgCOBUoIS5V7B8hmOZxCpHjzNnCodkqNmxUumNh4GyCw$ 
> 
> - Fixed indentation in sample header.
> 
>  kernel/trace/trace_events.c                | 7 +++++++
>  samples/trace_events/trace-events-sample.h | 8 ++++++--
>  2 files changed, 13 insertions(+), 2 deletions(-)
> 
> diff --git a/kernel/trace/trace_events.c b/kernel/trace/trace_events.c
> index 8e7603acca21..ceeedcb5940b 100644
> --- a/kernel/trace/trace_events.c
> +++ b/kernel/trace/trace_events.c
> @@ -470,6 +470,7 @@ static void test_event_printk(struct trace_event_call *call)
>  			case '%':
>  				continue;
>  			case 'p':
> + do_pointer:
>  				/* Find dereferencing fields */
>  				switch (fmt[i + 1]) {
>  				case 'B': case 'R': case 'r':
> @@ -498,6 +499,12 @@ static void test_event_printk(struct trace_event_call *call)
>  						continue;
>  					if (fmt[i + j] == '*') {
>  						star = true;
> +						/* Handle %*pbl case */
> +						if (!j && fmt[i + 1] == 'p') {
> +							arg++;
> +							i++;
> +							goto do_pointer;
> +						}
>  						continue;
>  					}
>  					if ((fmt[i + j] == 's')) {
> diff --git a/samples/trace_events/trace-events-sample.h b/samples/trace_events/trace-events-sample.h
> index 999f78d380ae..1a05fc153353 100644
> --- a/samples/trace_events/trace-events-sample.h
> +++ b/samples/trace_events/trace-events-sample.h
> @@ -319,7 +319,8 @@ TRACE_EVENT(foo_bar,
>  		__assign_cpumask(cpum, cpumask_bits(mask));
>  	),
>  
> -	TP_printk("foo %s %d %s %s %s %s %s %s (%s) (%s) %s", __entry->foo, __entry->bar,
> +	TP_printk("foo %s %d %s %s %s %s %s %s (%s) (%s) %s [%d] %*pbl",
> +		  __entry->foo, __entry->bar,
>  
>  /*
>   * Notice here the use of some helper functions. This includes:
> @@ -370,7 +371,10 @@ TRACE_EVENT(foo_bar,
>  
>  		  __get_str(str), __get_str(lstr),
>  		  __get_bitmask(cpus), __get_cpumask(cpum),
> -		  __get_str(vstr))
> +		  __get_str(vstr),
> +		  __get_dynamic_array_len(cpus),
> +		  __get_dynamic_array_len(cpus),
> +		  __get_dynamic_array(cpus))
>  );
>  
>  /*