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:	Thu, 2 Feb 2012 12:00:30 +0100
From:	"Indan Zupancic" <indan@....nu>
To:	"Takuo Koguchi" <takuo.koguchi.sw@...achi.com>
Cc:	linux-kernel@...r.kernel.org, masami.hiramatsu.pt@...achi.com,
	linux@....linux.org.uk, rostedt@...dmis.org, fweisbec@...il.com,
	mingo@...hat.com, jbaron@...hat.com, yrl.pp-manager.tt@...achi.com,
	mcgrathr@...gle.com
Subject: Re: [PATCH] ARM: Wire up HAVE_SYSCALL_TRACEPOINTS

On Thu, February 2, 2012 10:21, Takuo Koguchi wrote:
> Hello,
> I am glad to start getting responses.
> (2012/02/01 10:47), Indan Zupancic wrote:
>> Hello,
>>
>> CC'ing Roland.
>>
>> On Thu, December 1, 2011 12:01, takuo.koguchi.sw@...achi.com wrote:
>>> diff --git a/arch/arm/Kconfig b/arch/arm/Kconfig
>>> index 44789ef..84181b3 100644
>>> --- a/arch/arm/Kconfig
>>> +++ b/arch/arm/Kconfig
>>> @@ -13,6 +13,7 @@ config ARM
>>>   	select HAVE_KPROBES if !XIP_KERNEL
>>>   	select HAVE_KRETPROBES if (HAVE_KPROBES)
>>>   	select HAVE_FUNCTION_TRACER if (!XIP_KERNEL)
>>> +	select HAVE_SYSCALL_TRACEPOINTS
>> Your patch totally ignores OABI, it should at least depend on CONFIG_AEABI
>> and on !CONFIG_OABI_COMPAT.
> Right.  As Russel King suggested, this patch depends on those configs
> until very large NR_syscalls is properly handled by ftrace.

It has nothing to do with large NR_syscalls. Supporting OABI is hard,
e.g. it doesn't put the syscall nr in r7, it's encoded as part of the
syscall instruction. Also the ABI for some system calls is different,
with different arg layouts (alignment of 64 bit args is different).

