[PATCH] more robustness against ptrace errors

[Dmitry V. Levin]

On Fri, Feb 13, 2009 at 11:37:07AM +0100, Denys Vlasenko wrote:
[...]
> Please review.

Sorry for the long delay.
The change looks good indeed.  See my comments below, though.

> +	if (do_ptrace(PTRACE_SET_SYSCALL, tcp, NULL, new, "SET_SYSCALL") != 0)

The "SET_SYSCALL" is deducable from PTRACE_SET_SYSCALL number, isn't it?
Could we avoid this redundancy?

>  long
> -do_ptrace(int request, struct tcb *tcp, void *addr, void *data)
> +do_ptrace(int request, struct tcb *tcp, void *addr, long data, const char *msg)
>  {
> +	int err;
>  	long l;
>  
>  	errno = 0;
>  	l = ptrace(request, tcp->pid, addr, data);
> -	/* Non-ESRCH errors might be our invalid reg/mem accesses,
> -	 * we do not record them. */
> -	if (errno == ESRCH)
> -		tcp->ptrace_errno = ESRCH;
> +	err = errno;
> +	if (err) {
> +		tcp->ptrace_errno = err;
> +		if (err != ESRCH) {
> +			fprintf(stderr, "strace: ptrace(PTRACE_%s,%u,%p,%lu): %s\n",
> +				msg, (int) tcp->pid, addr, data, strerror(err));
> +		}
> +		return -1;
> +	}
> +	return l;
> +}

fprintf() may clobber errno, which is tested after do_ptrace() return
several times in the code, so we have to either reset errno back to the
value set by ptrace() call, or change *all* do_ptrace() callers to check
its return value instead of errno.

> +static long
> +do_ptrace_peekdata(struct tcb *tcp, void *addr, int started)
> +{
> +	int err;
> +	long l;
> +
> +	errno = 0;
> +	l = ptrace(PTRACE_PEEKDATA, tcp->pid, addr, 0);
> +	err = errno;
> +	if (err) {
> +		if (started && (err == EPERM || err == EIO)) {
> +			/* Ran into 'end of memory' - not an error */
> +			return 0;
> +		}
> +		/* If error happens at first call, we have a bogus address. */
> +		if (addr != NULL && err != EIO) {
> +			if (errno != ESRCH) {
> +				fprintf(stderr, "strace: ptrace(PTRACE_PEEKDATA,%u,%p,0): %s\n",
> +					(int) tcp->pid, addr, strerror(err));
> +				perror("ptrace: umoven");
> +			}
> +			tcp->ptrace_errno = errno;
> +			return -1;
> +		}
> +	}
>  	return l;
>  }

Previous comment about errno is applicable to this function as well.


-- 
ldv
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 197 bytes
Desc: not available
URL: <http://lists.strace.io/pipermail/strace-devel/attachments/20090224/cef63969/attachment.bin>