Skip to contentSkip to conversation

tracing: Fixes for 7.1

PATCH New Steven Rostedt 1 participant 3 messages May 21

Subsystems (2)
lore
Share & tools
lore
#0 lore raw

cover tracing: Fixes for 7.1

tracing fixes for v7.1:

- Avoid NULL return from hist_field_name()

The function hist_field_name() is directly passed to a strcat() which does not handle "NULL" characters. Return a zero length string when size is greater than the limit.

This is used only to output already created histograms and no field currently is greater than the limit. But it should still not return NULL.

- Do not call map->ops->elt_free() on allocation failure

When elt_alloc() fails, it should not call the map->ops->elt_free() function if it exists, as that function may not be able to handle the free on allocation failures. The ->elt_free() should only be called when elt_alloc() succeeds.

git://git.kernel.org/pub/scm/linux/kernel/git/trace/linux-trace.git
trace/fixes

Head SHA1: 8f0f5c4fb9df0e19a341e0c6ed8dc4fda9124f03

David Carlier (1):
tracing: Avoid NULL return from hist_field_name() on truncation

Masami Hiramatsu (Google) (1):

      tracing: Do not call map->ops->elt_free() if elt_alloc() fails

----

 kernel/trace/trace_events_hist.c |  6 ++----
 kernel/trace/tracing_map.c       | 17 +++++++++++++----
 2 files changed, 15 insertions(+), 8 deletions(-)
#1 lore raw

PATCH 1/2 tracing: Avoid NULL return from hist_field_name() on truncation

From: David Carlier <[email protected]>

hist_field_name() returns "" everywhere except the fully-qualified VAR_REF/EXPR case, where snprintf() truncation returns NULL early and bypasses the bottom NULL->"" guard. Callers don't expect NULL:
strcat(expr, hist_field_name(field, 0)) at trace_events_hist.c:1758 and the strcmp() in the sort-key match loop at :4804 both deref it.

system and event_name are bounded by MAX_EVENT_NAME_LEN, but the field name on a VAR_REF is kstrdup'd from a histogram variable name parsed out of the trigger string and has no length cap, so a long enough var name in a fully qualified reference can reach the truncation path.

Keep the length check but leave field_name as "" on overflow.

Fixes 5ec1d1e97de1 ("tracing: Rebuild full_name on each hist_field_name() call")
---
 kernel/trace/trace_events_hist.c | 6 ++----
 1 file changed, 2 insertions(+), 4 deletions(-)
kernel/trace/trace_events_hist.c +2 -4
index 0dbbf6cca9bc..eb2c2bc8bc3d 100644
@@ -1369,10 +1369,8 @@ static const char *hist_field_name(struct hist_field *field,
 			len = snprintf(full_name, sizeof(full_name), fmt,
 				       field->system, field->event_name,
 				       field->name);
-			if (len >= sizeof(full_name))
-				return NULL;
-
-			field_name = full_name;
+			if (len < sizeof(full_name))
+				field_name = full_name;
 		} else
 			field_name = field->name;
 	} else if (field->flags & HIST_FIELD_FL_TIMESTAMP)
Signature · 1 line
2.53.0
#2 lore raw

PATCH 2/2 tracing: Do not call map->ops->elt_free() if elt_alloc() fails

In reply to Steven Rostedt

From: "Masami Hiramatsu (Google)" <[email protected]>

In paths where tracing_map_elt_alloc() failed to allocate objects, the map->ops->elt_alloc() call was never successful. In this case, map->ops->elt_free() should not be called.

Cc Tom Zanussi <[email protected]>
Cc Mathieu Desnoyers <[email protected]>
Cc Rosen Penev <[email protected]>
Fixes 2734b629525a ("tracing: Add per-element variable support to tracing_map")
---
 kernel/trace/tracing_map.c | 17 +++++++++++++----
 1 file changed, 13 insertions(+), 4 deletions(-)
kernel/trace/tracing_map.c +13 -4
index bf1a507695b6..0dd7927df22a 100644
@@ -386,13 +386,11 @@ static void tracing_map_elt_init_fields(struct tracing_map_elt *elt)
 	}
 }
 
-static void tracing_map_elt_free(struct tracing_map_elt *elt)
+static void __tracing_map_elt_free(struct tracing_map_elt *elt)
 {
 	if (!elt)
 		return;
 
-	if (elt->map->ops && elt->map->ops->elt_free)
-		elt->map->ops->elt_free(elt);
 	kfree(elt->fields);
 	kfree(elt->vars);
 	kfree(elt->var_set);
@@ -400,6 +398,17 @@ static void tracing_map_elt_free(struct tracing_map_elt *elt)
 	kfree(elt);
 }
 
+static void tracing_map_elt_free(struct tracing_map_elt *elt)
+{
+	if (!elt)
+		return;
+
+	/* Only objects initialized with alloc_elt() should be passed to free_elt().*/
+	if (elt->map->ops && elt->map->ops->elt_free)
+		elt->map->ops->elt_free(elt);
+	__tracing_map_elt_free(elt);
+}
+
 static struct tracing_map_elt *tracing_map_elt_alloc(struct tracing_map *map)
 {
 	struct tracing_map_elt *elt;
@@ -444,7 +453,7 @@ static struct tracing_map_elt *tracing_map_elt_alloc(struct tracing_map *map)
 	}
 	return elt;
  free:
-	tracing_map_elt_free(elt);
+	__tracing_map_elt_free(elt);
 
 	return ERR_PTR(err);
 }
Signature · 1 line
2.53.0