diff mbox series

[1/8] ring-buffer: Use kaslr address instead of text delta

Message ID 20250205225103.090463674@goodmis.org (mailing list archive)
State Superseded
Headers show
Series ring-buffer/tracing: Save module information in persistent memory | expand

Commit Message

Steven Rostedt Feb. 5, 2025, 10:50 p.m. UTC 
From: Steven Rostedt <rostedt@goodmis.org>

Instead of saving off the text and data pointers and using them to compare
with the current boot's text and data pointers, just save off the KASLR
offset. Then that can be used to figure out how to read the previous boots
buffer.

The last_boot_info will now show this offset, but only if it is for a
previous boot:

  # cat instances/boot_mapped/last_boot_info
  Offset: 39000000

  # echo function > instances/boot_mapped/current_tracer
  # cat instances/boot_mapped/last_boot_info
  Offset: current

If the KASLR offset saved is for the current boot, the last_boot_info will
show the value of "current".

Signed-off-by: Steven Rostedt (Google) <rostedt@goodmis.org>
---
 include/linux/ring_buffer.h |  3 +--
 kernel/trace/ring_buffer.c  | 31 ++++++++++++-------------------
 kernel/trace/trace.c        | 30 +++++++++++++++++++++---------
 kernel/trace/trace.h        |  9 +++++----
 4 files changed, 39 insertions(+), 34 deletions(-)

Comments

Masami Hiramatsu (Google) Feb. 6, 2025, 12:32 a.m. UTC | #1 
On Wed, 05 Feb 2025 17:50:32 -0500
Steven Rostedt <rostedt@goodmis.org> wrote:

> From: Steven Rostedt <rostedt@goodmis.org>
> 
> Instead of saving off the text and data pointers and using them to compare
> with the current boot's text and data pointers, just save off the KASLR
> offset. Then that can be used to figure out how to read the previous boots
> buffer.
> 
> The last_boot_info will now show this offset, but only if it is for a
> previous boot:
> 
>   # cat instances/boot_mapped/last_boot_info
>   Offset: 39000000
> 
>   # echo function > instances/boot_mapped/current_tracer
>   # cat instances/boot_mapped/last_boot_info
>   Offset: current
> 
> If the KASLR offset saved is for the current boot, the last_boot_info will
> show the value of "current".

Looks good to me.

Acked-by: Masami Hiramatsu (Google) <mhiramat@kernel.org>

Thank you!

