linux-kernel - Re: [PATCH v2 03/27] staging: unisys: iovmcall

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20160601133618.GC4578@hmsreliant.think-freely.org>
Date:	Wed, 1 Jun 2016 09:36:18 -0400
From:	Neil Horman <nhorman@...hat.com>
To:	David Kershner <david.kershner@...sys.com>
Cc:	corbet@....net, tglx@...utronix.de, mingo@...hat.com,
	hpa@...or.com, gregkh@...uxfoundation.org,
	erik.arfvidson@...sys.com, timothy.sell@...sys.com,
	hofrat@...dl.org, dzickus@...hat.com, jes.sorensen@...hat.com,
	alexander.curtin@...sys.com, janani.rvchndrn@...il.com,
	sudipm.mukherjee@...il.com, prarit@...hat.com,
	david.binder@...sys.com, dan.j.williams@...el.com,
	linux-kernel@...r.kernel.org, linux-doc@...r.kernel.org,
	driverdev-devel@...uxdriverproject.org, sparmaintainer@...sys.com
Subject: Re: [PATCH v2 03/27] staging: unisys: iovmcall_gnuc.h change -1
 return values

On Tue, May 31, 2016 at 10:26:29PM -0400, David Kershner wrote:
> From: Erik Arfvidson <erik.arfvidson@...sys.com>
> 
> This patch changes the vague -1 return values to -EPERM.
> This operation is not supported is a good alternative
> to -1 because the return is basically telling the caller
> that the processor doesn't support vmcall operations.
> 
> Signed-off-by: Erik Arfvidson <erik.arfvidson@...sys.com>
> Signed-off-by: David Kershner <david.kershner@...sys.com>
> Reviewed-by: Tim Sell <Timothy.Sell@...sys.com>
> ---
>  drivers/staging/unisys/visorbus/iovmcall_gnuc.h | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/staging/unisys/visorbus/iovmcall_gnuc.h b/drivers/staging/unisys/visorbus/iovmcall_gnuc.h
> index b08b6ec..98ea7f3 100644
> --- a/drivers/staging/unisys/visorbus/iovmcall_gnuc.h
> +++ b/drivers/staging/unisys/visorbus/iovmcall_gnuc.h
> @@ -22,7 +22,7 @@ __unisys_vmcall_gnuc(unsigned long tuple, unsigned long reg_ebx,
>  
>  	cpuid(0x00000001, &cpuid_eax, &cpuid_ebx, &cpuid_ecx, &cpuid_edx);
>  	if (!(cpuid_ecx & 0x80000000))
> -		return -1;
> +		return -EPERM;
>  
>  	__asm__ __volatile__(".byte 0x00f, 0x001, 0x0c1" : "=a"(result) :
>  		"a"(tuple), "b"(reg_ebx), "c"(reg_ecx));
> @@ -40,7 +40,7 @@ __unisys_extended_vmcall_gnuc(unsigned long long tuple,
>  
>  	cpuid(0x00000001, &cpuid_eax, &cpuid_ebx, &cpuid_ecx, &cpuid_edx);
>  	if (!(cpuid_ecx & 0x80000000))
> -		return -1;
> +		return -EPERM;
>  
>  	__asm__ __volatile__(".byte 0x00f, 0x001, 0x0c1" : "=a"(result) :
>  		"a"(tuple), "b"(reg_ebx), "c"(reg_ecx), "d"(reg_edx));
> -- 
> 1.9.1
> 

This gets properly checked as far as return codes go for the most part, which is
good, but in the case of an issuing of the
VMCALL_QUERY_GUEST_VIRTUAL_TIME_OFFSET ioctl, the return code is simply copied
into a user space buffer.  That might be expected if -1 is used as an error
condition marker, but I would check before you assume that a user space caller
will handle -EINVAL properly there.