All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH 0/2] A couple hist trigger fixes
@ 2016-04-25 19:01 Tom Zanussi
  2016-04-25 19:01 ` [PATCH 1/2] tracing: Add check for NULL event field when creating hist field Tom Zanussi
                   ` (2 more replies)
  0 siblings, 3 replies; 4+ messages in thread
From: Tom Zanussi @ 2016-04-25 19:01 UTC (permalink / raw)
  To: rostedt; +Cc: dan.carpenter, linux-kernel, Tom Zanussi

These are a couple of small fixes for problems pointed out by Dan
Carpenter.

patch 1, pointed out by Smatch though not actually a bug, cleans up
the code to explicitly check for something that was implied.  Thanks
to Steve Rostedt for suggesting the fix.

patch 2 fixes a PTR_ERR problem similar to the one previously
reported by Dan.

The following changes since commit d50c744ecde7ee3ba4d7ffb0e1c55e7a2f6bbc8e:

  tracing: Fix unsigned comparison to zero in hist trigger code (2016-04-19 18:56:05 -0400)

are available in the git repository at:

  git://git.yoctoproject.org/linux-yocto-contrib.git tzanussi/hist-trigger-err-fixes
  http://git.yoctoproject.org/cgit/cgit.cgi/linux-yocto-contrib/log/?h=tzanussi/hist-trigger-err-fixes

Tom Zanussi (2):
  tracing: Add check for NULL event field when creating hist field
  tracing: Handle tracing_map_alloc_elts() error path correctly

 kernel/trace/trace_events_hist.c | 3 +++
 kernel/trace/tracing_map.c       | 8 ++++++--
 2 files changed, 9 insertions(+), 2 deletions(-)

-- 
1.9.3

^ permalink raw reply	[flat|nested] 4+ messages in thread

* [PATCH 1/2] tracing: Add check for NULL event field when creating hist field
  2016-04-25 19:01 [PATCH 0/2] A couple hist trigger fixes Tom Zanussi
@ 2016-04-25 19:01 ` Tom Zanussi
  2016-04-25 19:01 ` [PATCH 2/2] tracing: Handle tracing_map_alloc_elts() error path correctly Tom Zanussi
  2016-04-26 13:41 ` [PATCH 0/2] A couple hist trigger fixes Steven Rostedt
  2 siblings, 0 replies; 4+ messages in thread
From: Tom Zanussi @ 2016-04-25 19:01 UTC (permalink / raw)
  To: rostedt; +Cc: dan.carpenter, linux-kernel, Tom Zanussi

Smatch flagged create_hist_field() as possibly being able to
dereference a NULL pointer, although the current code exits in all
cases where the event field could be NULL, so it's not actually a
problem.

Still, to prevent future changes to the code from overlooking new
cases, make the NULL pointer check explicit and warn once in that
case.

Signed-off-by: Tom Zanussi <tom.zanussi@linux.intel.com>
---
 kernel/trace/trace_events_hist.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/kernel/trace/trace_events_hist.c b/kernel/trace/trace_events_hist.c
index f98b6b3..0c05b8a 100644
--- a/kernel/trace/trace_events_hist.c
+++ b/kernel/trace/trace_events_hist.c
@@ -371,6 +371,9 @@ static struct hist_field *create_hist_field(struct ftrace_event_field *field,
 		goto out;
 	}
 
+	if (WARN_ON_ONCE(!field))
+		goto out;
+
 	if (is_string_field(field)) {
 		flags |= HIST_FIELD_FL_STRING;
 
-- 
1.9.3

^ permalink raw reply related	[flat|nested] 4+ messages in thread

* [PATCH 2/2] tracing: Handle tracing_map_alloc_elts() error path correctly
  2016-04-25 19:01 [PATCH 0/2] A couple hist trigger fixes Tom Zanussi
  2016-04-25 19:01 ` [PATCH 1/2] tracing: Add check for NULL event field when creating hist field Tom Zanussi
@ 2016-04-25 19:01 ` Tom Zanussi
  2016-04-26 13:41 ` [PATCH 0/2] A couple hist trigger fixes Steven Rostedt
  2 siblings, 0 replies; 4+ messages in thread
From: Tom Zanussi @ 2016-04-25 19:01 UTC (permalink / raw)
  To: rostedt; +Cc: dan.carpenter, linux-kernel, Tom Zanussi

If tracing_map_elt_alloc() fails, it will return ERR_PTR() instead of
NULL, so change the check to IS_ERROR().  We also need to set the
failed entry in the map->elts array to NULL instead of ERR_PTR() so
tracing_map_free_elts() doesn't try freeing an ERR_PTR().

