[PATCH] tracing/synthetic: Handle dynamic string read faults

[PATCH] tracing/synthetic: Handle dynamic string read faults

[Lee Jones]

Commit 0934ae9977c2 ("tracing: Fix reading strings from synthetic
events") switched synthetic event dynamic strings to use
fetch_store_strlen() and fetch_store_string(), relying on commit
2e9906f84fc7 ("tracing: Add "(fault)" name injection to kernel probes")
to reserve and store FAULT_STRING when a string address faults.

When commit 4ed8f337dee3 ("Revert "tracing: Add "(fault)" name injection
to kernel probes"") and commit 797311bce5c2 ("tracing/probes: Fix to
record 0-length data_loc in fetch_store_string*() if fails") later
reverted FAULT_STRING injection in favour of recording a 0-length
data_loc on fault, synthetic events were left unchanged.

Consequently, a faulting kernel string address causes get_field_size()
to add a negative error code (-EFAULT) to fields_size, under-reserving
the trace buffer entry, while trace_string() casts the negative return
value to an unsigned length and passes it to fetch_store_string() and
write_synth_entry(), corrupting the ring buffer.

Skip non-positive lengths in get_field_size(), record a 0-length
dynamic field in trace_string() when fetching a dynamic string fails,
and print FAULT_STRING in print_synth_event() when a dynamic string has
zero length, matching trace_probe.

Fixes: 4ed8f337dee3 ("Revert "tracing: Add "(fault)" name injection to kernel probes"")
Cc: stable@vger.kernel.org
Signed-off-by: Lee Jones <lee@kernel.org>
---
 kernel/trace/trace_events_synth.c | 19 ++++++++++++++++---
 1 file changed, 16 insertions(+), 3 deletions(-)

diff --git a/kernel/trace/trace_events_synth.c b/kernel/trace/trace_events_synth.c
index dc15658a887c..7506e2e1f64a 100644
--- a/kernel/trace/trace_events_synth.c
+++ b/kernel/trace/trace_events_synth.c
@@ -376,7 +376,9 @@ static enum print_line_t print_synth_event(struct trace_iterator *iter,
 				union trace_synth_field *data = &entry->fields[n_u64];
 
 				trace_seq_printf(s, print_fmt, se->fields[i]->name,
-						 (char *)entry + data->as_dynamic.offset,
+						 data->as_dynamic.len ?
+						 (char *)entry + data->as_dynamic.offset :
+						 FAULT_STRING,
 						 i == se->n_fields - 1 ? "" : " ");
 				n_u64++;
 			} else {
@@ -438,11 +440,20 @@ static unsigned int trace_string(struct synth_trace_event *entry,
 	if (is_dynamic) {
 		union trace_synth_field *data = &entry->fields[*n_u64];
 
-		len = fetch_store_strlen((unsigned long)str_val);
 		data->as_dynamic.offset = struct_size(entry, fields, event->n_u64) + data_size;
-		data->as_dynamic.len = len;
+
+		ret = fetch_store_strlen((unsigned long)str_val);
+		if (ret <= 0) {
+			data->as_dynamic.len = 0;
+			(*n_u64)++;
+			return 0;
+		}
+
+		data->as_dynamic.len = ret;
 
 		ret = fetch_store_string((unsigned long)str_val, &entry->fields[*n_u64], entry);
+		if (ret > 0)
+			len = ret;
 
 		(*n_u64)++;
 	} else {
@@ -522,6 +533,8 @@ static __always_inline int get_field_size(struct synth_event *event,
 			len *= sizeof(unsigned long);
 		} else {
 			len = fetch_store_strlen((unsigned long)str_val);
+			if (len <= 0)
+				continue;
 		}
 
 		fields_size += len;
-- 
2.56.0.385.gd3acb90ef8-goog