All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH 1/2] Make _FORTIFY_SOURCE defines dependent on the feature
@ 2019-09-25 19:59 Ian Rogers
  2019-09-25 19:59 ` [PATCH 2/2] Avoid raising segv using an obvious null dereference Ian Rogers
                   ` (2 more replies)
  0 siblings, 3 replies; 6+ messages in thread
From: Ian Rogers @ 2019-09-25 19:59 UTC (permalink / raw)
  To: Peter Zijlstra, Ingo Molnar, Arnaldo Carvalho de Melo,
	Alexander Shishkin, Jiri Olsa, Namhyung Kim, Andi Kleen,
	linux-kernel
  Cc: Stephane Eranian, Ian Rogers

Unconditionally defining _FORTIFY_SOURCE can break tools that don't work
with it, such as memory sanitizers:
https://github.com/google/sanitizers/wiki/AddressSanitizer#faq

Signed-off-by: Ian Rogers <irogers@google.com>
---
 tools/lib/subcmd/Makefile | 8 +++++++-
 1 file changed, 7 insertions(+), 1 deletion(-)

diff --git a/tools/lib/subcmd/Makefile b/tools/lib/subcmd/Makefile
index ed61fb3a46c0..5b2cd5e58df0 100644
--- a/tools/lib/subcmd/Makefile
+++ b/tools/lib/subcmd/Makefile
@@ -20,7 +20,13 @@ MAKEFLAGS += --no-print-directory
 LIBFILE = $(OUTPUT)libsubcmd.a
 
 CFLAGS := $(EXTRA_WARNINGS) $(EXTRA_CFLAGS)
-CFLAGS += -ggdb3 -Wall -Wextra -std=gnu99 -U_FORTIFY_SOURCE -D_FORTIFY_SOURCE=2 -fPIC
+CFLAGS += -ggdb3 -Wall -Wextra -std=gnu99 -fPIC
+
+ifeq ($(DEBUG),0)
+  ifeq ($(feature-fortify-source), 1)
+    CFLAGS += -U_FORTIFY_SOURCE -D_FORTIFY_SOURCE=2
+  endif
+endif
 
 ifeq ($(CC_NO_CLANG), 0)
   CFLAGS += -O3
-- 
2.23.0.351.gc4317032e6-goog


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

* [PATCH 2/2] Avoid raising segv using an obvious null dereference
  2019-09-25 19:59 [PATCH 1/2] Make _FORTIFY_SOURCE defines dependent on the feature Ian Rogers
@ 2019-09-25 19:59 ` Ian Rogers
  2019-09-26 15:24   ` Arnaldo Carvalho de Melo
  2019-10-07 14:49   ` [tip: perf/urgent] perf tests: Avoid raising SEGV using an obvious NULL dereference tip-bot2 for Ian Rogers
  2019-09-26 15:18 ` [PATCH 1/2] Make _FORTIFY_SOURCE defines dependent on the feature Arnaldo Carvalho de Melo
  2019-10-07 14:49 ` [tip: perf/urgent] libsubcmd: " tip-bot2 for Ian Rogers
  2 siblings, 2 replies; 6+ messages in thread
From: Ian Rogers @ 2019-09-25 19:59 UTC (permalink / raw)
  To: Peter Zijlstra, Ingo Molnar, Arnaldo Carvalho de Melo,
	Alexander Shishkin, Jiri Olsa, Namhyung Kim, Andi Kleen,
	linux-kernel
  Cc: Stephane Eranian, Ian Rogers

An optimized build such as:
make -C tools/perf CLANG=1 CC=clang EXTRA_CFLAGS="-O3
will turn the dereference operation into a ud2 instruction, raising a SIGILL
rather than a SIGSEGV. Use raise(..) for correctness and clarity.

Similar issues were addressed in Numfor Mbiziwo-Tiapo's patch:
https://lkml.org/lkml/2019/7/8/1234

Signed-off-by: Ian Rogers <irogers@google.com>
---
 tools/perf/tests/perf-hooks.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/tools/perf/tests/perf-hooks.c b/tools/perf/tests/perf-hooks.c
index dbc27199c65e..dd865e0bea12 100644
--- a/tools/perf/tests/perf-hooks.c
+++ b/tools/perf/tests/perf-hooks.c
@@ -19,12 +19,11 @@ static void sigsegv_handler(int sig __maybe_unused)
 static void the_hook(void *_hook_flags)
 {
 	int *hook_flags = _hook_flags;
-	int *p = NULL;
 
 	*hook_flags = 1234;
 
 	/* Generate a segfault, test perf_hooks__recover */
-	*p = 0;
+	raise(SIGSEGV);
 }
 
 int test__perf_hooks(struct test *test __maybe_unused, int subtest __maybe_unused)
-- 
2.23.0.351.gc4317032e6-goog


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

* Re: [PATCH 1/2] Make _FORTIFY_SOURCE defines dependent on the feature
  2019-09-25 19:59 [PATCH 1/2] Make _FORTIFY_SOURCE defines dependent on the feature Ian Rogers
  2019-09-25 19:59 ` [PATCH 2/2] Avoid raising segv using an obvious null dereference Ian Rogers
