From mboxrd@z Thu Jan 1 00:00:00 1970 From: "tiejun.chen" Date: Wed, 03 Apr 2013 01:28:01 +0000 Subject: Re: [RFC PATCH v2 1/6] kvm: add device control API Message-Id: <515B85A1.4000602@windriver.com> List-Id: References: <1360820960-12537-1-git-send-email-scottwood@freescale.com> <1364856473-25245-1-git-send-email-scottwood@freescale.com> <1364856473-25245-2-git-send-email-scottwood@freescale.com> <515A81ED.9060106@windriver.com> <1364923807.24520.2@snotra> In-Reply-To: <1364923807.24520.2@snotra> MIME-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit To: Scott Wood Cc: Alexander Graf , kvm-ppc@vger.kernel.org, kvm@vger.kernel.org, paulus@samba.org On 04/03/2013 01:30 AM, Scott Wood wrote: > On 04/02/2013 01:59:57 AM, tiejun.chen wrote: >> On 04/02/2013 06:47 AM, Scott Wood wrote: >>> diff --git a/virt/kvm/kvm_main.c b/virt/kvm/kvm_main.c >>> index ff71541..ed033c0 100644 >>> --- a/virt/kvm/kvm_main.c >>> +++ b/virt/kvm/kvm_main.c >>> @@ -2158,6 +2158,17 @@ out: >>> } >>> #endif >>> >>> +static int kvm_ioctl_create_device(struct kvm *kvm, >>> + struct kvm_create_device *cd) >>> +{ >>> + bool test = cd->flags & KVM_CREATE_DEVICE_TEST; >>> + >>> + switch (cd->type) { >>> + default: >>> + return -ENODEV; >>> + } >> >> Even after apply patch 5, looks here still misses something like: >> >> if (test) >> WARN_ON_ONCE(!cd->type); > > Why? How does userspace passing in a bad type value mean the kernel needs to > report internal badness, why is a value of zero worse than any other bad value, > and why only when the test flag is set? I just mean we need do something here since looks the 'test' variable is defined but unused, right? But please correct this as you expect :) And if the userspace can't guarantee cd->type is never zero, we should return -ENODEV as well after that switch(). Tiejun