Re: [PATCH V2 1/3] kvm: use kmalloc() instead of kzalloc() during iodev register/unregister
On Tue, 2015-08-25 at 15:47 +0800, Jason Wang wrote: All fields of kvm_io_range were initialized or copied explicitly afterwards. So switch to use kmalloc(). Is there any compiler added alignment padding in either structure? If so, those padding areas would now be uninitialized and may leak kernel data if copied to user-space. diff --git a/virt/kvm/kvm_main.c b/virt/kvm/kvm_main.c [] @@ -3248,7 +3248,7 @@ int kvm_io_bus_register_dev(struct kvm *kvm, enum kvm_bus bus_idx, gpa_t addr, if (bus-dev_count - bus-ioeventfd_count NR_IOBUS_DEVS - 1) return -ENOSPC; - new_bus = kzalloc(sizeof(*bus) + ((bus-dev_count + 1) * + new_bus = kmalloc(sizeof(*bus) + ((bus-dev_count + 1) * sizeof(struct kvm_io_range)), GFP_KERNEL); if (!new_bus) return -ENOMEM; @@ -3280,7 +3280,7 @@ int kvm_io_bus_unregister_dev(struct kvm *kvm, enum kvm_bus bus_idx, if (r) return r; - new_bus = kzalloc(sizeof(*bus) + ((bus-dev_count - 1) * + new_bus = kmalloc(sizeof(*bus) + ((bus-dev_count - 1) * sizeof(struct kvm_io_range)), GFP_KERNEL); if (!new_bus) return -ENOMEM; -- To unsubscribe from this list: send the line unsubscribe kvm in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH V2 1/3] kvm: use kmalloc() instead of kzalloc() during iodev register/unregister
On Wed, 2015-08-26 at 13:39 +0800, Jason Wang wrote: On 08/25/2015 11:29 PM, Joe Perches wrote: On Tue, 2015-08-25 at 15:47 +0800, Jason Wang wrote: All fields of kvm_io_range were initialized or copied explicitly afterwards. So switch to use kmalloc(). Is there any compiler added alignment padding in either structure? If so, those padding areas would now be uninitialized and may leak kernel data if copied to user-space. I get your concern, but I don't a way to copy them to userspace, did you? I didn't look. I just wanted you to be aware there's a difference and a reason why kzalloc might be used even though all structure members are initialized. -- To unsubscribe from this list: send the line unsubscribe kvm in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH V2 1/3] kvm: use kmalloc() instead of kzalloc() during iodev register/unregister
On 08/26/2015 01:45 PM, Joe Perches wrote: On Wed, 2015-08-26 at 13:39 +0800, Jason Wang wrote: On 08/25/2015 11:29 PM, Joe Perches wrote: On Tue, 2015-08-25 at 15:47 +0800, Jason Wang wrote: All fields of kvm_io_range were initialized or copied explicitly afterwards. So switch to use kmalloc(). Is there any compiler added alignment padding in either structure? If so, those padding areas would now be uninitialized and may leak kernel data if copied to user-space. I get your concern, but I don't a way to copy them to userspace, did you? I didn't look. I just wanted you to be aware there's a difference and a reason why kzalloc might be used even though all structure members are initialized. I see, thanks for the reminding. Looks like we are safe and I will add something like kvm_io_range was never accessed by userspace in the commit log if there's a new version. -- To unsubscribe from this list: send the line unsubscribe kvm in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH V2 1/3] kvm: use kmalloc() instead of kzalloc() during iodev register/unregister
On 08/25/2015 11:29 PM, Joe Perches wrote: On Tue, 2015-08-25 at 15:47 +0800, Jason Wang wrote: All fields of kvm_io_range were initialized or copied explicitly afterwards. So switch to use kmalloc(). Is there any compiler added alignment padding in either structure? If so, those padding areas would now be uninitialized and may leak kernel data if copied to user-space. I get your concern, but I don't a way to copy them to userspace, did you? -- To unsubscribe from this list: send the line unsubscribe kvm in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