@ 2019-09-26 15:18 ` Arnaldo Carvalho de Melo
  2019-10-07 14:49 ` [tip: perf/urgent] libsubcmd: " tip-bot2 for Ian Rogers
  2 siblings, 0 replies; 6+ messages in thread
From: Arnaldo Carvalho de Melo @ 2019-09-26 15:18 UTC (permalink / raw)
  To: Ian Rogers
  Cc: Peter Zijlstra, Ingo Molnar, Alexander Shishkin, Jiri Olsa,
	Namhyung Kim, Andi Kleen, linux-kernel, Stephane Eranian,
	Josh Poimboeuf

Em Wed, Sep 25, 2019 at 12:59:23PM -0700, Ian Rogers escreveu:
> Unconditionally defining _FORTIFY_SOURCE can break tools that don't work
> with it, such as memory sanitizers:
> https://github.com/google/sanitizers/wiki/AddressSanitizer#faq

Thanks, and added this:

Cc: Josh Poimboeuf <jpoimboe@redhat.com>
Fixes: 4b6ab94eabe4 ("perf subcmd: Create subcmd library")
 
> Signed-off-by: Ian Rogers <irogers@google.com>
> ---
>  tools/lib/subcmd/Makefile | 8 +++++++-
>  1 file changed, 7 insertions(+), 1 deletion(-)
> 
> diff --git a/tools/lib/subcmd/Makefile b/tools/lib/subcmd/Makefile
> index ed61fb3a46c0..5b2cd5e58df0 100644
> --- a/tools/lib/subcmd/Makefile
> +++ b/tools/lib/subcmd/Makefile
> @@ -20,7 +20,13 @@ MAKEFLAGS += --no-print-directory
>  LIBFILE = $(OUTPUT)libsubcmd.a
>  
>  CFLAGS := $(EXTRA_WARNINGS) $(EXTRA_CFLAGS)
> -CFLAGS += -ggdb3 -Wall -Wextra -std=gnu99 -U_FORTIFY_SOURCE -D_FORTIFY_SOURCE=2 -fPIC
> +CFLAGS += -ggdb3 -Wall -Wextra -std=gnu99 -fPIC
> +
> +ifeq ($(DEBUG),0)
> +  ifeq ($(feature-fortify-source), 1)
> +    CFLAGS += -U_FORTIFY_SOURCE -D_FORTIFY_SOURCE=2
> +  endif
> +endif
>  
>  ifeq ($(CC_NO_CLANG), 0)
>    CFLAGS += -O3
> -- 
> 2.23.0.351.gc4317032e6-goog

-- 

- Arnaldo

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

