[PATCH] Fix umove/umoven return value

[Dmitry V. Levin]

On Tue, Feb 26, 2013 at 12:32:40PM +0100, Denys Vlasenko wrote:
> On 02/26/2013 04:07 AM, Ben Noordhuis wrote:
> > On Tue, Feb 26, 2013 at 3:45 AM, Dmitry V. Levin <ldv at altlinux.org> wrote:
> >>> diff --git a/util.c b/util.c
> >>> index 405670e..af73f09 100644
> >>> --- a/util.c
> >>> +++ b/util.c
> >>> @@ -800,6 +800,8 @@ umoven(struct tcb *tcp, long addr, int len, char *laddr)
> >>>                               remote, 1,
> >>>                               /*flags:*/ 0
> >>>               );
> >>> +             if (r == len)
> >>> +                     return 0;
> >>
> >> This should rather be (r >= 0), and errno != ENOSYS shouldn't fall back to
> >> vm_readv_didnt_work case.
> > 
> > r >= 0 seems less safe than r == len.  What if you get less bytes than expected?
> 
> In fact old code (one which uses PTRACE_PEEKDATA) does return 0
> on short reads... but I think you are right, it's not what it should do.

I've examined the code, nowhere short reads are expected and processed
properly, the most likely result of umoven/umovestr returning >= 0 after
a short read is decoding of random garbage instead of unread bytes.

> I propose that we comment out old code for now:
> 
> #if 0
> //FIXME: wrong. printpath doesn't use this routine. Callers expect full copies.
> //Ok to remove?
>                         if (started && (errno==EPERM || errno==EIO)) {
>                                 /* Ran into 'end of memory' - stupid "printpath" */
>                                 return 0;
>                         }
> #endif
> 
> and we can delete it later (or uncomment if I'm wrong).

Not only this hunk of umoven is wrong, the same hunk of umovestr is also
wrong despite of the fact that printpath uses umovestr: a short read in
umovestr would end up with printpathn printing some garbage from stack.

I'm going to cleanup these two functions now.


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