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

Groups > linux.kernel > #1212777 > unrolled thread

[PATCH V2 1/3] kvm: use kmalloc() instead of kzalloc() during iodev register/unregister

Back to article view | Back to linux.kernel

Contents

  [PATCH V2 1/3] kvm: use kmalloc() instead of kzalloc() during iodev register/unregister Jason Wang <jasowang@redhat.com> - 2015-08-25 09:50 +0200
    Re: [PATCH V2 1/3] kvm: use kmalloc() instead of kzalloc() during  iodev register/unregister Joe Perches <joe@perches.com> - 2015-08-25 17:30 +0200
      Re: [PATCH V2 1/3] kvm: use kmalloc() instead of kzalloc() during  iodev register/unregister Jason Wang <jasowang@redhat.com> - 2015-08-26 07:40 +0200
        Re: [PATCH V2 1/3] kvm: use kmalloc() instead of kzalloc() during  iodev register/unregister Joe Perches <joe@perches.com> - 2015-08-26 07:50 +0200
          Re: [PATCH V2 1/3] kvm: use kmalloc() instead of kzalloc() during  iodev register/unregister Jason Wang <jasowang@redhat.com> - 2015-08-26 07:50 +0200

#1212777 — [PATCH V2 1/3] kvm: use kmalloc() instead of kzalloc() during iodev register/unregister

All fields of kvm_io_range were initialized or copied explicitly
afterwards. So switch to use kmalloc().

Cc: Gleb Natapov <gleb@kernel.org>
Cc: Paolo Bonzini <pbonzini@redhat.com>
Cc: Michael S. Tsirkin <mst@redhat.com>
Signed-off-by: Jason Wang <jasowang@redhat.com>
---
 virt/kvm/kvm_main.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/virt/kvm/kvm_main.c b/virt/kvm/kvm_main.c
index 8b8a444..0d79fe8 100644
--- 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;
-- 
2.1.4

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [next] | [standalone]

#1213115 — 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 linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

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

#1213548 — 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 linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

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

#1213549 — 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 linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

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

#1213551 — 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 linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web