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]
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/

Powered by blists - more mailing lists