* [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 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
* 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* [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