* [PATCH] perf top: add concurrent access protection of the SLsmg screen management
@ 2021-12-29 8:55 yaowenbin
2021-12-31 14:10 ` Jiri Olsa
2022-01-02 14:47 ` Arnaldo Carvalho de Melo
0 siblings, 2 replies; 4+ messages in thread
From: yaowenbin @ 2021-12-29 8:55 UTC (permalink / raw)
To: peterz, mingo, acme, mark.rutland, alexander.shishkin, jolsa,
namhyung, yaowenbin1, linux-kernel
Cc: hewenliang4, wuxu.wu
When the following command is executed several times, a coredump file is generated.
$ timeout -k 9 5 perf top -e task-clock
*******
*******
*******
0.01% [kernel] [k] __do_softirq
0.01% libpthread-2.28.so [.] __pthread_mutex_lock
0.01% [kernel] [k] __ll_sc_atomic64_sub_return
double free or corruption (!prev) perf top --sort comm,dso
timeout: the monitored command dumped core
When we terminate "perf top" using sending signal method, SLsmg_reset_smg
function is called. SLsmg_reset_smg resets the SLsmg screen management routines
by freeing all memory allocated while it was active. However SLsmg_reinit_smg
function maybe be called by another thread. SLsmg_reinit_smg function will
free the same memory accessed by SLsmg_reset_smg functon, thus it results
in double free. SLsmg_reinit_smg function is called already protected by
ui__lock, so we fix the problem by adding pthread_mutex_trylock of ui__lock
when calling SLsmg_reset_smg function.
Signed-off-by: yaowenbin <yaowenbin1@huawei.com>
Signed-off-by: hewenliang <hewenliang4@huawei.com>
Signed-off-by: Wenyu Liu <liuwenyu7@huawei.com>
---
tools/perf/ui/tui/setup.c | 8 +++++---
1 file changed, 5 insertions(+), 3 deletions(-)
diff --git a/tools/perf/ui/tui/setup.c b/tools/perf/ui/tui/setup.c
index d4ac41679..1fdf92062 100644
--- a/tools/perf/ui/tui/setup.c
+++ b/tools/perf/ui/tui/setup.c
@@ -170,9 +170,11 @@ void ui__exit(bool wait_for_ok)
"Press any key...", 0);
SLtt_set_cursor_visibility(1);
- SLsmg_refresh();
- SLsmg_reset_smg();
+ if (!pthread_mutex_trylock(&ui__lock)) {
+ SLsmg_refresh();
+ SLsmg_reset_smg();
+ pthread_mutex_unlock(&ui__lock);
+ }
SLang_reset_tty();
-
perf_error__unregister(&perf_tui_eops);
}
--
2.27.0
^ permalink raw reply related [flat|nested] 4+ messages in thread
* Re: [PATCH] perf top: add concurrent access protection of the SLsmg screen management
2021-12-29 8:55 [PATCH] perf top: add concurrent access protection of the SLsmg screen management yaowenbin
@ 2021-12-31 14:10 ` Jiri Olsa
2022-01-04 7:21 ` yaowenbin
2022-01-02 14:47 ` Arnaldo Carvalho de Melo
1 sibling, 1 reply; 4+ messages in thread
From: Jiri Olsa @ 2021-12-31 14:10 UTC (permalink / raw)
To: yaowenbin
Cc: peterz, mingo, acme, mark.rutland, alexander.shishkin, namhyung,
linux-kernel, hewenliang4, wuxu.wu
On Wed, Dec 29, 2021 at 04:55:19PM +0800, yaowenbin wrote:
> When the following command is executed several times, a coredump file is generated.
> $ timeout -k 9 5 perf top -e task-clock
> *******
> *******
> *******
> 0.01% [kernel] [k] __do_softirq
> 0.01% libpthread-2.28.so [.] __pthread_mutex_lock
> 0.01% [kernel] [k] __ll_sc_atomic64_sub_return
> double free or corruption (!prev) perf top --sort comm,dso
> timeout: the monitored command dumped core
>
> When we terminate "perf top" using sending signal method, SLsmg_reset_smg
> function is called. SLsmg_reset_smg resets the SLsmg screen management routines
> by freeing all memory allocated while it was active. However SLsmg_reinit_smg
> function maybe be called by another thread. SLsmg_reinit_smg function will
> free the same memory accessed by SLsmg_reset_smg functon, thus it results
> in double free. SLsmg_reinit_smg function is called already protected by
> ui__lock, so we fix the problem by adding pthread_mutex_trylock of ui__lock
> when calling SLsmg_reset_smg function.
>
> Signed-off-by: yaowenbin <yaowenbin1@huawei.com>
> Signed-off-by: hewenliang <hewenliang4@huawei.com>
> Signed-off-by: Wenyu Liu <liuwenyu7@huawei.com>
> ---
> tools/perf/ui/tui/setup.c | 8 +++++---
> 1 file changed, 5 insertions(+), 3 deletions(-)
>
> diff --git a/tools/perf/ui/tui/setup.c b/tools/perf/ui/tui/setup.c
> index d4ac41679..1fdf92062 100644
> --- a/tools/perf/ui/tui/setup.c
> +++ b/tools/perf/ui/tui/setup.c
> @@ -170,9 +170,11 @@ void ui__exit(bool wait_for_ok)
> "Press any key...", 0);
>
> SLtt_set_cursor_visibility(1);
> - SLsmg_refresh();
> - SLsmg_reset_smg();
> + if (!pthread_mutex_trylock(&ui__lock)) {
trylock because it can be called from signal, right?
could you plase add some comment about that
thanks,
jirka
> + SLsmg_refresh();
> + SLsmg_reset_smg();
> + pthread_mutex_unlock(&ui__lock);
> + }
> SLang_reset_tty();
> -
> perf_error__unregister(&perf_tui_eops);
> }
> --
> 2.27.0
>
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH] perf top: add concurrent access protection of the SLsmg screen management
2021-12-29 8:55 [PATCH] perf top: add concurrent access protection of the SLsmg screen management yaowenbin
2021-12-31 14:10 ` Jiri Olsa
@ 2022-01-02 14:47 ` Arnaldo Carvalho de Melo
1 sibling, 0 replies; 4+ messages in thread
From: Arnaldo Carvalho de Melo @ 2022-01-02 14:47 UTC (permalink / raw)
To: yaowenbin
Cc: peterz, mingo, mark.rutland, alexander.shishkin, jolsa, namhyung,
linux-kernel, hewenliang4, wuxu.wu
Em Wed, Dec 29, 2021 at 04:55:19PM +0800, yaowenbin escreveu:
> When the following command is executed several times, a coredump file is generated.
> $ timeout -k 9 5 perf top -e task-clock
> *******
> *******
> *******
> 0.01% [kernel] [k] __do_softirq
> 0.01% libpthread-2.28.so [.] __pthread_mutex_lock
> 0.01% [kernel] [k] __ll_sc_atomic64_sub_return
> double free or corruption (!prev) perf top --sort comm,dso
> timeout: the monitored command dumped core
Thanks, tested, reproduced the problem without the patch, not managed to
reproduce in 50 calls to the reproducer after applying the patch.
- Arnaldo
> When we terminate "perf top" using sending signal method, SLsmg_reset_smg
> function is called. SLsmg_reset_smg resets the SLsmg screen management routines
> by freeing all memory allocated while it was active. However SLsmg_reinit_smg
> function maybe be called by another thread. SLsmg_reinit_smg function will
> free the same memory accessed by SLsmg_reset_smg functon, thus it results
> in double free. SLsmg_reinit_smg function is called already protected by
> ui__lock, so we fix the problem by adding pthread_mutex_trylock of ui__lock
> when calling SLsmg_reset_smg function.
>
> Signed-off-by: yaowenbin <yaowenbin1@huawei.com>
> Signed-off-by: hewenliang <hewenliang4@huawei.com>
> Signed-off-by: Wenyu Liu <liuwenyu7@huawei.com>
> ---
> tools/perf/ui/tui/setup.c | 8 +++++---
> 1 file changed, 5 insertions(+), 3 deletions(-)
>
> diff --git a/tools/perf/ui/tui/setup.c b/tools/perf/ui/tui/setup.c
> index d4ac41679..1fdf92062 100644
> --- a/tools/perf/ui/tui/setup.c
> +++ b/tools/perf/ui/tui/setup.c
> @@ -170,9 +170,11 @@ void ui__exit(bool wait_for_ok)
> "Press any key...", 0);
>
> SLtt_set_cursor_visibility(1);
> - SLsmg_refresh();
> - SLsmg_reset_smg();
> + if (!pthread_mutex_trylock(&ui__lock)) {
> + SLsmg_refresh();
> + SLsmg_reset_smg();
> + pthread_mutex_unlock(&ui__lock);
> + }
> SLang_reset_tty();
> -
> perf_error__unregister(&perf_tui_eops);
> }
> --
> 2.27.0
--
- Arnaldo
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH] perf top: add concurrent access protection of the SLsmg screen management
2021-12-31 14:10 ` Jiri Olsa
@ 2022-01-04 7:21 ` yaowenbin
0 siblings, 0 replies; 4+ messages in thread
From: yaowenbin @ 2022-01-04 7:21 UTC (permalink / raw)
To: Jiri Olsa
Cc: peterz, mingo, acme, mark.rutland, alexander.shishkin, namhyung,
linux-kernel, hewenliang4, wuxu.wu
In ui__refresh_dimensions function, ui__lock will also be acquired, so we use trylock
in the signal handler ui__exit to avoid deadlock.
Thanks.
在 2021/12/31 22:10, Jiri Olsa 写道:
> On Wed, Dec 29, 2021 at 04:55:19PM +0800, yaowenbin wrote:
>> When the following command is executed several times, a coredump file is generated.
>> $ timeout -k 9 5 perf top -e task-clock
>> *******
>> *******
>> *******
>> 0.01% [kernel] [k] __do_softirq
>> 0.01% libpthread-2.28.so [.] __pthread_mutex_lock
>> 0.01% [kernel] [k] __ll_sc_atomic64_sub_return
>> double free or corruption (!prev) perf top --sort comm,dso
>> timeout: the monitored command dumped core
>>
>> When we terminate "perf top" using sending signal method, SLsmg_reset_smg
>> function is called. SLsmg_reset_smg resets the SLsmg screen management routines
>> by freeing all memory allocated while it was active. However SLsmg_reinit_smg
>> function maybe be called by another thread. SLsmg_reinit_smg function will
>> free the same memory accessed by SLsmg_reset_smg functon, thus it results
>> in double free. SLsmg_reinit_smg function is called already protected by
>> ui__lock, so we fix the problem by adding pthread_mutex_trylock of ui__lock
>> when calling SLsmg_reset_smg function.
>>
>> Signed-off-by: yaowenbin <yaowenbin1@huawei.com>
>> Signed-off-by: hewenliang <hewenliang4@huawei.com>
>> Signed-off-by: Wenyu Liu <liuwenyu7@huawei.com>
>> ---
>> tools/perf/ui/tui/setup.c | 8 +++++---
>> 1 file changed, 5 insertions(+), 3 deletions(-)
>>
>> diff --git a/tools/perf/ui/tui/setup.c b/tools/perf/ui/tui/setup.c
>> index d4ac41679..1fdf92062 100644
>> --- a/tools/perf/ui/tui/setup.c
>> +++ b/tools/perf/ui/tui/setup.c
>> @@ -170,9 +170,11 @@ void ui__exit(bool wait_for_ok)
>> "Press any key...", 0);
>>
>> SLtt_set_cursor_visibility(1);
>> - SLsmg_refresh();
>> - SLsmg_reset_smg();
>> + if (!pthread_mutex_trylock(&ui__lock)) {
>
> trylock because it can be called from signal, right?
> could you plase add some comment about that
>
> thanks,
> jirka
>
>> + SLsmg_refresh();
>> + SLsmg_reset_smg();
>> + pthread_mutex_unlock(&ui__lock);
>> + }
>> SLang_reset_tty();
>> -
>> perf_error__unregister(&perf_tui_eops);
>> }
>> --
>> 2.27.0
>>
>
> .
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2022-01-04 7:21 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-12-29 8:55 [PATCH] perf top: add concurrent access protection of the SLsmg screen management yaowenbin
2021-12-31 14:10 ` Jiri Olsa
2022-01-04 7:21 ` yaowenbin
2022-01-02 14:47 ` Arnaldo Carvalho de Melo
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.