>
>>>   	select HAVE_FTRACE_MCOUNT_RECORD if (!XIP_KERNEL)
>>>   	select HAVE_DYNAMIC_FTRACE if (!XIP_KERNEL)
>>>   	select HAVE_FUNCTION_GRAPH_TRACER if (!THUMB2_KERNEL)
>>> diff --git a/arch/arm/include/asm/syscall.h b/arch/arm/include/asm/syscall.h
>>> new file mode 100644
>>> index 0000000..cabeb67
>>> --- /dev/null
>>> +++ b/arch/arm/include/asm/syscall.h
>>> @@ -0,0 +1,45 @@
>>> +#ifndef _ASM_ARM_SYSCALL_H
>>> +#define _ASM_ARM_SYSCALL_H
>>> +
>>> +extern const unsigned long sys_call_table[];
>>> +
>>> +#include<linux/sched.h>
>>> +
>>> +static inline long syscall_get_nr(struct task_struct *task,
>>> +				  struct pt_regs *regs)
>>> +{
>>> +	return regs->ARM_r7;
>>> +}
>>> +
>>> +static inline long syscall_get_return_value(struct task_struct *task,
>>> +					    struct pt_regs *regs)
>>> +{
>>> +	return regs->ARM_r0;
>>> +}
>>> +
>>> +static inline void syscall_get_arguments(struct task_struct *task,
>>> +					 struct pt_regs *regs,
>>> +					 unsigned int i, unsigned int n,
>>> +					 unsigned long *args)
>>> +{
>>> +	BUG_ON(i);
>> This is unacceptable, that would make this function useless.
>>
>> See Rolands patch:
>>
>> https://lkml.org/lkml/2009/4/24/399 for a better implementation.
> Thanks.  My apology for the lack of investigation.

I got both links from Will Drewry, I'm just jumping in and passing it on.

>>> +#endif	/* _ASM_ARM_SYSCALL_H */
>>> diff --git a/arch/arm/include/asm/thread_info.h b/arch/arm/include/asm/thread_info.h
>>> index 7b5cc8d..2509028 100644
>>> --- a/arch/arm/include/asm/thread_info.h
>>> +++ b/arch/arm/include/asm/thread_info.h
>>> @@ -139,6 +139,7 @@ extern void vfp_flush_hwstate(struct thread_info *);
>>>   #define TIF_NEED_RESCHED	1
>>>   #define TIF_NOTIFY_RESUME	2	/* callback before returning to user */
>>>   #define TIF_SYSCALL_TRACE	8
>>> +#define TIF_SYSCALL_TRACEPOINT	15
>>>   #define TIF_POLLING_NRFLAG	16
>>>   #define TIF_USING_IWMMXT	17
>>>   #define TIF_MEMDIE		18	/* is terminating due to OOM killer */
>>> @@ -150,11 +151,13 @@ extern void vfp_flush_hwstate(struct thread_info *);
>>>   #define _TIF_NEED_RESCHED	(1<<  TIF_NEED_RESCHED)
>>>   #define _TIF_NOTIFY_RESUME	(1<<  TIF_NOTIFY_RESUME)
>>>   #define _TIF_SYSCALL_TRACE	(1<<  TIF_SYSCALL_TRACE)
>>> +#define _TIF_SYSCALL_TRACEPOINT	(1<<  TIF_SYSCALL_TRACEPOINT)
>>>   #define _TIF_POLLING_NRFLAG	(1<<  TIF_POLLING_NRFLAG)
>>>   #define _TIF_USING_IWMMXT	(1<<  TIF_USING_IWMMXT)
>>>   #define _TIF_FREEZE		(1<<  TIF_FREEZE)
>>>   #define _TIF_RESTORE_SIGMASK	(1<<  TIF_RESTORE_SIGMASK)
>>>   #define _TIF_SECCOMP		(1<<  TIF_SECCOMP)
>>> +#define _TIF_SYSCALL_T_OR_A	(_TIF_SYSCALL_TRACE | _TIF_SYSCALL_TRACEPOINT)
>> What does 'A' stand for?
> 'A' originally stands for AUDIT.  I should have used a better name as
> there is no _TIF_SYSCALL_AUDIT for ARM.
> Probably it is better to define _TIF_WORK_SYSCALL_ENTRY and
> _TIF_WORK_SYSCALL_EXIT properly.

That would be much better.

>>>   /*
>>>    * Change these and you break ASM code in entry-common.S
>>> diff --git a/arch/arm/include/asm/unistd.h b/arch/arm/include/asm/unistd.h
>>> index 4a11237..f4eac2d 100644
>>> --- a/arch/arm/include/asm/unistd.h
>>> +++ b/arch/arm/include/asm/unistd.h
>>> @@ -405,6 +405,9 @@
>>>   #define __NR_process_vm_readv		(__NR_SYSCALL_BASE+376)
>>>   #define __NR_process_vm_writev		(__NR_SYSCALL_BASE+377)
>>>
>>> +#ifndef __ASSEMBLY__
>>> +#define NR_syscalls		378
>>> +#endif
>>>   /*
>>>    * The following SWIs are ARM private.
>>>    */
>>> diff --git a/arch/arm/kernel/entry-common.S b/arch/arm/kernel/entry-common.S
>>> index b2a27b6..a1577c2 100644
>>> --- a/arch/arm/kernel/entry-common.S
>>> +++ b/arch/arm/kernel/entry-common.S
>>> @@ -87,7 +87,7 @@ ENTRY(ret_from_fork)
>>>   	get_thread_info tsk
>>>   	ldr	r1, [tsk, #TI_FLAGS]		@ check for syscall tracing
>>>   	mov	why, #1
>>> -	tst	r1, #_TIF_SYSCALL_TRACE		@ are we tracing syscalls?
>>> +	tst	r1, #_TIF_SYSCALL_T_OR_A	@ are we tracing syscalls?
>>>   	beq	ret_slow_syscall
>>>   	mov	r1, sp
>>>   	mov	r0, #1				@ trace exit [IP = 1]
>>> @@ -443,7 +443,7 @@ ENTRY(vector_swi)
>>>   1:
>>>   #endif
>>>
>>> -	tst	r10, #_TIF_SYSCALL_TRACE		@ are we tracing syscalls?
>>> +	tst	r10, #_TIF_SYSCALL_T_OR_A	@ are we tracing syscalls?
>>>   	bne	__sys_trace
>>>
>>>   	cmp	scno, #NR_syscalls		@ check upper syscall limit
>>> diff --git a/arch/arm/kernel/ptrace.c b/arch/arm/kernel/ptrace.c
>>> index 483727a..a690c9f 100644
>>> --- a/arch/arm/kernel/ptrace.c
>>> +++ b/arch/arm/kernel/ptrace.c
>>> @@ -28,6 +28,9 @@
>>>   #include<asm/system.h>
>>>   #include<asm/traps.h>
>>>
>>> +#define CREATE_TRACE_POINTS
>>> +#include<trace/events/syscalls.h>
>>> +
>>>   #define REG_PC	15
>>>   #define REG_PSR	16
>>>   /*
>>> @@ -906,6 +909,13 @@ asmlinkage int syscall_trace(int why, struct pt_regs *regs, int
>>> scno)
>>>   {
>>>   	unsigned long ip;
>> Not used, you get the same info from 'why'.
> Sorry. But I do not understand what you suggest here.  This is the
> existing code.

Sorry, I missed that.

>>
>>> +	if (unlikely(test_thread_flag(TIF_SYSCALL_TRACEPOINT))) {
>>> +		if (why)
>>> +			trace_sys_exit(regs, regs->ARM_r0);
>>> +		else
>>> +			trace_sys_enter(regs, scno);
>>> +	}
>>> +
>>>   	if (!test_thread_flag(TIF_SYSCALL_TRACE))
>>>   		return scno;
>>>   	if (!(current->ptrace&  PT_PTRACED))
>>
>
> Thanks,
>
> Takuo
>

Greetings,

Indan


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

Powered by Openwall GNU/*/Linux Powered by OpenVZ