Re: [PATCH] Linux/gdbserver: Fix memory read ptrace fallback issues

[Pedro Alves <palves@redhat.com>]

Hi Maciej,

On 05/16/2012 11:56 PM, Maciej W. Rozycki wrote:

>  While trying to investigate suspicious MIPS16/Linux test suite results 
> noted with the recent MIPS ISA bit solution proposal I have come across 
> this problem in gdbserver.  It contributed to about 2550 failures per 
> multilib.
> 
>  In linux_read_memory we make a two-way choice as to how we read 
> debuggee's memory -- we prefer pread64 or lseek/read of /proc/<pid>/mem 
> and failing that we resort to a long boring series of PEEKTEXT ptrace 
> requests.  We use this in particular in linux_qxfer_libraries_svr4 to 
> access names of shared libraries loaded.  There we rely on 
> linux_read_memory to return data in the buffer provided even if the 
> function wasn't able to read the whole number of bytes requested.
> 
>  This has two problems as I revealed.  If any call in the pread64 fails 
> then the supplied buffer is obviously not filled.


It looks like the pread64/lseek/read path actually reads directly
into MYADDR (the supplied buffer).

> Then if the ptrace

> fallback path is not able to retrieve the whole number of bytes requested, 
> then it does not supply partially retrieved data to the buffer.  This is 
> the scenario that I observed.


Right.

>  The change below addresses these two problems.  Any data successfully 
> retrieved by the ptrace loop is copied over to the user buffer even if 
> linux_read_memory returns unsuccessfully.  If pread64 successfully 
> retrieves any data, then ptrace is not used to read that data again, the 
> attempt to read data is resumed at the point where pread64 stopped.  This 
> will normally not retrieve any further data as pread64 would have provided 
> that (internally, in the Linux kernel, the read handlers for 
> /proc/<pid>/maps special files are implemented as a wrapper around the 
> very same worker function that the PEEKTEXT ptrace request uses.


So how about just returning immediately if reading from /proc manages to
read something?  From what you say, the PEEKTEXT fallback then won't just
normally fail; it'll _always_ fail.  I suspect that'd make the code a bit
simpler.

>    /* Copy appropriate bytes out of the buffer.  */
> +  i *= sizeof (PTRACE_XFER_TYPE);
> +  i -= memaddr & (sizeof (PTRACE_XFER_TYPE) - 1);
>    memcpy (myaddr,
>  	  (char *) buffer + (memaddr & (sizeof (PTRACE_XFER_TYPE) - 1)),
> -	  len);
> +	  i < len ? i : len);
>  
> -  return 0;
> +  return errno;


You shouldn't assume that "errno" is preserved across library
calls (memcpy in this case).

>  }
>  



-- 
Pedro Alves