tracing_map_free_elts() should also zero out what it frees so a
reentrant call won't find previously freed elements.

Signed-off-by: Tom Zanussi <tom.zanussi@linux.intel.com>
---
 kernel/trace/tracing_map.c | 8 ++++++--
 1 file changed, 6 insertions(+), 2 deletions(-)

diff --git a/kernel/trace/tracing_map.c b/kernel/trace/tracing_map.c
index e0f1729..979f6d6 100644
--- a/kernel/trace/tracing_map.c
+++ b/kernel/trace/tracing_map.c
@@ -366,10 +366,13 @@ static void tracing_map_free_elts(struct tracing_map *map)
 	if (!map->elts)
 		return;
 
-	for (i = 0; i < map->max_elts; i++)
+	for (i = 0; i < map->max_elts; i++) {
 		tracing_map_elt_free(*(TRACING_MAP_ELT(map->elts, i)));
+		*(TRACING_MAP_ELT(map->elts, i)) = NULL;
+	}
 
 	tracing_map_array_free(map->elts);
+	map->elts = NULL;
 }
 
 static int tracing_map_alloc_elts(struct tracing_map *map)
@@ -383,7 +386,8 @@ static int tracing_map_alloc_elts(struct tracing_map *map)
 
 	for (i = 0; i < map->max_elts; i++) {
 		*(TRACING_MAP_ELT(map->elts, i)) = tracing_map_elt_alloc(map);
-		if (!(*(TRACING_MAP_ELT(map->elts, i)))) {
+		if (IS_ERR(*(TRACING_MAP_ELT(map->elts, i)))) {
+			*(TRACING_MAP_ELT(map->elts, i)) = NULL;
 			tracing_map_free_elts(map);
 
 			return -ENOMEM;
-- 
1.9.3

^ permalink raw reply related	[flat|nested] 4+ messages in thread

* Re: [PATCH 0/2] A couple hist trigger fixes
  2016-04-25 19:01 [PATCH 0/2] A couple hist trigger fixes Tom Zanussi
  2016-04-25 19:01 ` [PATCH 1/2] tracing: Add check for NULL event field when creating hist field Tom Zanussi
  2016-04-25 19:01 ` [PATCH 2/2] tracing: Handle tracing_map_alloc_elts() error path correctly Tom Zanussi
@ 2016-04-26 13:41 ` Steven Rostedt
  2 siblings, 0 replies; 4+ messages in thread
From: Steven Rostedt @ 2016-04-26 13:41 UTC (permalink / raw)
  To: Tom Zanussi; +Cc: dan.carpenter, linux-kernel

On Mon, 25 Apr 2016 14:01:26 -0500
Tom Zanussi <tom.zanussi@linux.intel.com> wrote:

> These are a couple of small fixes for problems pointed out by Dan
> Carpenter.
> 
> patch 1, pointed out by Smatch though not actually a bug, cleans up
> the code to explicitly check for something that was implied.  Thanks
> to Steve Rostedt for suggesting the fix.
> 
> patch 2 fixes a PTR_ERR problem similar to the one previously
> reported by Dan.
> 
> The following changes since commit d50c744ecde7ee3ba4d7ffb0e1c55e7a2f6bbc8e:
> 
>   tracing: Fix unsigned comparison to zero in hist trigger code (2016-04-19 18:56:05 -0400)
> 
> are available in the git repository at:
> 
>   git://git.yoctoproject.org/linux-yocto-contrib.git tzanussi/hist-trigger-err-fixes
>   http://git.yoctoproject.org/cgit/cgit.cgi/linux-yocto-contrib/log/?h=tzanussi/hist-trigger-err-fixes
> 
> Tom Zanussi (2):
>   tracing: Add check for NULL event field when creating hist field
>   tracing: Handle tracing_map_alloc_elts() error path correctly
> 
>  kernel/trace/trace_events_hist.c | 3 +++
>  kernel/trace/tracing_map.c       | 8 ++++++--
>  2 files changed, 9 insertions(+), 2 deletions(-)
> 

Applied.

Thanks Tom,

-- Steve

^ permalink raw reply	[flat|nested] 4+ messages in thread

end of thread, other threads:[~2016-04-26 13:41 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2016-04-25 19:01 [PATCH 0/2] A couple hist trigger fixes Tom Zanussi
2016-04-25 19:01 ` [PATCH 1/2] tracing: Add check for NULL event field when creating hist field Tom Zanussi
2016-04-25 19:01 ` [PATCH 2/2] tracing: Handle tracing_map_alloc_elts() error path correctly Tom Zanussi
2016-04-26 13:41 ` [PATCH 0/2] A couple hist trigger fixes Steven Rostedt

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.