Re: [kvm:queue 6/21] arch/x86/kvm/cpuid.c:564:39: sparse: dereference of noderef expression

Re: [kvm:queue 6/21] arch/x86/kvm/cpuid.c:564:39: sparse: dereference of noderef expression

[Borislav Petkov]

On Sat, Nov 02, 2013 at 06:07:59PM +0800, kbuild test robot wrote:
> tree:   git://git.kernel.org/pub/scm/virt/kvm/kvm.git queue
> head:   81e87e26796782e014fd1f2bb9cd8fb6ce4021a8
> commit: 9c15bb1d0a8411f9bb3395d21d5309bde7da0c1c [6/21] kvm: Add KVM_GET_EMULATED_CPUID
> reproduce: make C=1 CF=-D__CHECK_ENDIAN__
> 
> 
> sparse warnings: (new ones prefixed by >>)
> 
> >> arch/x86/kvm/cpuid.c:564:39: sparse: dereference of noderef expression
> >> arch/x86/kvm/cpuid.c:565:39: sparse: dereference of noderef expression
> >> arch/x86/kvm/cpuid.c:566:39: sparse: dereference of noderef expression
> 
> vim +564 arch/x86/kvm/cpuid.c
> 
>    558		 *
>    559		 * Sadly, this wasn't enforced for KVM_GET_SUPPORTED_CPUID and so we
>    560		 * have to give ourselves satisfied only with the emulated side. /me
>    561		 * sheds a tear.
>    562		 */
>    563		for (i = 0; i < num_entries; i++) {
>  > 564			if (entries[i].padding[0] ||
>    565			    entries[i].padding[1] ||
>    566			    entries[i].padding[2])
>    567				return true;
>    568		}
>    569		return false;

Ok, how's that:

--
From: Borislav Petkov <bp@suse.de>
Subject: [PATCH] kvm, cpuid: Fix sparse warning

We need to copy padding to kernel space first before looking at it.

Reported-by: kbuild test robot <fengguang.wu@intel.com>
Signed-off-by: Borislav Petkov <bp@suse.de>
---
 arch/x86/kvm/cpuid.c | 8 +++++---
 1 file changed, 5 insertions(+), 3 deletions(-)

diff --git a/arch/x86/kvm/cpuid.c b/arch/x86/kvm/cpuid.c
index 8f66fba804e4..c6976257eff5 100644
--- a/arch/x86/kvm/cpuid.c
+++ b/arch/x86/kvm/cpuid.c
@@ -564,6 +564,7 @@ static bool sanity_check_entries(struct kvm_cpuid_entry2 __user *entries,
 				 __u32 num_entries, unsigned int ioctl_type)
 {
 	int i;
+	__u32 pad[3];
 
 	if (ioctl_type != KVM_GET_EMULATED_CPUID)
 		return false;
@@ -577,9 +578,10 @@ static bool sanity_check_entries(struct kvm_cpuid_entry2 __user *entries,
 	 * sheds a tear.
 	 */
 	for (i = 0; i < num_entries; i++) {
-		if (entries[i].padding[0] ||
-		    entries[i].padding[1] ||
-		    entries[i].padding[2])
+		if (copy_from_user(pad, entries[i].padding, sizeof(pad)))
+			return true;
+
+		if (pad[0] || pad[1] || pad[2])
 			return true;
 	}
 	return false;
-- 
1.8.0


-- 
Regards/Gruss,
Boris.

Re: [kvm:queue 6/21] arch/x86/kvm/cpuid.c:564:39: sparse: dereference of noderef expression

[Paolo Bonzini]

Il 06/11/2013 15:46, Borislav Petkov ha scritto:
> From: Borislav Petkov <bp@suse.de>
> Subject: [PATCH] kvm, cpuid: Fix sparse warning
> 
> We need to copy padding to kernel space first before looking at it.
> 
> Reported-by: kbuild test robot <fengguang.wu@intel.com>
> Signed-off-by: Borislav Petkov <bp@suse.de>
> ---
>  arch/x86/kvm/cpuid.c | 8 +++++---
>  1 file changed, 5 insertions(+), 3 deletions(-)
> 
> diff --git a/arch/x86/kvm/cpuid.c b/arch/x86/kvm/cpuid.c
> index 8f66fba804e4..c6976257eff5 100644
> --- a/arch/x86/kvm/cpuid.c
> +++ b/arch/x86/kvm/cpuid.c
> @@ -564,6 +564,7 @@ static bool sanity_check_entries(struct kvm_cpuid_entry2 __user *entries,
>  				 __u32 num_entries, unsigned int ioctl_type)
>  {
>  	int i;
> +	__u32 pad[3];
>  
>  	if (ioctl_type != KVM_GET_EMULATED_CPUID)
>  		return false;
> @@ -577,9 +578,10 @@ static bool sanity_check_entries(struct kvm_cpuid_entry2 __user *entries,
>  	 * sheds a tear.
>  	 */
>  	for (i = 0; i < num_entries; i++) {
> -		if (entries[i].padding[0] ||
> -		    entries[i].padding[1] ||
> -		    entries[i].padding[2])
> +		if (copy_from_user(pad, entries[i].padding, sizeof(pad)))
> +			return true;
> +
> +		if (pad[0] || pad[1] || pad[2])
>  			return true;
>  	}
>  	return false;
> 

Applied by Gleb -- thanks.

Paolo