linux-kernel - Re: [PATCH v3 1/7 UPDATE] perf tools: Find vdso with the consider of cross-platform

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <573ADECF.1070408@huawei.com>
Date:	Tue, 17 May 2016 17:05:19 +0800
From:	Hekuang <hekuang@...wei.com>
To:	Adrian Hunter <adrian.hunter@...el.com>
CC:	<peterz@...radead.org>, <mingo@...hat.com>, <acme@...nel.org>,
	<alexander.shishkin@...ux.intel.com>, <jolsa@...hat.com>,
	<wangnan0@...wei.com>, <jpoimboe@...hat.com>, <ak@...ux.intel.com>,
	<eranian@...gle.com>, <namhyung@...nel.org>,
	<sukadev@...ux.vnet.ibm.com>, <masami.hiramatsu.pt@...achi.com>,
	<tumanova@...ux.vnet.ibm.com>, <kan.liang@...el.com>,
	<penberg@...nel.org>, <dsahern@...il.com>,
	<linux-kernel@...r.kernel.org>
Subject: Re: [PATCH v3 1/7 UPDATE] perf tools: Find vdso with the consider of
 cross-platform



在 2016/5/17 15:33, Adrian Hunter 写道:
> On 13/05/16 11:51, He Kuang wrote:
>> There's a problem in machine__findnew_vdso(), vdso buildid generated
>> by a 32-bit machine stores it with the name 'vdso', but when
>> processing buildid on a 64-bit machine with the same 'perf.data', perf
>> will search for vdso named as 'vdso32' and get failed.
>>
>> This patch tries to find the exsiting dsos in machine->dsos by thread
>> dso_type. 64-bit thread tries to find vdso with name 'vdso', because
>> all 64-bit vdso is named as that. 32-bit thread first tries to find
>> vdso with name 'vdso32' if this thread was run on 64-bit machine, if
>> failed, then it tries 'vdso' which indicates that the thread was run
>> on 32-bit machine when recording.
>>
>> Signed-off-by: He Kuang <hekuang@...wei.com>
>> ---
>>   tools/perf/util/vdso.c | 40 +++++++++++++++++++++++++++++++++++++---
>>   1 file changed, 37 insertions(+), 3 deletions(-)
>>
>> diff --git a/tools/perf/util/vdso.c b/tools/perf/util/vdso.c
>> index 44d440d..99f4a3d 100644
>> --- a/tools/perf/util/vdso.c
>> +++ b/tools/perf/util/vdso.c
>> @@ -134,8 +134,6 @@ static struct dso *__machine__addnew_vdso(struct machine *machine, const char *s
>>   	return dso;
>>   }
>>   
>> -#if BITS_PER_LONG == 64
>> -
>>   static enum dso_type machine__thread_dso_type(struct machine *machine,
>>   					      struct thread *thread)
>>   {
>> @@ -156,6 +154,8 @@ static enum dso_type machine__thread_dso_type(struct machine *machine,
>>   	return dso_type;
>>   }
>>   
>> +#if BITS_PER_LONG == 64
>> +
>>   static int vdso__do_copy_compat(FILE *f, int fd)
>>   {
>>   	char buf[4096];
>> @@ -283,8 +283,38 @@ static int __machine__findnew_vdso_compat(struct machine *machine,
>>   
>>   #endif
>>   
>> +static struct dso *machine__find_vdso(struct machine *machine,
>> +				      struct thread *thread)
>> +{
>> +	struct dso *dso = NULL;
>> +	enum dso_type dso_type;
>> +
>> +	dso_type = machine__thread_dso_type(machine, thread);
>> +	switch (dso_type) {
>> +	case DSO__TYPE_32BIT:
>> +		dso = __dsos__find(&machine->dsos, DSO__NAME_VDSO32, true);
>> +		if (!dso)
>> +			dso = __dsos__find(&machine->dsos, DSO__NAME_VDSO,
>> +					   true);
> So if we have not yet added the 32-bit vdso but have added the 64-bit vdso,
> we will return the wrong one.
>
> Can we check it? e.g.
>
> 		if (!dso)
> 			dso = __dsos__find(&machine->dsos, DSO__NAME_VDSO,
> 					   true);
> 			if (dso_type != dso__type(dso, machine))
> 				dso = NULL;
>
>> +		break;
>> +	case DSO__TYPE_X32BIT:
>> +		dso = __dsos__find(&machine->dsos, DSO__NAME_VDSOX32, true);
>> +		if (!dso)
>> +			dso = __dsos__find(&machine->dsos, DSO__NAME_VDSO,
>> +					   true);
> The x32 vdso is never called DSO__NAME_VDSO so this is not correct, but for
> the same reason we don't need this __dsos__find() anyway.

Thanks, update
>> +		break;
>> +	case DSO__TYPE_64BIT:
>> +	case DSO__TYPE_UNKNOWN:
>> +	default:
>> +		dso = __dsos__find(&machine->dsos, DSO__NAME_VDSO, true);
>> +		break;
>> +	}
>> +
>> +	return dso;
>> +}
>> +
>>   struct dso *machine__findnew_vdso(struct machine *machine,
>> -				  struct thread *thread __maybe_unused)
>> +				  struct thread *thread)
>>   {
>>   	struct vdso_info *vdso_info;
>>   	struct dso *dso = NULL;
>> @@ -297,6 +327,10 @@ struct dso *machine__findnew_vdso(struct machine *machine,
>>   	if (!vdso_info)
>>   		goto out_unlock;
>>   
>> +	dso = machine__find_vdso(machine, thread);
>> +	if (dso)
>> +		goto out_unlock;
>> +
>>   #if BITS_PER_LONG == 64
>>   	if (__machine__findnew_vdso_compat(machine, thread, vdso_info, &dso))
>>   		goto out_unlock;
>>
>