> 
> Signed-off-by: Steven Rostedt (Google) <rostedt@goodmis.org>
> ---
>  include/linux/ring_buffer.h |  3 +--
>  kernel/trace/ring_buffer.c  | 31 ++++++++++++-------------------
>  kernel/trace/trace.c        | 30 +++++++++++++++++++++---------
>  kernel/trace/trace.h        |  9 +++++----
>  4 files changed, 39 insertions(+), 34 deletions(-)
> 
> diff --git a/include/linux/ring_buffer.h b/include/linux/ring_buffer.h
> index 17fbb7855295..8de035f4f0d9 100644
> --- a/include/linux/ring_buffer.h
> +++ b/include/linux/ring_buffer.h
> @@ -94,8 +94,7 @@ struct trace_buffer *__ring_buffer_alloc_range(unsigned long size, unsigned flag
>  					       unsigned long range_size,
>  					       struct lock_class_key *key);
>  
> -bool ring_buffer_last_boot_delta(struct trace_buffer *buffer, long *text,
> -				 long *data);
> +bool ring_buffer_last_boot_delta(struct trace_buffer *buffer, unsigned long *kaslr_addr);
>  
>  /*
>   * Because the ring buffer is generic, if other users of the ring buffer get
> diff --git a/kernel/trace/ring_buffer.c b/kernel/trace/ring_buffer.c
> index b8e0ae15ca5b..7146b780176f 100644
> --- a/kernel/trace/ring_buffer.c
> +++ b/kernel/trace/ring_buffer.c
> @@ -31,6 +31,7 @@
>  
>  #include <asm/local64.h>
>  #include <asm/local.h>
> +#include <asm/setup.h>
>  
>  #include "trace.h"
>  
> @@ -49,8 +50,7 @@ static void update_pages_handler(struct work_struct *work);
>  struct ring_buffer_meta {
>  	int		magic;
>  	int		struct_size;
> -	unsigned long	text_addr;
> -	unsigned long	data_addr;
> +	unsigned long	kaslr_addr;
>  	unsigned long	first_buffer;
>  	unsigned long	head_buffer;
>  	unsigned long	commit_buffer;
> @@ -550,8 +550,7 @@ struct trace_buffer {
>  	unsigned long			range_addr_start;
>  	unsigned long			range_addr_end;
>  
> -	long				last_text_delta;
> -	long				last_data_delta;
> +	unsigned long			kaslr_addr;
>  
>  	unsigned int			subbuf_size;
>  	unsigned int			subbuf_order;
> @@ -1874,16 +1873,13 @@ static void rb_meta_validate_events(struct ring_buffer_per_cpu *cpu_buffer)
>  	}
>  }
>  
> -/* Used to calculate data delta */
> -static char rb_data_ptr[] = "";
> -
> -#define THIS_TEXT_PTR		((unsigned long)rb_meta_init_text_addr)
> -#define THIS_DATA_PTR		((unsigned long)rb_data_ptr)
> -
>  static void rb_meta_init_text_addr(struct ring_buffer_meta *meta)
>  {
> -	meta->text_addr = THIS_TEXT_PTR;
> -	meta->data_addr = THIS_DATA_PTR;
> +#ifdef CONFIG_RANDOMIZE_BASE
> +	meta->kaslr_addr = kaslr_offset();
> +#else
> +	meta->kaslr_addr = 0;
> +#endif
>  }
>  
>  static void rb_range_meta_init(struct trace_buffer *buffer, int nr_pages)
> @@ -1906,8 +1902,7 @@ static void rb_range_meta_init(struct trace_buffer *buffer, int nr_pages)
>  			meta->first_buffer += delta;
>  			meta->head_buffer += delta;
>  			meta->commit_buffer += delta;
> -			buffer->last_text_delta = THIS_TEXT_PTR - meta->text_addr;
> -			buffer->last_data_delta = THIS_DATA_PTR - meta->data_addr;
> +			buffer->kaslr_addr = meta->kaslr_addr;
>  			continue;
>  		}
>  
> @@ -2459,17 +2454,15 @@ struct trace_buffer *__ring_buffer_alloc_range(unsigned long size, unsigned flag
>   *
>   * Returns: The true if the delta is non zero
>   */
> -bool ring_buffer_last_boot_delta(struct trace_buffer *buffer, long *text,
> -				 long *data)
> +bool ring_buffer_last_boot_delta(struct trace_buffer *buffer, unsigned long *kaslr_addr)
>  {
>  	if (!buffer)
>  		return false;
>  
> -	if (!buffer->last_text_delta)
> +	if (!buffer->kaslr_addr)
>  		return false;
>  
> -	*text = buffer->last_text_delta;
> -	*data = buffer->last_data_delta;
> +	*kaslr_addr = buffer->kaslr_addr;
>  
>  	return true;
>  }
> diff --git a/kernel/trace/trace.c b/kernel/trace/trace.c
> index 1496a5ac33ae..a9e8eaf1d47e 100644
> --- a/kernel/trace/trace.c
> +++ b/kernel/trace/trace.c
> @@ -50,7 +50,7 @@
>  #include <linux/irq_work.h>
>  #include <linux/workqueue.h>
>  
> -#include <asm/setup.h> /* COMMAND_LINE_SIZE */
> +#include <asm/setup.h> /* COMMAND_LINE_SIZE and kaslr_offset() */
>  
>  #include "trace.h"
>  #include "trace_output.h"
> @@ -4193,7 +4193,7 @@ static enum print_line_t print_trace_fmt(struct trace_iterator *iter)
>  		 * safe to use if the array has delta offsets
>  		 * Force printing via the fields.
>  		 */
> -		if ((tr->text_delta || tr->data_delta) &&
> +		if ((tr->text_delta) &&
>  		    event->type > __TRACE_LAST_TYPE)
>  			return print_event_fields(iter, event);
>  
> @@ -5996,7 +5996,7 @@ ssize_t tracing_resize_ring_buffer(struct trace_array *tr,
>  
>  static void update_last_data(struct trace_array *tr)
>  {
> -	if (!tr->text_delta && !tr->data_delta)
> +	if (!(tr->flags & TRACE_ARRAY_FL_LAST_BOOT))
>  		return;
>  
>  	/*
> @@ -6009,7 +6009,8 @@ static void update_last_data(struct trace_array *tr)
>  
>  	/* Using current data now */
>  	tr->text_delta = 0;
> -	tr->data_delta = 0;
> +
> +	tr->flags &= ~TRACE_ARRAY_FL_LAST_BOOT;
>  }
>  
>  /**
> @@ -6827,8 +6828,17 @@ tracing_last_boot_read(struct file *filp, char __user *ubuf, size_t cnt, loff_t
>  
>  	seq_buf_init(&seq, buf, 64);
>  
> -	seq_buf_printf(&seq, "text delta:\t%ld\n", tr->text_delta);
> -	seq_buf_printf(&seq, "data delta:\t%ld\n", tr->data_delta);
> +	/*
> +	 * Do not leak KASLR address. This only shows the KASLR address of
> +	 * the last boot. When the ring buffer is started, the LAST_BOOT
> +	 * flag gets cleared, and this should only report "current".
> +	 * Otherwise it shows the KASLR address from the previous boot which
> +	 * should not be the same as the current boot.
> +	 */
> +	if (!(tr->flags & TRACE_ARRAY_FL_LAST_BOOT))
> +		seq_buf_puts(&seq, "Offset: current\n");
> +	else
> +		seq_buf_printf(&seq, "Offset: %lx\n", tr->kaslr_addr);
>  
>  	return simple_read_from_buffer(ubuf, cnt, ppos, buf, seq_buf_used(&seq));
>  }
> @@ -9212,8 +9222,10 @@ allocate_trace_buffer(struct trace_array *tr, struct array_buffer *buf, int size
>  						      tr->range_addr_start,
>  						      tr->range_addr_size);
>  
> -		ring_buffer_last_boot_delta(buf->buffer,
> -					    &tr->text_delta, &tr->data_delta);
> +#ifdef CONFIG_RANDOMIZE_BASE
> +		if (ring_buffer_last_boot_delta(buf->buffer, &tr->kaslr_addr))
> +			tr->text_delta = kaslr_offset() - tr->kaslr_addr;
> +#endif
>  		/*
>  		 * This is basically the same as a mapped buffer,
>  		 * with the same restrictions.
> @@ -10461,7 +10473,7 @@ __init static void enable_instances(void)
>  		 * to it.
>  		 */
>  		if (start) {
> -			tr->flags |= TRACE_ARRAY_FL_BOOT;
> +			tr->flags |= TRACE_ARRAY_FL_BOOT | TRACE_ARRAY_FL_LAST_BOOT;
>  			tr->ref++;
>  		}
>  
> diff --git a/kernel/trace/trace.h b/kernel/trace/trace.h
> index 9c21ba45b7af..abe8169c3e87 100644
> --- a/kernel/trace/trace.h
> +++ b/kernel/trace/trace.h
> @@ -348,8 +348,8 @@ struct trace_array {
>  	unsigned int		mapped;
>  	unsigned long		range_addr_start;
>  	unsigned long		range_addr_size;
> +	unsigned long		kaslr_addr;
>  	long			text_delta;
> -	long			data_delta;
>  
>  	struct trace_pid_list	__rcu *filtered_pids;
>  	struct trace_pid_list	__rcu *filtered_no_pids;
> @@ -433,9 +433,10 @@ struct trace_array {
>  };
>  
>  enum {
> -	TRACE_ARRAY_FL_GLOBAL	= BIT(0),
> -	TRACE_ARRAY_FL_BOOT	= BIT(1),
> -	TRACE_ARRAY_FL_MOD_INIT	= BIT(2),
> +	TRACE_ARRAY_FL_GLOBAL		= BIT(0),
> +	TRACE_ARRAY_FL_BOOT		= BIT(1),
> +	TRACE_ARRAY_FL_LAST_BOOT	= BIT(2),
> +	TRACE_ARRAY_FL_MOD_INIT		= BIT(3),
>  };
>  
>  #ifdef CONFIG_MODULES
> -- 
> 2.45.2
> 
>
diff mbox series

Patch

diff --git a/include/linux/ring_buffer.h b/include/linux/ring_buffer.h
index 17fbb7855295..8de035f4f0d9 100644
--- a/include/linux/ring_buffer.h
+++ b/include/linux/ring_buffer.h
@@ -94,8 +94,7 @@  struct trace_buffer *__ring_buffer_alloc_range(unsigned long size, unsigned flag
 					       unsigned long range_size,
 					       struct lock_class_key *key);
 
-bool ring_buffer_last_boot_delta(struct trace_buffer *buffer, long *text,
-				 long *data);
+bool ring_buffer_last_boot_delta(struct trace_buffer *buffer, unsigned long *kaslr_addr);
 
 /*
  * Because the ring buffer is generic, if other users of the ring buffer get
diff --git a/kernel/trace/ring_buffer.c b/kernel/trace/ring_buffer.c
index b8e0ae15ca5b..7146b780176f 100644
--- a/kernel/trace/ring_buffer.c
+++ b/kernel/trace/ring_buffer.c
@@ -31,6 +31,7 @@ 
 
 #include <asm/local64.h>
 #include <asm/local.h>
+#include <asm/setup.h>
 
 #include "trace.h"
 
@@ -49,8 +50,7 @@  static void update_pages_handler(struct work_struct *work);
 struct ring_buffer_meta {
 	int		magic;
 	int		struct_size;
-	unsigned long	text_addr;
-	unsigned long	data_addr;
+	unsigned long	kaslr_addr;
 	unsigned long	first_buffer;
 	unsigned long	head_buffer;
 	unsigned long	commit_buffer;
@@ -550,8 +550,7 @@  struct trace_buffer {
 	unsigned long			range_addr_start;
 	unsigned long			range_addr_end;
 
-	long				last_text_delta;
-	long				last_data_delta;
+	unsigned long			kaslr_addr;
 
 	unsigned int			subbuf_size;
 	unsigned int			subbuf_order;
@@ -1874,16 +1873,13 @@  static void rb_meta_validate_events(struct ring_buffer_per_cpu *cpu_buffer)
 	}
 }
 
-/* Used to calculate data delta */
-static char rb_data_ptr[] = "";
-
-#define THIS_TEXT_PTR		((unsigned long)rb_meta_init_text_addr)
-#define THIS_DATA_PTR		((unsigned long)rb_data_ptr)
-
 static void rb_meta_init_text_addr(struct ring_buffer_meta *meta)
 {
-	meta->text_addr = THIS_TEXT_PTR;
-	meta->data_addr = THIS_DATA_PTR;
+#ifdef CONFIG_RANDOMIZE_BASE
+	meta->kaslr_addr = kaslr_offset();
+#else
+	meta->kaslr_addr = 0;
+#endif
 }
 
 static void rb_range_meta_init(struct trace_buffer *buffer, int nr_pages)
@@ -1906,8 +1902,7 @@  static void rb_range_meta_init(struct trace_buffer *buffer, int nr_pages)
 			meta->first_buffer += delta;
 			meta->head_buffer += delta;
 			meta->commit_buffer += delta;
-			buffer->last_text_delta = THIS_TEXT_PTR - meta->text_addr;
-			buffer->last_data_delta = THIS_DATA_PTR - meta->data_addr;
+			buffer->kaslr_addr = meta->kaslr_addr;
 			continue;
 		}
 
@@ -2459,17 +2454,15 @@  struct trace_buffer *__ring_buffer_alloc_range(unsigned long size, unsigned flag
  *
  * Returns: The true if the delta is non zero
  */
-bool ring_buffer_last_boot_delta(struct trace_buffer *buffer, long *text,
-				 long *data)
+bool ring_buffer_last_boot_delta(struct trace_buffer *buffer, unsigned long *kaslr_addr)
 {
 	if (!buffer)
 		return false;
 
-	if (!buffer->last_text_delta)
+	if (!buffer->kaslr_addr)
 		return false;
 
-	*text = buffer->last_text_delta;
-	*data = buffer->last_data_delta;
+	*kaslr_addr = buffer->kaslr_addr;
 
 	return true;
 }
diff --git a/kernel/trace/trace.c b/kernel/trace/trace.c
index 1496a5ac33ae..a9e8eaf1d47e 100644
--- a/kernel/trace/trace.c
+++ b/kernel/trace/trace.c
@@ -50,7 +50,7 @@ 
 #include <linux/irq_work.h>
 #include <linux/workqueue.h>
 
-#include <asm/setup.h> /* COMMAND_LINE_SIZE */
+#include <asm/setup.h> /* COMMAND_LINE_SIZE and kaslr_offset() */
 
 #include "trace.h"
 #include "trace_output.h"
@@ -4193,7 +4193,7 @@  static enum print_line_t print_trace_fmt(struct trace_iterator *iter)
 		 * safe to use if the array has delta offsets
 		 * Force printing via the fields.
 		 */
-		if ((tr->text_delta || tr->data_delta) &&
+		if ((tr->text_delta) &&
 		    event->type > __TRACE_LAST_TYPE)
 			return print_event_fields(iter, event);
 
@@ -5996,7 +5996,7 @@  ssize_t tracing_resize_ring_buffer(struct trace_array *tr,
 
 static void update_last_data(struct trace_array *tr)
 {
-	if (!tr->text_delta && !tr->data_delta)
+	if (!(tr->flags & TRACE_ARRAY_FL_LAST_BOOT))
 		return;
 
 	/*
@@ -6009,7 +6009,8 @@  static void update_last_data(struct trace_array *tr)
 
 	/* Using current data now */
 	tr->text_delta = 0;
-	tr->data_delta = 0;
+
+	tr->flags &= ~TRACE_ARRAY_FL_LAST_BOOT;
 }
 
 /**
@@ -6827,8 +6828,17 @@  tracing_last_boot_read(struct file *filp, char __user *ubuf, size_t cnt, loff_t
 
 	seq_buf_init(&seq, buf, 64);
 
-	seq_buf_printf(&seq, "text delta:\t%ld\n", tr->text_delta);
-	seq_buf_printf(&seq, "data delta:\t%ld\n", tr->data_delta);
+	/*
+	 * Do not leak KASLR address. This only shows the KASLR address of
+	 * the last boot. When the ring buffer is started, the LAST_BOOT
+	 * flag gets cleared, and this should only report "current".
+	 * Otherwise it shows the KASLR address from the previous boot which
+	 * should not be the same as the current boot.
+	 */
+	if (!(tr->flags & TRACE_ARRAY_FL_LAST_BOOT))
+		seq_buf_puts(&seq, "Offset: current\n");
+	else
+		seq_buf_printf(&seq, "Offset: %lx\n", tr->kaslr_addr);
 
 	return simple_read_from_buffer(ubuf, cnt, ppos, buf, seq_buf_used(&seq));
 }
@@ -9212,8 +9222,10 @@  allocate_trace_buffer(struct trace_array *tr, struct array_buffer *buf, int size
 						      tr->range_addr_start,
 						      tr->range_addr_size);
 
-		ring_buffer_last_boot_delta(buf->buffer,
-					    &tr->text_delta, &tr->data_delta);
+#ifdef CONFIG_RANDOMIZE_BASE
+		if (ring_buffer_last_boot_delta(buf->buffer, &tr->kaslr_addr))
+			tr->text_delta = kaslr_offset() - tr->kaslr_addr;
+#endif
 		/*
 		 * This is basically the same as a mapped buffer,
 		 * with the same restrictions.
@@ -10461,7 +10473,7 @@  __init static void enable_instances(void)
 		 * to it.
 		 */
 		if (start) {
-			tr->flags |= TRACE_ARRAY_FL_BOOT;
+			tr->flags |= TRACE_ARRAY_FL_BOOT | TRACE_ARRAY_FL_LAST_BOOT;
 			tr->ref++;
 		}
 
diff --git a/kernel/trace/trace.h b/kernel/trace/trace.h
index 9c21ba45b7af..abe8169c3e87 100644
--- a/kernel/trace/trace.h
+++ b/kernel/trace/trace.h
@@ -348,8 +348,8 @@  struct trace_array {
 	unsigned int		mapped;
 	unsigned long		range_addr_start;
 	unsigned long		range_addr_size;
+	unsigned long		kaslr_addr;
 	long			text_delta;
-	long			data_delta;
 
 	struct trace_pid_list	__rcu *filtered_pids;
 	struct trace_pid_list	__rcu *filtered_no_pids;
@@ -433,9 +433,10 @@  struct trace_array {
 };
 
 enum {
-	TRACE_ARRAY_FL_GLOBAL	= BIT(0),
-	TRACE_ARRAY_FL_BOOT	= BIT(1),
-	TRACE_ARRAY_FL_MOD_INIT	= BIT(2),
+	TRACE_ARRAY_FL_GLOBAL		= BIT(0),
+	TRACE_ARRAY_FL_BOOT		= BIT(1),
+	TRACE_ARRAY_FL_LAST_BOOT	= BIT(2),
+	TRACE_ARRAY_FL_MOD_INIT		= BIT(3),
 };
 
 #ifdef CONFIG_MODULES