lkml.org 
[lkml]   [2019]   [May]   [7]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
SubjectRe: [PATCH] fix use-after-free in perf_sched__lat
From
Date
Hi Jiri,
Thanks for your reply.

On 2019/5/7 16:51, Jiri Olsa wrote:
> On Fri, May 03, 2019 at 10:35:55AM +0800, Wei Li wrote:
>> After thread is added to machine->threads[i].dead in
>> __machine__remove_thread, the machine->threads[i].dead is freed
>> when calling free(session) in perf_session__delete(). So it get a
>> Segmentation fault when accessing it in thread__put().
>>
>> In this patch, we delay the perf_session__delete until all threads
>> have been deleted.
>>
>> This can be reproduced by following steps:
>> ulimit -c unlimited
>> export MALLOC_MMAP_THRESHOLD_=0
>
> what's this for?

When we set env "MALLOC_MMAP_THRESHOLD_=0",the memory-allocation functions employ
mmap instead of increasing the program break using sbrk what may be maintained
with cache. Thus it can report "Segmentation fault" immediately when going into
this use-after-free code.

>
>> perf sched record sleep 10
>> perf sched latency --sort max
>> Segmentation fault (core dumped)
>>
>> Signed-off-by: Zhipeng Xie <xiezhipeng1@huawei.com>
>> Signed-off-by: Wei Li <liwei391@huawei.com>
>> ---
>> tools/perf/builtin-sched.c | 44 ++++++++++++++++++++++++++++++++++++--
>> 1 file changed, 42 insertions(+), 2 deletions(-)
>>
>> diff --git a/tools/perf/builtin-sched.c b/tools/perf/builtin-sched.c
>> index cbf39dab19c1..17849ae2eb1e 100644
>> --- a/tools/perf/builtin-sched.c
>> +++ b/tools/perf/builtin-sched.c
>> @@ -3130,11 +3130,48 @@ static void perf_sched__merge_lat(struct perf_sched *sched)
>> static int perf_sched__lat(struct perf_sched *sched)
>> {
>> struct rb_node *next;
>> + const struct perf_evsel_str_handler handlers[] = {
>> + { "sched:sched_switch", process_sched_switch_event, },
>> + { "sched:sched_stat_runtime", process_sched_runtime_event, },
>> + { "sched:sched_wakeup", process_sched_wakeup_event, },
>> + { "sched:sched_wakeup_new", process_sched_wakeup_event, },
>> + { "sched:sched_migrate_task", process_sched_migrate_task_event, },
>> + };
>> + struct perf_session *session;
>> + struct perf_data data = {
>> + .file = {
>> + .path = input_name,
>> + },
>
> I can't compile this:
>
> builtin-sched.c: In function ‘perf_sched__lat’:
> builtin-sched.c:3144:12: error: initialization discards ‘const’ qualifier from pointer target type [-Werror=discarded-qualifiers]
> .path = input_name,
>
Sorry, this place has been changed recently in mainline code, i will update to
the latest code.

>
>> + .mode = PERF_DATA_MODE_READ,
>> + .force = sched->force,
>> + };
>> + int rc = -1;
>>
>> setup_pager();
>>
>> - if (perf_sched__read_events(sched))
>
> so it's basically perf_sched__read_events code in here now, right?
>
> might be better to add __perf_sched__read_events function
> that would take session argument, something like:
>
> session = perf_session__new(&data, false, &sched->tool);
> ...
> __perf_sched__read_events(sched, session)
> ...
> perf_session__delete(session);
>
> to avoid the code ducplication

Yes, what you suggest is reasonable, i will send a v2 with your suggestion soon.

Thanks,
Wei

\
 
 \ /
  Last update: 2019-05-08 05:47    [W:0.035 / U:0.944 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site