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

Pierre Morel
2008-08-26T12:36:54Z

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