| Message ID | 20211205194719.16987-1-amhamza.mgc@gmail.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | KVM: x86: fix for missing initialization of return status variable | expand |
Ameer Hamza <amhamza.mgc@gmail.com> writes: > If undefined ioctl number is passed to the kvm_vcpu_ioctl_device_attr > function, it should return with error status. > > Addresses-Coverity: 1494124 ("Uninitialized scalar variable") > > Signed-off-by: Ameer Hamza <amhamza.mgc@gmail.com> > --- > arch/x86/kvm/x86.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c > index e0aa4dd53c7f..55b90c185717 100644 > --- a/arch/x86/kvm/x86.c > +++ b/arch/x86/kvm/x86.c > @@ -5001,7 +5001,7 @@ static int kvm_vcpu_ioctl_device_attr(struct kvm_vcpu *vcpu, > void __user *argp) > { > struct kvm_device_attr attr; > - int r; > + int r = -EINVAL; > > if (copy_from_user(&attr, argp, sizeof(attr))) > return -EFAULT; The reported issue is not real, kvm_vcpu_ioctl_device_attr() is never called with anything but [KVM_HAS_DEVICE_ATTR, KVM_GET_DEVICE_ATTR, KVM_SET_DEVICE_ATTR] as 'ioctl' and the switch below covers all three. Instead of initializing 'r' we could've added a 'default' case to the switch, either returning something like EINVAL or just BUG(). Hope it'll silence coverity.
On Mon, Dec 06, 2021 at 10:06:26AM +0100, Vitaly Kuznetsov wrote: > Ameer Hamza <amhamza.mgc@gmail.com> writes: > > > If undefined ioctl number is passed to the kvm_vcpu_ioctl_device_attr > > function, it should return with error status. > > > > Addresses-Coverity: 1494124 ("Uninitialized scalar variable") > > > > Signed-off-by: Ameer Hamza <amhamza.mgc@gmail.com> > > --- > > arch/x86/kvm/x86.c | 2 +- > > 1 file changed, 1 insertion(+), 1 deletion(-) > > > > diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c > > index e0aa4dd53c7f..55b90c185717 100644 > > --- a/arch/x86/kvm/x86.c > > +++ b/arch/x86/kvm/x86.c > > @@ -5001,7 +5001,7 @@ static int kvm_vcpu_ioctl_device_attr(struct kvm_vcpu *vcpu, > > void __user *argp) > > { > > struct kvm_device_attr attr; > > - int r; > > + int r = -EINVAL; > > > > if (copy_from_user(&attr, argp, sizeof(attr))) > > return -EFAULT; > > The reported issue is not real, kvm_vcpu_ioctl_device_attr() is never > called with anything but [KVM_HAS_DEVICE_ATTR, KVM_GET_DEVICE_ATTR, > KVM_SET_DEVICE_ATTR] as 'ioctl' and the switch below covers all > three. Instead of initializing 'r' we could've added a 'default' case to > the switch, either returning something like EINVAL or just BUG(). Hope > it'll silence coverity. Thank you for your kind response. I agree with you and I think its logical to add default case here. Let me update the patch. Best Regards, Hamza
diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c index e0aa4dd53c7f..55b90c185717 100644 --- a/arch/x86/kvm/x86.c +++ b/arch/x86/kvm/x86.c @@ -5001,7 +5001,7 @@ static int kvm_vcpu_ioctl_device_attr(struct kvm_vcpu *vcpu, void __user *argp) { struct kvm_device_attr attr; - int r; + int r = -EINVAL; if (copy_from_user(&attr, argp, sizeof(attr))) return -EFAULT;
If undefined ioctl number is passed to the kvm_vcpu_ioctl_device_attr function, it should return with error status. Addresses-Coverity: 1494124 ("Uninitialized scalar variable") Signed-off-by: Ameer Hamza <amhamza.mgc@gmail.com> --- arch/x86/kvm/x86.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)