Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1562128 > unrolled thread

[PATCH] MIPS: KVM: Return directly after a failed copy_from_user() in kvm_arch_vcpu_ioctl()

Started bySF Markus Elfring <elfring@users.sourceforge.net>
First post2017-01-18 21:10 +0100
Last post2017-01-19 12:20 +0100
Articles 6 — 4 participants

Back to article view | Back to linux.kernel

Contents

  [PATCH] MIPS: KVM: Return directly after a failed copy_from_user() in  kvm_arch_vcpu_ioctl() SF Markus Elfring <elfring@users.sourceforge.net> - 2017-01-18 21:10 +0100
    Re: [PATCH] MIPS: KVM: Return directly after a failed  copy_from_user() in kvm_arch_vcpu_ioctl() Paolo Bonzini <pbonzini@redhat.com> - 2017-01-19 11:00 +0100
      [PATCH v2] MIPS: KVM: Return directly after a failed copy_from_user()  in kvm_arch_vcpu_ioctl() SF Markus Elfring <elfring@users.sourceforge.net> - 2017-01-19 11:30 +0100
        Re: [PATCH v2] MIPS: KVM: Return directly after a failed  copy_from_user() in kvm_arch_vcpu_ioctl() Paolo Bonzini <pbonzini@redhat.com> - 2017-01-19 11:30 +0100
          Re: [PATCH v2] MIPS: KVM: Return directly after a failed  copy_from_user() in kvm_arch_vcpu_ioctl() James Hogan <james.hogan@imgtec.com> - 2017-01-19 13:10 +0100
    Re: [PATCH] MIPS: KVM: Return directly after a failed  copy_from_user() in kvm_arch_vcpu_ioctl() kbuild test robot <lkp@intel.com> - 2017-01-19 12:20 +0100

#1562128 — [PATCH] MIPS: KVM: Return directly after a failed copy_from_user() in kvm_arch_vcpu_ioctl()

FromSF Markus Elfring <elfring@users.sourceforge.net>
Date2017-01-18 21:10 +0100
Subject[PATCH] MIPS: KVM: Return directly after a failed copy_from_user() in kvm_arch_vcpu_ioctl()
Message-ID<t14Iq-91-13@gated-at.bofh.it>
From: Markus Elfring <elfring@users.sourceforge.net>
Date: Wed, 18 Jan 2017 20:43:41 +0100

Return directly after a call of the function "copy_from_user" failed
in a case block.

Signed-off-by: Markus Elfring <elfring@users.sourceforge.net>
---
 arch/mips/kvm/mips.c | 7 ++-----
 1 file changed, 2 insertions(+), 5 deletions(-)

