Re: [PATCH] x86-64: ptrace ia32 BP fix

[Roland McGrath <roland@redhat.com>]

> On Tue, Jul 05, 2005 at 02:31:15AM -0700, Roland McGrath wrote:
> > --- a/arch/x86_64/ia32/ia32entry.S
> > +++ b/arch/x86_64/ia32/ia32entry.S
> > @@ -102,6 +102,7 @@ sysenter_do_call:	
> >  	.byte	0xf, 0x35
> >  
> >  sysenter_tracesys:
> > +	movl	%r9d,%ebp
> >  	SAVE_REST
> 
> This is wrong because it will clobber ebp before it is saved.
> It is only saved in SAVE_REST.

It is right because it stores %ebp before it is saved in the argument block
that ptrace can access.  That is the point of it.  %r9d has the value
loaded from (%rbp) just prior to this code, which is what %ebp should
reflect to match the i386 behavior.

> 
> >  	CLEAR_RREGS
> >  	movq	$-ENOSYS,RAX(%rsp)	/* really needed? */
> > @@ -109,13 +110,7 @@ sysenter_tracesys:
> >  	call	syscall_trace_enter
> >  	LOAD_ARGS ARGOFFSET  /* reload args from stack in case ptrace changed it */
> >  	RESTORE_REST
> > -	movl	%ebp, %ebp
> > -	/* no need to do an access_ok check here because rbp has been
> > -	   32bit zero extended */ 
> > -1:	movl	(%rbp),%r9d
> > -	.section __ex_table,"a"
> > -	.quad 1b,ia32_badarg
> > -	.previous
> > +	movl	%ebp,%r9d
> 
> And this also cannot be correct because RESTORE_REST has overwritten %rbp
> already.

This is also correct because RESTORE_REST has stored into %rbp the value in
the argument block, which ptrace may have modified.  This movl ensures that
this changed value is what the system call's argument will be.

The patch is tested and works.  Just try strace on a 32-bit program that
calls mmap2 and look at the 6th argument value.  It shows a stack address
without this patch.  With this patch, it shows the argument value the same
as strace on a native i386 kernel does.


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