Re: [RFC] [Patch 1/1] [Self Ptrace] System call notification with self_ptrace

[Pierre Morel <pmorel@linux.vnet.ibm.com>]

Hi,

Dave Hansen wrote:
> On Mon, 2008-08-25 at 09:34 +0200, Pierre Morel wrote:
>   
>> +       if ((current->ptrace & PT_SELF)
>> +               && (regs->orig_ax != __NR_rt_sigreturn)
>> +               && (regs->orig_ax != __NR_ptrace)) {
>> +               if (!entryexit) {
>> +               struct siginfo info;
>> +
>> +               memset(&info, 0, sizeof(struct siginfo));
>> +               info.si_signo = SIGSYS;
>> +               info.si_code = SYS_SYSCALL;
>> +               info.si_addr = (void *) regs->orig_ax;
>> +               send_sig_info(SIGSYS, &info, current);
>> +               }
>> +               return 1; /* Skip system call, deliver signal. */
>> +       }
>>     
>
> The indenting here looks messed up.
>   
You are right, I will rework the patch and send it again, it has a lot of
formating errors indeed.
> Also, there looks to be a pretty substantial amount of copy-and-paste
> code in those little if()s.  It's only going to get worse as we add more
> architectures.  If there's ever a little buglet in that bit of code, or
> we need to tweak it it some way, it'll be a bitch to fix.
>
> For instance, if you have a little arch-independent helper like this:
>
> static inline int is_self_ptracing(unsigned long syscall_reg)
> {
> 	if (!(current->ptrace & PT_SELF))
> 		return 0;
> 	if (syscall_reg == __NR_rt_sigreturn)
> 		return 0;
> 	if (syscall_reg == __NR_ptrace)
> 		return 0;
> 	return 1;
> }
>
> You can call it like this:
>
> 	if (is_self_ptracing(regs->gprs[2]))
> ...
> 	if (is_self_ptracing(regs->orig_ax))
> ...
> 	if (is_self_ptracing(regs->orig_rax))
>
> Something similar can probably be done for the siginfo construction.
>   
Yes, thank you it is a good tip.
> You should basically try and think of ways to abstract this stuff every
> single time you touch arch code.  
>
> Why don't you also mention why you really want this feature.  That's
> missing from the description. 
>   
Yes, you are right too, I will rework the patch description too.
> -- Dave
>
>   
Thanks for the comments, I rework the patch.

Pierre

-- 
=============
Pierre Morel
RTOS and Embedded Linux