public inbox for linux-kernel@vger.kernel.org
 help / color / mirror / Atom feed
From: Dave Hansen <dave@linux.vnet.ibm.com>
To: Pierre Morel <pmorel@linux.vnet.ibm.com>
Cc: Andrew Morton <akpm@linux-foundation.org>,
	linux-kernel@vger.kernel.org, Oleg Nesterov <oleg@tv-sign.ru>,
	Roland McGrath <roland@redhat.com>,
	Heiko Carstens <heicars2@linux.vnet.ibm.com>,
	sameske@linux.vnet.ibm.com,
	Martin Schwidefsky <schwidefsky@de.ibm.com>
Subject: Re: [RFC] [Patch 1/1] [Self Ptrace] System call notification with self_ptrace
Date: Mon, 25 Aug 2008 09:33:12 -0700	[thread overview]
Message-ID: <1219681992.20559.196.camel@nimitz> (raw)
In-Reply-To: <48B26083.8080506@linux.vnet.ibm.com>

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.

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.

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. 

-- Dave


  reply	other threads:[~2008-08-25 16:33 UTC|newest]

Thread overview: 11+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2008-08-25  7:34 [RFC] [Patch 1/1] [Self Ptrace] System call notification with self_ptrace Pierre Morel
2008-08-25 16:33 ` Dave Hansen [this message]
2008-08-26 12:33   ` Pierre Morel
2008-08-25 16:54 ` Oleg Nesterov
2008-08-26 14:04   ` Pierre Morel
2008-08-26 16:27     ` Oleg Nesterov
2008-08-27 14:32       ` Pierre Morel
2008-08-27 16:24         ` Oleg Nesterov
2008-08-28 12:03           ` Pierre Morel
2008-08-28 12:32             ` Oleg Nesterov
2008-08-28 13:24               ` Pierre Morel

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=1219681992.20559.196.camel@nimitz \
    --to=dave@linux.vnet.ibm.com \
    --cc=akpm@linux-foundation.org \
    --cc=heicars2@linux.vnet.ibm.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=oleg@tv-sign.ru \
    --cc=pmorel@linux.vnet.ibm.com \
    --cc=roland@redhat.com \
    --cc=sameske@linux.vnet.ibm.com \
    --cc=schwidefsky@de.ibm.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox