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

[Pierre Morel]

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

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