2009-01-08 13:56:19

by Eugene Teo

[permalink] [raw]

Subject: [PATCH] IB: check for memory allocation failure

---

Fix error-path NULL deref in c2_register_device().

Signed-off-by: Eugene Teo <[email protected]>
---
drivers/infiniband/hw/amso1100/c2_provider.c | 14 +++++++++-----
1 file changed, 9 insertions(+), 5 deletions(-)

diff --git a/drivers/infiniband/hw/amso1100/c2_provider.c b/drivers/infiniband/hw/amso1100/c2_provider.c
index 5119d65..8cede6c 100644
--- a/drivers/infiniband/hw/amso1100/c2_provider.c
+++ b/drivers/infiniband/hw/amso1100/c2_provider.c
@@ -780,11 +780,11 @@ int c2_register_device(struct c2_dev *dev)
/* Register pseudo network device */
dev->pseudo_netdev = c2_pseudo_netdev_init(dev);
if (!dev->pseudo_netdev)
- goto out3;
+ goto out4;

ret = register_netdev(dev->pseudo_netdev);
if (ret)
- goto out2;
+ goto out3;

pr_debug("%s:%u\n", __func__, __LINE__);
strlcpy(dev->ibdev.name, "amso%d", IB_DEVICE_NAME_MAX);
@@ -851,6 +851,8 @@ int c2_register_device(struct c2_dev *dev)
dev->ibdev.post_recv = c2_post_receive;

dev->ibdev.iwcm = kmalloc(sizeof(*dev->ibdev.iwcm), GFP_KERNEL);
+ if (!dev->ibdev.iwcm)
+ goto out2;
dev->ibdev.iwcm->add_ref = c2_add_ref;
dev->ibdev.iwcm->rem_ref = c2_rem_ref;
dev->ibdev.iwcm->get_qp = c2_get_qp;
@@ -870,15 +872,17 @@ int c2_register_device(struct c2_dev *dev)
if (ret)
goto out0;
}
- goto out3;
+ goto out4;

out0:
ib_unregister_device(&dev->ibdev);
out1:
- unregister_netdev(dev->pseudo_netdev);
+ kfree(dev->ibdev.iwcm);
out2:
- free_netdev(dev->pseudo_netdev);
+ unregister_netdev(dev->pseudo_netdev);
out3:
+ free_netdev(dev->pseudo_netdev);
+out4:
pr_debug("%s:%u ret=%d\n", __func__, __LINE__, ret);
return ret;
}

2009-01-08 14:17:47

by Niels de Vos

[permalink] [raw]

Subject: Re: [PATCH] IB: check for memory allocation failure

---

Hi Eugene,

please see below.

Cheers,
Niels


Eugene Teo wrote:
> Fix error-path NULL deref in c2_register_device().
>
> Signed-off-by: Eugene Teo <[email protected]>
> ---
> drivers/infiniband/hw/amso1100/c2_provider.c | 14 +++++++++-----
> 1 file changed, 9 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/infiniband/hw/amso1100/c2_provider.c b/drivers/infiniband/hw/amso1100/c2_provider.c
> index 5119d65..8cede6c 100644
> --- a/drivers/infiniband/hw/amso1100/c2_provider.c
> +++ b/drivers/infiniband/hw/amso1100/c2_provider.c
> @@ -780,11 +780,11 @@ int c2_register_device(struct c2_dev *dev)
> /* Register pseudo network device */
> dev->pseudo_netdev = c2_pseudo_netdev_init(dev);
> if (!dev->pseudo_netdev)
> - goto out3;
> + goto out4;
>
> ret = register_netdev(dev->pseudo_netdev);
> if (ret)
> - goto out2;
> + goto out3;
>
> pr_debug("%s:%u\n", __func__, __LINE__);
> strlcpy(dev->ibdev.name, "amso%d", IB_DEVICE_NAME_MAX);
> @@ -851,6 +851,8 @@ int c2_register_device(struct c2_dev *dev)
> dev->ibdev.post_recv = c2_post_receive;
>
> dev->ibdev.iwcm = kmalloc(sizeof(*dev->ibdev.iwcm), GFP_KERNEL);
> + if (!dev->ibdev.iwcm)
> + goto out2;

Is it not needed to set 'ret = -ENOMEM'?


> dev->ibdev.iwcm->add_ref = c2_add_ref;
> dev->ibdev.iwcm->rem_ref = c2_rem_ref;
> dev->ibdev.iwcm->get_qp = c2_get_qp;
> @@ -870,15 +872,17 @@ int c2_register_device(struct c2_dev *dev)
> if (ret)
> goto out0;
> }
> - goto out3;
> + goto out4;
>
> out0:
> ib_unregister_device(&dev->ibdev);
> out1:
> - unregister_netdev(dev->pseudo_netdev);
> + kfree(dev->ibdev.iwcm);
> out2:
> - free_netdev(dev->pseudo_netdev);
> + unregister_netdev(dev->pseudo_netdev);
> out3:
> + free_netdev(dev->pseudo_netdev);
> +out4:
> pr_debug("%s:%u ret=%d\n", __func__, __LINE__, ret);
> return ret;
> }
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to [email protected]
> More majordomo info at http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at http://www.tux.org/lkml/

---

Attachments:

signature.asc (189.00 B)
OpenPGP digital signature