* Re: [PATCH 2/2] Avoid raising segv using an obvious null dereference
  2019-09-25 19:59 ` [PATCH 2/2] Avoid raising segv using an obvious null dereference Ian Rogers
@ 2019-09-26 15:24   ` Arnaldo Carvalho de Melo
  2019-10-07 14:49   ` [tip: perf/urgent] perf tests: Avoid raising SEGV using an obvious NULL dereference tip-bot2 for Ian Rogers
  1 sibling, 0 replies; 6+ messages in thread
From: Arnaldo Carvalho de Melo @ 2019-09-26 15:24 UTC (permalink / raw)
  To: Ian Rogers
  Cc: Peter Zijlstra, Ingo Molnar, Alexander Shishkin, Jiri Olsa,
	Namhyung Kim, Andi Kleen, linux-kernel, Stephane Eranian

Em Wed, Sep 25, 2019 at 12:59:24PM -0700, Ian Rogers escreveu:
> An optimized build such as:
> make -C tools/perf CLANG=1 CC=clang EXTRA_CFLAGS="-O3
> will turn the dereference operation into a ud2 instruction, raising a SIGILL
> rather than a SIGSEGV. Use raise(..) for correctness and clarity.
> 
> Similar issues were addressed in Numfor Mbiziwo-Tiapo's patch:
> https://lkml.org/lkml/2019/7/8/1234

Added:

Cc: Numfor Mbiziwo-Tiapo <nums@google.com>

And:

Cc: Wang Nan <wangnan0@huawei.com>
Fixes: a074865e60ed ("perf tools: Introduce perf hooks")

And:

Committer testing:

Before:

  [root@quaco ~]# perf test hooks
  55: perf hooks                                            : Ok
  [root@quaco ~]# perf test -v hooks
  55: perf hooks                                            :
  --- start ---
  test child forked, pid 17092
  SIGSEGV is observed as expected, try to recover.
  Fatal error (SEGFAULT) in perf hook 'test'
  test child finished with 0
  ---- end ----
  perf hooks: Ok
  [root@quaco ~]# 

After:

  [root@quaco ~]# perf test hooks
  55: perf hooks                                            : Ok
  [root@quaco ~]# perf test -v hooks
  55: perf hooks                                            :
  --- start ---
  test child forked, pid 17909
  SIGSEGV is observed as expected, try to recover.
  Fatal error (SEGFAULT) in perf hook 'test'
  test child finished with 0
  ---- end ----
  perf hooks: Ok
  [root@quaco ~]#


 ----------------------------------------------------------------------------

Thanks, applied.

- Arnaldo
 
> Signed-off-by: Ian Rogers <irogers@google.com>
> ---
>  tools/perf/tests/perf-hooks.c | 3 +--
>  1 file changed, 1 insertion(+), 2 deletions(-)
> 
> diff --git a/tools/perf/tests/perf-hooks.c b/tools/perf/tests/perf-hooks.c
> index dbc27199c65e..dd865e0bea12 100644
> --- a/tools/perf/tests/perf-hooks.c
> +++ b/tools/perf/tests/perf-hooks.c
> @@ -19,12 +19,11 @@ static void sigsegv_handler(int sig __maybe_unused)
>  static void the_hook(void *_hook_flags)
>  {
>  	int *hook_flags = _hook_flags;
> -	int *p = NULL;
>  
>  	*hook_flags = 1234;
>  
>  	/* Generate a segfault, test perf_hooks__recover */
> -	*p = 0;
> +	raise(SIGSEGV);
>  }
>  
>  int test__perf_hooks(struct test *test __maybe_unused, int subtest __maybe_unused)
> -- 
> 2.23.0.351.gc4317032e6-goog

-- 

- Arnaldo

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

* [tip: perf/urgent] perf tests: Avoid raising SEGV using an obvious NULL dereference
  2019-09-25 19:59 ` [PATCH 2/2] Avoid raising segv using an obvious null dereference Ian Rogers
  2019-09-26 15:24   ` Arnaldo Carvalho de Melo
@ 2019-10-07 14:49   ` tip-bot2 for Ian Rogers
  1 sibling, 0 replies; 6+ messages in thread
From: tip-bot2 for Ian Rogers @ 2019-10-07 14:49 UTC (permalink / raw)
  To: linux-tip-commits
  Cc: Ian Rogers, Arnaldo Carvalho de Melo, Alexander Shishkin,
	Andi Kleen, Jiri Olsa, Namhyung Kim, Peter Zijlstra,
	Stephane Eranian, Wang Nan, Ingo Molnar, Borislav Petkov,
	linux-kernel