diff --git a/arch/mips/kvm/mips.c b/arch/mips/kvm/mips.c
index 06a60b19acfb..1dad78f74e8c 100644
--- a/arch/mips/kvm/mips.c
+++ b/arch/mips/kvm/mips.c
@@ -1152,10 +1152,8 @@ long kvm_arch_vcpu_ioctl(struct file *filp, unsigned int ioctl,
 		{
 			struct kvm_mips_interrupt irq;
 
-			r = -EFAULT;
 			if (copy_from_user(&irq, argp, sizeof(irq)))
-				goto out;
-
+				return -EFAULT;
 			kvm_debug("[%d] %s: irq: %d\n", vcpu->vcpu_id, __func__,
 				  irq.irq);
 
@@ -1165,9 +1163,8 @@ long kvm_arch_vcpu_ioctl(struct file *filp, unsigned int ioctl,
 	case KVM_ENABLE_CAP: {
 		struct kvm_enable_cap cap;
 
-		r = -EFAULT;
 		if (copy_from_user(&cap, argp, sizeof(cap)))
-			goto out;
+			return -EFAULT;
 		r = kvm_vcpu_ioctl_enable_cap(vcpu, &cap);
 		break;
 	}
-- 
2.11.0

[toc] | [next] | [standalone]

#1562544 — Re: [PATCH] MIPS: KVM: Return directly after a failed copy_from_user() in kvm_arch_vcpu_ioctl()

FromPaolo Bonzini <pbonzini@redhat.com>
Date2017-01-19 11:00 +0100
SubjectRe: [PATCH] MIPS: KVM: Return directly after a failed copy_from_user() in kvm_arch_vcpu_ioctl()
Message-ID<t1hFE-8hK-25@gated-at.bofh.it>
In reply to#1562128

On 18/01/2017 20:52, SF Markus Elfring wrote:
> From: Markus Elfring <elfring@users.sourceforge.net>
> Date: Wed, 18 Jan 2017 20:43:41 +0100
> 
> Return directly after a call of the function "copy_from_user" failed
> in a case block.
> 
> Signed-off-by: Markus Elfring <elfring@users.sourceforge.net>

The "out" label is now unused, so you should remove it.

Paolo

> ---
>  arch/mips/kvm/mips.c | 7 ++-----
>  1 file changed, 2 insertions(+), 5 deletions(-)
> 
> diff --git a/arch/mips/kvm/mips.c b/arch/mips/kvm/mips.c
> index 06a60b19acfb..1dad78f74e8c 100644
> --- a/arch/mips/kvm/mips.c
> +++ b/arch/mips/kvm/mips.c
> @@ -1152,10 +1152,8 @@ long kvm_arch_vcpu_ioctl(struct file *filp, unsigned int ioctl,
>  		{
>  			struct kvm_mips_interrupt irq;
>  
> -			r = -EFAULT;
>  			if (copy_from_user(&irq, argp, sizeof(irq)))
> -				goto out;
> -
> +				return -EFAULT;
>  			kvm_debug("[%d] %s: irq: %d\n", vcpu->vcpu_id, __func__,
>  				  irq.irq);
>  
> @@ -1165,9 +1163,8 @@ long kvm_arch_vcpu_ioctl(struct file *filp, unsigned int ioctl,
>  	case KVM_ENABLE_CAP: {
>  		struct kvm_enable_cap cap;
>  
> -		r = -EFAULT;
>  		if (copy_from_user(&cap, argp, sizeof(cap)))
> -			goto out;
> +			return -EFAULT;
>  		r = kvm_vcpu_ioctl_enable_cap(vcpu, &cap);
>  		break;
>  	}
>

[toc] | [prev] | [next] | [standalone]

#1562567 — [PATCH v2] MIPS: KVM: Return directly after a failed copy_from_user() in kvm_arch_vcpu_ioctl()

FromSF Markus Elfring <elfring@users.sourceforge.net>
Date2017-01-19 11:30 +0100
Subject[PATCH v2] MIPS: KVM: Return directly after a failed copy_from_user() in kvm_arch_vcpu_ioctl()
Message-ID<t1i8F-jB-7@gated-at.bofh.it>
In reply to#1562544
From: Markus Elfring <elfring@users.sourceforge.net>
Date: Thu, 19 Jan 2017 11:10:26 +0100

* Return directly after a call of the function "copy_from_user" failed
  in a case block.

* Delete the jump label "out" which became unnecessary with
  this refactoring.

Signed-off-by: Markus Elfring <elfring@users.sourceforge.net>
---

V2:
A label was also removed at the end.

 arch/mips/kvm/mips.c | 9 ++-------
 1 file changed, 2 insertions(+), 7 deletions(-)

diff --git a/arch/mips/kvm/mips.c b/arch/mips/kvm/mips.c
index 06a60b19acfb..3534a0b9efed 100644
--- a/arch/mips/kvm/mips.c
+++ b/arch/mips/kvm/mips.c
@@ -1152,10 +1152,8 @@ long kvm_arch_vcpu_ioctl(struct file *filp, unsigned int ioctl,
 		{
 			struct kvm_mips_interrupt irq;
 
-			r = -EFAULT;
 			if (copy_from_user(&irq, argp, sizeof(irq)))
-				goto out;
-
+				return -EFAULT;
 			kvm_debug("[%d] %s: irq: %d\n", vcpu->vcpu_id, __func__,
 				  irq.irq);
 
@@ -1165,17 +1163,14 @@ long kvm_arch_vcpu_ioctl(struct file *filp, unsigned int ioctl,
 	case KVM_ENABLE_CAP: {
 		struct kvm_enable_cap cap;
 
-		r = -EFAULT;
 		if (copy_from_user(&cap, argp, sizeof(cap)))
-			goto out;
+			return -EFAULT;
 		r = kvm_vcpu_ioctl_enable_cap(vcpu, &cap);
 		break;
 	}
 	default:
 		r = -ENOIOCTLCMD;
 	}
-
-out:
 	return r;
 }
 
-- 
2.11.0

[toc] | [prev] | [next] | [standalone]

#1562568 — Re: [PATCH v2] MIPS: KVM: Return directly after a failed copy_from_user() in kvm_arch_vcpu_ioctl()

FromPaolo Bonzini <pbonzini@redhat.com>
Date2017-01-19 11:30 +0100
SubjectRe: [PATCH v2] MIPS: KVM: Return directly after a failed copy_from_user() in kvm_arch_vcpu_ioctl()
Message-ID<t1i8F-jB-9@gated-at.bofh.it>
In reply to#1562567

On 19/01/2017 11:20, SF Markus Elfring wrote:
> From: Markus Elfring <elfring@users.sourceforge.net>
> Date: Thu, 19 Jan 2017 11:10:26 +0100
> 
> * Return directly after a call of the function "copy_from_user" failed
>   in a case block.
> 
> * Delete the jump label "out" which became unnecessary with
>   this refactoring.
> 
> Signed-off-by: Markus Elfring <elfring@users.sourceforge.net>
> ---
> 
> V2:
> A label was also removed at the end.
> 
>  arch/mips/kvm/mips.c | 9 ++-------
>  1 file changed, 2 insertions(+), 7 deletions(-)
> 
> diff --git a/arch/mips/kvm/mips.c b/arch/mips/kvm/mips.c
> index 06a60b19acfb..3534a0b9efed 100644
> --- a/arch/mips/kvm/mips.c
> +++ b/arch/mips/kvm/mips.c
> @@ -1152,10 +1152,8 @@ long kvm_arch_vcpu_ioctl(struct file *filp, unsigned int ioctl,
>  		{
>  			struct kvm_mips_interrupt irq;
>  
> -			r = -EFAULT;
>  			if (copy_from_user(&irq, argp, sizeof(irq)))
> -				goto out;
> -
> +				return -EFAULT;
>  			kvm_debug("[%d] %s: irq: %d\n", vcpu->vcpu_id, __func__,
>  				  irq.irq);
>  
> @@ -1165,17 +1163,14 @@ long kvm_arch_vcpu_ioctl(struct file *filp, unsigned int ioctl,
>  	case KVM_ENABLE_CAP: {
>  		struct kvm_enable_cap cap;
>  
> -		r = -EFAULT;
>  		if (copy_from_user(&cap, argp, sizeof(cap)))
> -			goto out;
> +			return -EFAULT;
>  		r = kvm_vcpu_ioctl_enable_cap(vcpu, &cap);
>  		break;
>  	}
>  	default:
>  		r = -ENOIOCTLCMD;
>  	}
> -
> -out:
>  	return r;
>  }
>  
> 

Removing the label makes the patch worthwhile.

Reviewed-by: Paolo Bonzini <pbonzini@redhat.com>

[toc] | [prev] | [next] | [standalone]

#1562660 — Re: [PATCH v2] MIPS: KVM: Return directly after a failed copy_from_user() in kvm_arch_vcpu_ioctl()

FromJames Hogan <james.hogan@imgtec.com>
Date2017-01-19 13:10 +0100
SubjectRe: [PATCH v2] MIPS: KVM: Return directly after a failed copy_from_user() in kvm_arch_vcpu_ioctl()
Message-ID<t1jHs-1mS-37@gated-at.bofh.it>
In reply to#1562568

[Multipart message — attachments visible in raw view] — view raw

On Thu, Jan 19, 2017 at 11:27:52AM +0100, Paolo Bonzini wrote:
> 
> 
> On 19/01/2017 11:20, SF Markus Elfring wrote:
> > From: Markus Elfring <elfring@users.sourceforge.net>
> > Date: Thu, 19 Jan 2017 11:10:26 +0100
> > 
> > * Return directly after a call of the function "copy_from_user" failed
> >   in a case block.
> > 
> > * Delete the jump label "out" which became unnecessary with
> >   this refactoring.
> > 
> > Signed-off-by: Markus Elfring <elfring@users.sourceforge.net>
> > ---
> > 
> > V2:
> > A label was also removed at the end.
> > 
> >  arch/mips/kvm/mips.c | 9 ++-------
> >  1 file changed, 2 insertions(+), 7 deletions(-)
> > 
> > diff --git a/arch/mips/kvm/mips.c b/arch/mips/kvm/mips.c
> > index 06a60b19acfb..3534a0b9efed 100644
> > --- a/arch/mips/kvm/mips.c
> > +++ b/arch/mips/kvm/mips.c
> > @@ -1152,10 +1152,8 @@ long kvm_arch_vcpu_ioctl(struct file *filp, unsigned int ioctl,
> >  		{
> >  			struct kvm_mips_interrupt irq;
> >  
> > -			r = -EFAULT;
> >  			if (copy_from_user(&irq, argp, sizeof(irq)))
> > -				goto out;
> > -
> > +				return -EFAULT;
> >  			kvm_debug("[%d] %s: irq: %d\n", vcpu->vcpu_id, __func__,
> >  				  irq.irq);
> >  
> > @@ -1165,17 +1163,14 @@ long kvm_arch_vcpu_ioctl(struct file *filp, unsigned int ioctl,
> >  	case KVM_ENABLE_CAP: {
> >  		struct kvm_enable_cap cap;
> >  
> > -		r = -EFAULT;
> >  		if (copy_from_user(&cap, argp, sizeof(cap)))
> > -			goto out;
> > +			return -EFAULT;
> >  		r = kvm_vcpu_ioctl_enable_cap(vcpu, &cap);
> >  		break;
> >  	}
> >  	default:
> >  		r = -ENOIOCTLCMD;
> >  	}
> > -
> > -out:
> >  	return r;
> >  }
> >  
> > 
> 
> Removing the label makes the patch worthwhile.
> 
> Reviewed-by: Paolo Bonzini <pbonzini@redhat.com>

Thanks Markus & Paolo. Looks better now. I'll apply for 4.11.

Cheers
James

[toc] | [prev] | [next] | [standalone]

#1562624 — Re: [PATCH] MIPS: KVM: Return directly after a failed copy_from_user() in kvm_arch_vcpu_ioctl()

Fromkbuild test robot <lkp@intel.com>
Date2017-01-19 12:20 +0100
SubjectRe: [PATCH] MIPS: KVM: Return directly after a failed copy_from_user() in kvm_arch_vcpu_ioctl()
Message-ID<t1iV4-Qv-19@gated-at.bofh.it>
In reply to#1562128

[Multipart message — attachments visible in raw view] — view raw

Hi Markus,

[auto build test ERROR on kvm/linux-next]
[also build test ERROR on v4.10-rc4 next-20170119]
[if your patch is applied to the wrong git tree, please drop us a note to help improve the system]

url:    https://github.com/0day-ci/linux/commits/SF-Markus-Elfring/MIPS-KVM-Return-directly-after-a-failed-copy_from_user-in-kvm_arch_vcpu_ioctl/20170119-154855
base:   https://git.kernel.org/pub/scm/virt/kvm/kvm.git linux-next
config: mips-malta_kvm_defconfig (attached as .config)
compiler: mipsel-linux-gnu-gcc (Debian 6.1.1-9) 6.1.1 20160705
reproduce:
        wget https://git.kernel.org/cgit/linux/kernel/git/wfg/lkp-tests.git/plain/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # save the attached .config to linux build tree
        make.cross ARCH=mips 

All errors (new ones prefixed by >>):

   arch/mips/kvm/mips.c: In function 'kvm_arch_vcpu_ioctl':
>> arch/mips/kvm/mips.c:1175:1: error: label 'out' defined but not used [-Werror=unused-label]
    out:
    ^~~
   cc1: all warnings being treated as errors

vim +/out +1175 arch/mips/kvm/mips.c

5fafd8748 arch/mips/kvm/mips.c     James Hogan 2014-12-08  1169  		break;
5fafd8748 arch/mips/kvm/mips.c     James Hogan 2014-12-08  1170  	}
669e846e6 arch/mips/kvm/kvm_mips.c Sanjay Lal  2012-11-21  1171  	default:
4c73fb2b0 arch/mips/kvm/kvm_mips.c David Daney 2013-05-23  1172  		r = -ENOIOCTLCMD;
669e846e6 arch/mips/kvm/kvm_mips.c Sanjay Lal  2012-11-21  1173  	}
669e846e6 arch/mips/kvm/kvm_mips.c Sanjay Lal  2012-11-21  1174  
669e846e6 arch/mips/kvm/kvm_mips.c Sanjay Lal  2012-11-21 @1175  out:
669e846e6 arch/mips/kvm/kvm_mips.c Sanjay Lal  2012-11-21  1176  	return r;
669e846e6 arch/mips/kvm/kvm_mips.c Sanjay Lal  2012-11-21  1177  }
669e846e6 arch/mips/kvm/kvm_mips.c Sanjay Lal  2012-11-21  1178  

:::::: The code at line 1175 was first introduced by commit
:::::: 669e846e6c4e13f16d7418973609931e362cb16a KVM/MIPS32: MIPS arch specific APIs for KVM

:::::: TO: Sanjay Lal <sanjayl@kymasys.com>
:::::: CC: Ralf Baechle <ralf@linux-mips.org>

---
0-DAY kernel test infrastructure                Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all                   Intel Corporation

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web