[CRIU] [PATCH] compel: Do not loose sign of result in compat syscall

[Cyrill Gorcunov]

On Mon, Oct 30, 2017 at 12:18:12PM -0700, Andrey Vagin wrote:
> On Mon, Oct 30, 2017 at 05:57:31PM +0300, Cyrill Gorcunov wrote:
> > From: Cyrill Gorcunov <gorcunov at virtuozzo.com>
> > 
> > Regs are present in unsigned format so convert them
> > into signed first to provide results.
> > 
> > In particular if memfd_create syscall failed we won't
> > notice -ENOMEM error but rather treat it as unsigned
> > hex value
> > 
> >  | (05.303002) Putting parasite blob into 0x7f1c6ffe0000->0xfffffff4
> >  | (05.303234) Putting tsock into pid 42773
> > 
> > Signed-off-by: Cyrill Gorcunov <gorcunov at virtuozzo.com>
> > ---
> > 
> > Travis is running by now https://travis-ci.org/cyrillos/criu/builds/294898355
> > 
> >  compel/arch/x86/src/lib/infect.c | 7 +++++--
> >  1 file changed, 5 insertions(+), 2 deletions(-)
> > 
> > diff --git a/compel/arch/x86/src/lib/infect.c b/compel/arch/x86/src/lib/infect.c
> > index 9c919e64ef13..ac5f8b05e768 100644
> > --- a/compel/arch/x86/src/lib/infect.c
> > +++ b/compel/arch/x86/src/lib/infect.c
> > @@ -293,9 +293,10 @@ int compel_syscall(struct parasite_ctl *ctl, int nr, long *ret,
> >  		unsigned long arg6)
> >  {
> >  	user_regs_struct_t regs = ctl->orig.regs;
> > +	bool native = user_regs_native(&regs);
> >  	int err;
> >  
> > -	if (user_regs_native(&regs)) {
> > +	if (native) {
> >  		user_regs_struct64 *r = &regs.native;
> >  
> >  		r->ax  = (uint64_t)nr;
> > @@ -321,7 +322,9 @@ int compel_syscall(struct parasite_ctl *ctl, int nr, long *ret,
> >  		err = compel_execute_syscall(ctl, &regs, code_int_80);
> >  	}
> >  
> > -	*ret = get_user_reg(&regs, ax);
> > +	*ret = native ?
> > +		(long)get_user_reg(&regs, ax) :
> > +		(int)get_user_reg(&regs, ax);
> 
> mmap() can return a negative value, but it will be actually a valid
> address and we have to return it without modifications. How does this
> code handle this case?

This code has nothing to do with such issues, it's up to a caller
to verify the values obtained.

In particular for x86 IS_ERR_VALUE will catch the error if only
sign of the value is not lost, thus we have to preserve it.

More likely we need an additional check on top of this patch
to strip off sign propagated before it get converted into the
void pointer.