The following commit has been merged into the perf/urgent branch of tip:

Commit-ID:     e3e2cf3d5b1fe800b032e14c0fdcd9a6fb20cf3b
Gitweb:        https://git.kernel.org/tip/e3e2cf3d5b1fe800b032e14c0fdcd9a6fb20cf3b
Author:        Ian Rogers <irogers@google.com>
AuthorDate:    Wed, 25 Sep 2019 12:59:24 -07:00
Committer:     Arnaldo Carvalho de Melo <acme@redhat.com>
CommitterDate: Fri, 27 Sep 2019 09:26:14 -03:00

perf tests: Avoid raising SEGV using an obvious NULL dereference

An optimized build such as:

  make -C tools/perf CLANG=1 CC=clang EXTRA_CFLAGS="-O3

will turn the dereference operation into a ud2 instruction, raising a
SIGILL rather than a SIGSEGV. Use raise(..) for correctness and clarity.

Similar issues were addressed in Numfor Mbiziwo-Tiapo's patch:

  https://lkml.org/lkml/2019/7/8/1234

Committer testing:

Before:

  [root@quaco ~]# perf test hooks
  55: perf hooks                                            : Ok
  [root@quaco ~]# perf test -v hooks
  55: perf hooks                                            :
  --- start ---
  test child forked, pid 17092
  SIGSEGV is observed as expected, try to recover.
  Fatal error (SEGFAULT) in perf hook 'test'
  test child finished with 0
  ---- end ----
  perf hooks: Ok
  [root@quaco ~]#

After:

  [root@quaco ~]# perf test hooks
  55: perf hooks                                            : Ok
  [root@quaco ~]# perf test -v hooks
  55: perf hooks                                            :
  --- start ---
  test child forked, pid 17909
  SIGSEGV is observed as expected, try to recover.
  Fatal error (SEGFAULT) in perf hook 'test'
  test child finished with 0
  ---- end ----
  perf hooks: Ok
  [root@quaco ~]#

Fixes: a074865e60ed ("perf tools: Introduce perf hooks")
Signed-off-by: Ian Rogers <irogers@google.com>
Tested-by: Arnaldo Carvalho de Melo <acme@redhat.com>
Cc: Alexander Shishkin <alexander.shishkin@linux.intel.com>
Cc: Andi Kleen <ak@linux.intel.com>
Cc: Jiri Olsa <jolsa@redhat.com>
Cc: Namhyung Kim <namhyung@kernel.org>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Stephane Eranian <eranian@google.com>
Cc: Wang Nan <wangnan0@huawei.com>
Link: http://lore.kernel.org/lkml/20190925195924.152834-2-irogers@google.com
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
---
 tools/perf/tests/perf-hooks.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/tools/perf/tests/perf-hooks.c b/tools/perf/tests/perf-hooks.c
index dbc2719..dd865e0 100644
--- a/tools/perf/tests/perf-hooks.c
+++ b/tools/perf/tests/perf-hooks.c
@@ -19,12 +19,11 @@ static void sigsegv_handler(int sig __maybe_unused)
 static void the_hook(void *_hook_flags)
 {
 	int *hook_flags = _hook_flags;
-	int *p = NULL;
 
 	*hook_flags = 1234;
 
 	/* Generate a segfault, test perf_hooks__recover */
-	*p = 0;
+	raise(SIGSEGV);
 }
 
 int test__perf_hooks(struct test *test __maybe_unused, int subtest __maybe_unused)

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

* [tip: perf/urgent] libsubcmd: Make _FORTIFY_SOURCE defines dependent on the feature
  2019-09-25 19:59 [PATCH 1/2] Make _FORTIFY_SOURCE defines dependent on the feature Ian Rogers
  2019-09-25 19:59 ` [PATCH 2/2] Avoid raising segv using an obvious null dereference Ian Rogers
  2019-09-26 15:18 ` [PATCH 1/2] Make _FORTIFY_SOURCE defines dependent on the feature Arnaldo Carvalho de Melo
