linux-kernel - Re: [PATCH] Identify which executable object the userspace address belongs to. Store thread group leader id, and use it to lookup the address in the process's map. We could have looked up the address on thread's map, but the thread might not exist by the time we are called. The process might not exist either, but if you are reading trace

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: <490EB361.9090007@gmail.com>
Date:	Mon, 03 Nov 2008 10:16:33 +0200
From:	Török Edwin <edwintorok@...il.com>
To:	Ingo Molnar <mingo@...e.hu>
CC:	srostedt@...hat.com, a.p.zijlstra@...llo.nl, sandmann@...mi.au.dk,
	linux-kernel@...r.kernel.org
Subject: Re: [PATCH] Identify which executable object the userspace address
 belongs to. Store thread group leader id, and use it to lookup the address
 in the process's map. We could have looked up the address on thread's map,
 but the thread might not exist by the time we are called. The process might
 not exist either, but if you are reading trace_pipe, that is unlikely.

On 2008-11-03 09:47, Ingo Molnar wrote:
> * Török Edwin <edwintorok@...il.com> wrote:
>
>   
>> +		struct task_struct *task;
>> +		/* we do the lookup on the thread group leader,
>> +		 * since individual threads might have already quit! */
>>     
>
> please use the customary comment style:
>
>   /*
>    * Comment .....
>    * ...... goes here:
>    */
>
>   

. Can checkpatch.pl warn me of situations like this?

>> -		if (ret /*&& (sym_flags & TRACE_ITER_SYM_ADDR)*/)
>> +		if (mm) {
>> +			const struct vm_area_struct *vma = find_vma(mm, ip);
>> +			if (vma) {
>> +				file = vma->vm_file;
>> +				vmstart = vma->vm_start;
>> +			}
>> +		}
>> +		if (file) {
>> +			ret = trace_seq_path(s, &file->f_path);
>> +			if (ret)
>> +				ret = trace_seq_printf(s, "[+0x%lx]",
>> +						ip - vmstart);
>> +		}
>> +		if (ret && ((sym_flags & TRACE_ITER_SYM_ADDR) || !file))
>>  			ret = trace_seq_printf(s, " <" IP_FMT ">", ip);
>>     
>
> the function is getting a bit large - would it make sense to split out 
> this section into a helper inline function?
>   

Yes, I'll do that.

> another small nit: i cleaned up the subject line and the commit log 
> message for you - see the two patches below - could you please keep it 
> for future (v3) submissions of these patches? It's just small 
> formatting changes.
>   

Ok, luckily git-rebase -i allows me to rewrite the log message too ;)

> Your patches are a nice feature we want to have nevertheless - to be 
> able to see where a user-space app is running has been one of the 
> historically weak points of kernel instrumentation.

Thanks.
It currently works for x86 only, but architecture porters can add
support for theirs quite easily, it just needs to modeled after how
oprofile does it for example.
BTW would it make sense to change oprofile and the sysprof tracer to use
save_stack_trace_user? It would eliminate some code duplication.

Would it make sense to add a script that post-processes the output to
scripts/tracing?
It would parse a trace log (from trace or latency_trace) and use
addr2line to resolve the address to source:line, and if successful
replace the relative address with that; and also
group identical stack traces together.

Best regards,
--Edwin
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/