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

Groups > linux.kernel > #1431671 > unrolled thread

[PATCH] virtio: Return correct errno for function init_vq's failure

Back to article view | Back to linux.kernel

Contents

  [PATCH] virtio: Return correct errno for function init_vq's failure Minfei Huang <mnghuan@gmail.com> - 2016-06-27 04:10 +0200
    Re: [PATCH] virtio: Return correct errno for function init_vq's  failure Cornelia Huck <cornelia.huck@de.ibm.com> - 2016-07-06 11:20 +0200

#1431671 — [PATCH] virtio: Return correct errno for function init_vq's failure

The error number -ENOENT or 0 will be returned, if we can not allocate
more memory in function init_vq. If host can support multiple virtual
queues, and we fails to allocate necessary memory structures for vq,
kernel may crash due to incorrect returning.

To fix it, kernel will return correct value in init_vq.

Signed-off-by: Minfei Huang <mnghuan@gmail.com>
Signed-off-by: Minfei Huang <minfei.hmf@alibaba-inc.com>
---
 drivers/block/virtio_blk.c | 5 ++---
 1 file changed, 2 insertions(+), 3 deletions(-)

diff --git a/drivers/block/virtio_blk.c b/drivers/block/virtio_blk.c
index 42758b5..40ecb2b 100644
--- a/drivers/block/virtio_blk.c
+++ b/drivers/block/virtio_blk.c
@@ -393,11 +393,10 @@ static int init_vq(struct virtio_blk *vblk)
 	if (err)
 		num_vqs = 1;
 
+	err = -ENOMEM;
 	vblk->vqs = kmalloc(sizeof(*vblk->vqs) * num_vqs, GFP_KERNEL);
-	if (!vblk->vqs) {
-		err = -ENOMEM;
+	if (!vblk->vqs)
 		goto out;
-	}
 
 	names = kmalloc(sizeof(*names) * num_vqs, GFP_KERNEL);
 	if (!names)
-- 
2.7.4 (Apple Git-66)

[toc] | [next] | [standalone]

#1437583 — Re: [PATCH] virtio: Return correct errno for function init_vq's failure

On Mon, 27 Jun 2016 10:09:18 +0800
Minfei Huang <mnghuan@gmail.com> wrote:

> The error number -ENOENT or 0 will be returned, if we can not allocate
> more memory in function init_vq. If host can support multiple virtual
> queues, and we fails to allocate necessary memory structures for vq,
> kernel may crash due to incorrect returning.
> 
> To fix it, kernel will return correct value in init_vq.
> 
> Signed-off-by: Minfei Huang <mnghuan@gmail.com>
> Signed-off-by: Minfei Huang <minfei.hmf@alibaba-inc.com>
> ---
>  drivers/block/virtio_blk.c | 5 ++---
>  1 file changed, 2 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/block/virtio_blk.c b/drivers/block/virtio_blk.c
> index 42758b5..40ecb2b 100644
> --- a/drivers/block/virtio_blk.c
> +++ b/drivers/block/virtio_blk.c
> @@ -393,11 +393,10 @@ static int init_vq(struct virtio_blk *vblk)
>  	if (err)
>  		num_vqs = 1;
> 
> +	err = -ENOMEM;
>  	vblk->vqs = kmalloc(sizeof(*vblk->vqs) * num_vqs, GFP_KERNEL);
> -	if (!vblk->vqs) {
> -		err = -ENOMEM;
> +	if (!vblk->vqs)
>  		goto out;
> -	}
> 
>  	names = kmalloc(sizeof(*names) * num_vqs, GFP_KERNEL);
>  	if (!names)

The error handling in this function looks horrible.

When mq was introduced, init_vq started mixing up several things:
- The mq feature is not available - which is not an error, and
therefore should not have any influence on the return code.
- One of the several memory allocations failed - only ->vqs gets
special treatment, however.
- The ->find_vqs callback failed.

Your patch fixes the code, but it is still very convoluted due to the
temporary arrays.

May it be worthwile to introduce a helper for setting up the virtqueues
where all virtqueues are essentially the same and just get a
consecutive number? Michael?

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web