@ 2019-10-07 14:49 ` tip-bot2 for Ian Rogers
  2 siblings, 0 replies; 6+ messages in thread
From: tip-bot2 for Ian Rogers @ 2019-10-07 14:49 UTC (permalink / raw)
  To: linux-tip-commits
  Cc: Ian Rogers, Alexander Shishkin, Andi Kleen, Jiri Olsa,
	Josh Poimboeuf, Namhyung Kim, Peter Zijlstra, Stephane Eranian,
	Arnaldo Carvalho de Melo, Ingo Molnar, Borislav Petkov,
	linux-kernel

The following commit has been merged into the perf/urgent branch of tip:

Commit-ID:     4b0b2b096da9d296e0e5668cdfba8613bd6f5bc8
Gitweb:        https://git.kernel.org/tip/4b0b2b096da9d296e0e5668cdfba8613bd6f5bc8
Author:        Ian Rogers <irogers@google.com>
AuthorDate:    Wed, 25 Sep 2019 12:59:23 -07:00
Committer:     Arnaldo Carvalho de Melo <acme@redhat.com>
CommitterDate: Fri, 27 Sep 2019 09:26:14 -03:00

libsubcmd: Make _FORTIFY_SOURCE defines dependent on the feature

Unconditionally defining _FORTIFY_SOURCE can break tools that don't work
with it, such as memory sanitizers:

  https://github.com/google/sanitizers/wiki/AddressSanitizer#faq

Fixes: 4b6ab94eabe4 ("perf subcmd: Create subcmd library")
Signed-off-by: Ian Rogers <irogers@google.com>
Cc: Alexander Shishkin <alexander.shishkin@linux.intel.com>
Cc: Andi Kleen <ak@linux.intel.com>
Cc: Jiri Olsa <jolsa@redhat.com>
Cc: Josh Poimboeuf <jpoimboe@redhat.com>
Cc: Namhyung Kim <namhyung@kernel.org>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Stephane Eranian <eranian@google.com>
Link: http://lore.kernel.org/lkml/20190925195924.152834-1-irogers@google.com
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
---
 tools/lib/subcmd/Makefile | 8 +++++++-
 1 file changed, 7 insertions(+), 1 deletion(-)

diff --git a/tools/lib/subcmd/Makefile b/tools/lib/subcmd/Makefile
index ed61fb3..5b2cd5e 100644
--- a/tools/lib/subcmd/Makefile
+++ b/tools/lib/subcmd/Makefile
@@ -20,7 +20,13 @@ MAKEFLAGS += --no-print-directory
 LIBFILE = $(OUTPUT)libsubcmd.a
 
 CFLAGS := $(EXTRA_WARNINGS) $(EXTRA_CFLAGS)
-CFLAGS += -ggdb3 -Wall -Wextra -std=gnu99 -U_FORTIFY_SOURCE -D_FORTIFY_SOURCE=2 -fPIC
+CFLAGS += -ggdb3 -Wall -Wextra -std=gnu99 -fPIC
+
+ifeq ($(DEBUG),0)
+  ifeq ($(feature-fortify-source), 1)
+    CFLAGS += -U_FORTIFY_SOURCE -D_FORTIFY_SOURCE=2
+  endif
+endif
 
 ifeq ($(CC_NO_CLANG), 0)
   CFLAGS += -O3

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

end of thread, other threads:[~2019-10-07 14:50 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-09-25 19:59 [PATCH 1/2] Make _FORTIFY_SOURCE defines dependent on the feature Ian Rogers
2019-09-25 19:59 ` [PATCH 2/2] Avoid raising segv using an obvious null dereference Ian Rogers
2019-09-26 15:24   ` Arnaldo Carvalho de Melo
2019-10-07 14:49   ` [tip: perf/urgent] perf tests: Avoid raising SEGV using an obvious NULL dereference tip-bot2 for Ian Rogers
2019-09-26 15:18 ` [PATCH 1/2] Make _FORTIFY_SOURCE defines dependent on the feature Arnaldo Carvalho de Melo
2019-10-07 14:49 ` [tip: perf/urgent] libsubcmd: " tip-bot2 for Ian Rogers

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.