linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH] infiniband: i40iw: fix potential NULL pointer dereferences
@ 2019-03-09  5:27 Kangjie Lu
  2019-03-12 15:01 ` Jason Gunthorpe
  2019-03-12 15:36 ` Saleem, Shiraz
  0 siblings, 2 replies; 8+ messages in thread
From: Kangjie Lu @ 2019-03-09  5:27 UTC (permalink / raw)
  To: kjlu
  Cc: pakki001, Faisal Latif, Shiraz Saleem, Doug Ledford,
	Jason Gunthorpe, linux-rdma, linux-kernel

alloc_ordered_workqueue may fail and return NULL. Let's check
its return value to ensure it is not NULL so as to avoid
potential NULL pointer dereferences.

Signed-off-by: Kangjie Lu <kjlu@umn.edu>
---
 drivers/infiniband/hw/i40iw/i40iw_cm.c | 12 ++++++++++++
 1 file changed, 12 insertions(+)

diff --git a/drivers/infiniband/hw/i40iw/i40iw_cm.c b/drivers/infiniband/hw/i40iw/i40iw_cm.c
index 206cfb0016f8..ad9b4f235e30 100644
--- a/drivers/infiniband/hw/i40iw/i40iw_cm.c
+++ b/drivers/infiniband/hw/i40iw/i40iw_cm.c
@@ -3256,9 +3256,21 @@ void i40iw_setup_cm_core(struct i40iw_device *iwdev)
 
 	cm_core->event_wq = alloc_ordered_workqueue("iwewq",
 						    WQ_MEM_RECLAIM);
+	if (!cm_core->event_wq) {
+		i40iw_debug(cm_core->dev,
+				I40IW_DEBUG_CM,
+				"%s allocate event work queue failed\n",
+				__func__);
+	}
 
 	cm_core->disconn_wq = alloc_ordered_workqueue("iwdwq",
 						      WQ_MEM_RECLAIM);
+	if (!cm_core->disconn_wq) {
+		i40iw_debug(cm_core->dev,
+				I40IW_DEBUG_CM,
+				"%s allocate disconnect work queue failed\n",
+				__func__);
+	}
 }
 
 /**
-- 
2.17.1


^ permalink raw reply related	[flat|nested] 8+ messages in thread

* Re: [PATCH] infiniband: i40iw: fix potential NULL pointer dereferences
  2019-03-09  5:27 [PATCH] infiniband: i40iw: fix potential NULL pointer dereferences Kangjie Lu
@ 2019-03-12 15:01 ` Jason Gunthorpe
  2019-03-12 15:36 ` Saleem, Shiraz
  1 sibling, 0 replies; 8+ messages in thread
From: Jason Gunthorpe @ 2019-03-12 15:01 UTC (permalink / raw)
  To: Kangjie Lu
  Cc: pakki001, Faisal Latif, Shiraz Saleem, Doug Ledford, linux-rdma,
	linux-kernel

On Fri, Mar 08, 2019 at 11:27:50PM -0600, Kangjie Lu wrote:
> alloc_ordered_workqueue may fail and return NULL. Let's check
> its return value to ensure it is not NULL so as to avoid
> potential NULL pointer dereferences.
> 
> Signed-off-by: Kangjie Lu <kjlu@umn.edu>
>  drivers/infiniband/hw/i40iw/i40iw_cm.c | 12 ++++++++++++
>  1 file changed, 12 insertions(+)
> 
> diff --git a/drivers/infiniband/hw/i40iw/i40iw_cm.c b/drivers/infiniband/hw/i40iw/i40iw_cm.c
> index 206cfb0016f8..ad9b4f235e30 100644
> +++ b/drivers/infiniband/hw/i40iw/i40iw_cm.c
> @@ -3256,9 +3256,21 @@ void i40iw_setup_cm_core(struct i40iw_device *iwdev)
>  
>  	cm_core->event_wq = alloc_ordered_workqueue("iwewq",
>  						    WQ_MEM_RECLAIM);
> +	if (!cm_core->event_wq) {
> +		i40iw_debug(cm_core->dev,
> +				I40IW_DEBUG_CM,
> +				"%s allocate event work queue failed\n",
> +				__func__);
> +	}
>  
>  	cm_core->disconn_wq = alloc_ordered_workqueue("iwdwq",
>  						      WQ_MEM_RECLAIM);
> +	if (!cm_core->disconn_wq) {
> +		i40iw_debug(cm_core->dev,
> +				I40IW_DEBUG_CM,
> +				"%s allocate disconnect work queue failed\n",
> +				__func__);
> +	}
>  }

Same no as the mlx patches - handle the error or don't bother

Jason

^ permalink raw reply	[flat|nested] 8+ messages in thread

* RE: [PATCH] infiniband: i40iw: fix potential NULL pointer dereferences
  2019-03-09  5:27 [PATCH] infiniband: i40iw: fix potential NULL pointer dereferences Kangjie Lu
  2019-03-12 15:01 ` Jason Gunthorpe
@ 2019-03-12 15:36 ` Saleem, Shiraz
  2019-03-14  5:37   ` Kangjie Lu
  1 sibling, 1 reply; 8+ messages in thread
From: Saleem, Shiraz @ 2019-03-12 15:36 UTC (permalink / raw)
  To: Kangjie Lu
  Cc: pakki001, Latif, Faisal, Doug Ledford, Jason Gunthorpe,
	linux-rdma, linux-kernel

>Subject: [PATCH] infiniband: i40iw: fix potential NULL pointer dereferences
>
>alloc_ordered_workqueue may fail and return NULL. Let's check its return value
>to ensure it is not NULL so as to avoid potential NULL pointer dereferences.
>
>Signed-off-by: Kangjie Lu <kjlu@umn.edu>
>---
> drivers/infiniband/hw/i40iw/i40iw_cm.c | 12 ++++++++++++
> 1 file changed, 12 insertions(+)
>
>diff --git a/drivers/infiniband/hw/i40iw/i40iw_cm.c
>b/drivers/infiniband/hw/i40iw/i40iw_cm.c
>index 206cfb0016f8..ad9b4f235e30 100644
>--- a/drivers/infiniband/hw/i40iw/i40iw_cm.c
>+++ b/drivers/infiniband/hw/i40iw/i40iw_cm.c
>@@ -3256,9 +3256,21 @@ void i40iw_setup_cm_core(struct i40iw_device
>*iwdev)
>
> 	cm_core->event_wq = alloc_ordered_workqueue("iwewq",
> 						    WQ_MEM_RECLAIM);
>+	if (!cm_core->event_wq) {
>+		i40iw_debug(cm_core->dev,
>+				I40IW_DEBUG_CM,
>+				"%s allocate event work queue failed\n",
>+				__func__);
>+	}
>
> 	cm_core->disconn_wq = alloc_ordered_workqueue("iwdwq",
> 						      WQ_MEM_RECLAIM);
>+	if (!cm_core->disconn_wq) {
>+		i40iw_debug(cm_core->dev,
>+				I40IW_DEBUG_CM,
>+				"%s allocate disconnect work queue failed\n",
>+				__func__);
>+	}
> }

Hi Kangjie - Just doing a debug print doesn't suffice. We need to fail the i40iw_open() and unwind correctly.

Perhaps something like this would be required.

diff --git a/drivers/infiniband/hw/i40iw/i40iw.h b/drivers/infiniband/hw/i40iw/i40iw.h
index 2f2b442..8feec35 100644
--- a/drivers/infiniband/hw/i40iw/i40iw.h
+++ b/drivers/infiniband/hw/i40iw/i40iw.h
@@ -552,7 +552,7 @@ enum i40iw_status_code i40iw_obj_aligned_mem(struct i40iw_device *iwdev,

 void i40iw_request_reset(struct i40iw_device *iwdev);
 void i40iw_destroy_rdma_device(struct i40iw_ib_device *iwibdev);
-void i40iw_setup_cm_core(struct i40iw_device *iwdev);
+int i40iw_setup_cm_core(struct i40iw_device *iwdev);
 void i40iw_cleanup_cm_core(struct i40iw_cm_core *cm_core);
 void i40iw_process_ceq(struct i40iw_device *, struct i40iw_ceq *iwceq);
 void i40iw_process_aeq(struct i40iw_device *);
diff --git a/drivers/infiniband/hw/i40iw/i40iw_cm.c b/drivers/infiniband/hw/i40iw/i40iw_cm.c
index 206cfb0..dcc5fea 100644
--- a/drivers/infiniband/hw/i40iw/i40iw_cm.c
+++ b/drivers/infiniband/hw/i40iw/i40iw_cm.c
@@ -3237,7 +3237,7 @@ void i40iw_receive_ilq(struct i40iw_sc_vsi *vsi, struct i40iw_puda_buf *rbuf)
  * core
  * @iwdev: iwarp device structure
  */
-void i40iw_setup_cm_core(struct i40iw_device *iwdev)
+int i40iw_setup_cm_core(struct i40iw_device *iwdev)
 {
        struct i40iw_cm_core *cm_core = &iwdev->cm_core;

@@ -3256,9 +3256,20 @@ void i40iw_setup_cm_core(struct i40iw_device *iwdev)

        cm_core->event_wq = alloc_ordered_workqueue("iwewq",
                                                    WQ_MEM_RECLAIM);
-
+       if (!cm_core->event_wq)
+               goto error;
+       
        cm_core->disconn_wq = alloc_ordered_workqueue("iwdwq",
                                                      WQ_MEM_RECLAIM);
+       if (!cm_core->disconn_wq)
+               goto error;
+
+       return 0;
+error:
+       i40iw_cleanup_cm_core(&iwdev->cm_core);
+       i40iw_pr_err("fail to setup CM core");
+
+       return -ENOMEM;
 }

 /**
@@ -3278,8 +3289,10 @@ void i40iw_cleanup_cm_core(struct i40iw_cm_core *cm_core)
                del_timer_sync(&cm_core->tcp_timer);
        spin_unlock_irqrestore(&cm_core->ht_lock, flags);

-       destroy_workqueue(cm_core->event_wq);
-       destroy_workqueue(cm_core->disconn_wq);
+       if (cm_core->event_wq)
+               destroy_workqueue(cm_core->event_wq);
+       if (cm_core->disconn_wq)
+               destroy_workqueue(cm_core->disconn_wq);
 }

 /**
diff --git a/drivers/infiniband/hw/i40iw/i40iw_main.c b/drivers/infiniband/hw/i40iw/i40iw_main.c
index 68095f0..10932ba 100644
--- a/drivers/infiniband/hw/i40iw/i40iw_main.c
+++ b/drivers/infiniband/hw/i40iw/i40iw_main.c
@@ -1641,7 +1641,10 @@ static int i40iw_open(struct i40e_info *ldev, struct i40e_client *client)
        iwdev = &hdl->device;
        iwdev->hdl = hdl;
        dev = &iwdev->sc_dev;
-       i40iw_setup_cm_core(iwdev);
+       if (i40iw_setup_cm_core(iwdev)) {
+               kfree(iwdev->hdl);
+               return -ENOMEM;
+       }

        dev->back_dev = (void *)iwdev;
        iwdev->ldev = &hdl->ldev;
-- 
1.8.3.1

Shiraz

^ permalink raw reply related	[flat|nested] 8+ messages in thread

* [PATCH] infiniband: i40iw: fix potential NULL pointer dereferences
  2019-03-12 15:36 ` Saleem, Shiraz
@ 2019-03-14  5:37   ` Kangjie Lu
  2019-03-14 18:31     ` Saleem, Shiraz
  0 siblings, 1 reply; 8+ messages in thread
From: Kangjie Lu @ 2019-03-14  5:37 UTC (permalink / raw)
  To: kjlu
  Cc: pakki001, Faisal Latif, Shiraz Saleem, Doug Ledford,
	Jason Gunthorpe, linux-rdma, linux-kernel

alloc_ordered_workqueue may fail and return NULL.
The fix captures the failure and handles it properly to avoid
potential NULL pointer dereferences.

Signed-off-by: Kangjie Lu <kjlu@umn.edu>
---
V2: add return value to capture the error code

 drivers/infiniband/hw/i40iw/i40iw.h      |  2 +-
 drivers/infiniband/hw/i40iw/i40iw_cm.c   | 19 ++++++++++++++++---
 drivers/infiniband/hw/i40iw/i40iw_main.c |  5 ++++-
 3 files changed, 21 insertions(+), 5 deletions(-)

diff --git a/drivers/infiniband/hw/i40iw/i40iw.h b/drivers/infiniband/hw/i40iw/i40iw.h
index 2f2b4426ded7..8feec35f95a7 100644
--- a/drivers/infiniband/hw/i40iw/i40iw.h
+++ b/drivers/infiniband/hw/i40iw/i40iw.h
@@ -552,7 +552,7 @@ enum i40iw_status_code i40iw_obj_aligned_mem(struct i40iw_device *iwdev,
 
 void i40iw_request_reset(struct i40iw_device *iwdev);
 void i40iw_destroy_rdma_device(struct i40iw_ib_device *iwibdev);
-void i40iw_setup_cm_core(struct i40iw_device *iwdev);
+int i40iw_setup_cm_core(struct i40iw_device *iwdev);
 void i40iw_cleanup_cm_core(struct i40iw_cm_core *cm_core);
 void i40iw_process_ceq(struct i40iw_device *, struct i40iw_ceq *iwceq);
 void i40iw_process_aeq(struct i40iw_device *);
diff --git a/drivers/infiniband/hw/i40iw/i40iw_cm.c b/drivers/infiniband/hw/i40iw/i40iw_cm.c
index 206cfb0016f8..dda24f44239b 100644
--- a/drivers/infiniband/hw/i40iw/i40iw_cm.c
+++ b/drivers/infiniband/hw/i40iw/i40iw_cm.c
@@ -3237,7 +3237,7 @@ void i40iw_receive_ilq(struct i40iw_sc_vsi *vsi, struct i40iw_puda_buf *rbuf)
  * core
  * @iwdev: iwarp device structure
  */
-void i40iw_setup_cm_core(struct i40iw_device *iwdev)
+int i40iw_setup_cm_core(struct i40iw_device *iwdev)
 {
 	struct i40iw_cm_core *cm_core = &iwdev->cm_core;
 
@@ -3256,9 +3256,20 @@ void i40iw_setup_cm_core(struct i40iw_device *iwdev)
 
 	cm_core->event_wq = alloc_ordered_workqueue("iwewq",
 						    WQ_MEM_RECLAIM);
+	if (!cm_core->event_wq)
+		goto error;
 
 	cm_core->disconn_wq = alloc_ordered_workqueue("iwdwq",
 						      WQ_MEM_RECLAIM);
+	if (!cm_core->disconn_wq)
+		goto error;
+
+	return 0;
+error:
+	i40iw_cleanup_cm_core(&iwdev->cm_core);
+	i40iw_pr_err("fail to setup CM core");
+
+	return return -ENOMEM;
 }
 
 /**
@@ -3278,8 +3289,10 @@ void i40iw_cleanup_cm_core(struct i40iw_cm_core *cm_core)
 		del_timer_sync(&cm_core->tcp_timer);
 	spin_unlock_irqrestore(&cm_core->ht_lock, flags);
 
-	destroy_workqueue(cm_core->event_wq);
-	destroy_workqueue(cm_core->disconn_wq);
+	if (cm_core->event_wq)
+		destroy_workqueue(cm_core->event_wq);
+	if (cm_core->disconn_wq)
+		destroy_workqueue(cm_core->disconn_wq);
 }
 
 /**
diff --git a/drivers/infiniband/hw/i40iw/i40iw_main.c b/drivers/infiniband/hw/i40iw/i40iw_main.c
index 68095f00d08f..10932baee279 100644
--- a/drivers/infiniband/hw/i40iw/i40iw_main.c
+++ b/drivers/infiniband/hw/i40iw/i40iw_main.c
@@ -1641,7 +1641,10 @@ static int i40iw_open(struct i40e_info *ldev, struct i40e_client *client)
 	iwdev = &hdl->device;
 	iwdev->hdl = hdl;
 	dev = &iwdev->sc_dev;
-	i40iw_setup_cm_core(iwdev);
+	if (i40iw_setup_cm_core(iwdev)) {
+		kfree(iwdev->hdl);
+		return -ENOMEM;
+	}
 
 	dev->back_dev = (void *)iwdev;
 	iwdev->ldev = &hdl->ldev;
-- 
2.17.1


^ permalink raw reply related	[flat|nested] 8+ messages in thread

* RE: [PATCH] infiniband: i40iw: fix potential NULL pointer dereferences
  2019-03-14  5:37   ` Kangjie Lu
@ 2019-03-14 18:31     ` Saleem, Shiraz
  2019-03-15  6:57       ` [PATCH v2] " Kangjie Lu
  0 siblings, 1 reply; 8+ messages in thread
From: Saleem, Shiraz @ 2019-03-14 18:31 UTC (permalink / raw)
  To: Kangjie Lu
  Cc: pakki001, Latif, Faisal, Doug Ledford, Jason Gunthorpe,
	linux-rdma, linux-kernel

>Subject: [PATCH] infiniband: i40iw: fix potential NULL pointer dereferences
>
>alloc_ordered_workqueue may fail and return NULL.
>The fix captures the failure and handles it properly to avoid potential NULL pointer
>dereferences.
>
>Signed-off-by: Kangjie Lu <kjlu@umn.edu>
>---
>V2: add return value to capture the error code
>
> drivers/infiniband/hw/i40iw/i40iw.h      |  2 +-
> drivers/infiniband/hw/i40iw/i40iw_cm.c   | 19 ++++++++++++++++---
> drivers/infiniband/hw/i40iw/i40iw_main.c |  5 ++++-
> 3 files changed, 21 insertions(+), 5 deletions(-)
[....]

>a/drivers/infiniband/hw/i40iw/i40iw_cm.c b/drivers/infiniband/hw/i40iw/i40iw_cm.c
>index 206cfb0016f8..dda24f44239b 100644
>--- a/drivers/infiniband/hw/i40iw/i40iw_cm.c
>+++ b/drivers/infiniband/hw/i40iw/i40iw_cm.c
>@@ -3237,7 +3237,7 @@ void i40iw_receive_ilq(struct i40iw_sc_vsi *vsi, struct
>i40iw_puda_buf *rbuf)
>  * core
>  * @iwdev: iwarp device structure
>  */
>-void i40iw_setup_cm_core(struct i40iw_device *iwdev)
>+int i40iw_setup_cm_core(struct i40iw_device *iwdev)
> {
> 	struct i40iw_cm_core *cm_core = &iwdev->cm_core;
>
>@@ -3256,9 +3256,20 @@ void i40iw_setup_cm_core(struct i40iw_device *iwdev)
>
> 	cm_core->event_wq = alloc_ordered_workqueue("iwewq",
> 						    WQ_MEM_RECLAIM);
>+	if (!cm_core->event_wq)
>+		goto error;
>
> 	cm_core->disconn_wq = alloc_ordered_workqueue("iwdwq",
> 						      WQ_MEM_RECLAIM);
>+	if (!cm_core->disconn_wq)
>+		goto error;
>+
>+	return 0;
>+error:
>+	i40iw_cleanup_cm_core(&iwdev->cm_core);
>+	i40iw_pr_err("fail to setup CM core");
>+
>+	return return -ENOMEM;

please fix :)

> }
>


^ permalink raw reply	[flat|nested] 8+ messages in thread

* [PATCH v2] infiniband: i40iw: fix potential NULL pointer dereferences
  2019-03-14 18:31     ` Saleem, Shiraz
@ 2019-03-15  6:57       ` Kangjie Lu
  2019-03-18 13:56         ` Saleem, Shiraz
  2019-03-27 13:20         ` Jason Gunthorpe
  0 siblings, 2 replies; 8+ messages in thread
From: Kangjie Lu @ 2019-03-15  6:57 UTC (permalink / raw)
  To: kjlu
  Cc: Faisal Latif, Shiraz Saleem, Doug Ledford, Jason Gunthorpe,
	linux-rdma, linux-kernel

alloc_ordered_workqueue may fail and return NULL.
The fix captures the failure and handles it properly to avoid
potential NULL pointer dereferences.

Signed-off-by: Kangjie Lu <kjlu@umn.edu>
---
V2: add return value to capture the error code
---
 drivers/infiniband/hw/i40iw/i40iw.h      |  2 +-
 drivers/infiniband/hw/i40iw/i40iw_cm.c   | 19 ++++++++++++++++---
 drivers/infiniband/hw/i40iw/i40iw_main.c |  5 ++++-
 3 files changed, 21 insertions(+), 5 deletions(-)

diff --git a/drivers/infiniband/hw/i40iw/i40iw.h b/drivers/infiniband/hw/i40iw/i40iw.h
index 2f2b4426ded7..8feec35f95a7 100644
--- a/drivers/infiniband/hw/i40iw/i40iw.h
+++ b/drivers/infiniband/hw/i40iw/i40iw.h
@@ -552,7 +552,7 @@ enum i40iw_status_code i40iw_obj_aligned_mem(struct i40iw_device *iwdev,
 
 void i40iw_request_reset(struct i40iw_device *iwdev);
 void i40iw_destroy_rdma_device(struct i40iw_ib_device *iwibdev);
-void i40iw_setup_cm_core(struct i40iw_device *iwdev);
+int i40iw_setup_cm_core(struct i40iw_device *iwdev);
 void i40iw_cleanup_cm_core(struct i40iw_cm_core *cm_core);
 void i40iw_process_ceq(struct i40iw_device *, struct i40iw_ceq *iwceq);
 void i40iw_process_aeq(struct i40iw_device *);
diff --git a/drivers/infiniband/hw/i40iw/i40iw_cm.c b/drivers/infiniband/hw/i40iw/i40iw_cm.c
index 206cfb0016f8..2e20786b9a57 100644
--- a/drivers/infiniband/hw/i40iw/i40iw_cm.c
+++ b/drivers/infiniband/hw/i40iw/i40iw_cm.c
@@ -3237,7 +3237,7 @@ void i40iw_receive_ilq(struct i40iw_sc_vsi *vsi, struct i40iw_puda_buf *rbuf)
  * core
  * @iwdev: iwarp device structure
  */
-void i40iw_setup_cm_core(struct i40iw_device *iwdev)
+int i40iw_setup_cm_core(struct i40iw_device *iwdev)
 {
 	struct i40iw_cm_core *cm_core = &iwdev->cm_core;
 
@@ -3256,9 +3256,20 @@ void i40iw_setup_cm_core(struct i40iw_device *iwdev)
 
 	cm_core->event_wq = alloc_ordered_workqueue("iwewq",
 						    WQ_MEM_RECLAIM);
+	if (!cm_core->event_wq)
+		goto error;
 
 	cm_core->disconn_wq = alloc_ordered_workqueue("iwdwq",
 						      WQ_MEM_RECLAIM);
+	if (!cm_core->disconn_wq)
+		goto error;
+
+	return 0;
+error:
+	i40iw_cleanup_cm_core(&iwdev->cm_core);
+	i40iw_pr_err("fail to setup CM core");
+
+	return -ENOMEM;
 }
 
 /**
@@ -3278,8 +3289,10 @@ void i40iw_cleanup_cm_core(struct i40iw_cm_core *cm_core)
 		del_timer_sync(&cm_core->tcp_timer);
 	spin_unlock_irqrestore(&cm_core->ht_lock, flags);
 
-	destroy_workqueue(cm_core->event_wq);
-	destroy_workqueue(cm_core->disconn_wq);
+	if (cm_core->event_wq)
+		destroy_workqueue(cm_core->event_wq);
+	if (cm_core->disconn_wq)
+		destroy_workqueue(cm_core->disconn_wq);
 }
 
 /**
diff --git a/drivers/infiniband/hw/i40iw/i40iw_main.c b/drivers/infiniband/hw/i40iw/i40iw_main.c
index 68095f00d08f..10932baee279 100644
--- a/drivers/infiniband/hw/i40iw/i40iw_main.c
+++ b/drivers/infiniband/hw/i40iw/i40iw_main.c
@@ -1641,7 +1641,10 @@ static int i40iw_open(struct i40e_info *ldev, struct i40e_client *client)
 	iwdev = &hdl->device;
 	iwdev->hdl = hdl;
 	dev = &iwdev->sc_dev;
-	i40iw_setup_cm_core(iwdev);
+	if (i40iw_setup_cm_core(iwdev)) {
+		kfree(iwdev->hdl);
+		return -ENOMEM;
+	}
 
 	dev->back_dev = (void *)iwdev;
 	iwdev->ldev = &hdl->ldev;
-- 
2.17.1


^ permalink raw reply related	[flat|nested] 8+ messages in thread

* RE: [PATCH v2] infiniband: i40iw: fix potential NULL pointer dereferences
  2019-03-15  6:57       ` [PATCH v2] " Kangjie Lu
@ 2019-03-18 13:56         ` Saleem, Shiraz
  2019-03-27 13:20         ` Jason Gunthorpe
  1 sibling, 0 replies; 8+ messages in thread
From: Saleem, Shiraz @ 2019-03-18 13:56 UTC (permalink / raw)
  To: Kangjie Lu
  Cc: Latif, Faisal, Doug Ledford, Jason Gunthorpe, linux-rdma, linux-kernel

>Subject: [PATCH v2] infiniband: i40iw: fix potential NULL pointer dereferences

Something like "RDMA/i40iw: Handle workqueue allocation failure" is a more appropriate subject line. 
>
>alloc_ordered_workqueue may fail and return NULL.
>The fix captures the failure and handles it properly to avoid potential NULL pointer
>dereferences.
>
>Signed-off-by: Kangjie Lu <kjlu@umn.edu>
>---
>V2: add return value to capture the error code
>---
> drivers/infiniband/hw/i40iw/i40iw.h      |  2 +-
> drivers/infiniband/hw/i40iw/i40iw_cm.c   | 19 ++++++++++++++++---
> drivers/infiniband/hw/i40iw/i40iw_main.c |  5 ++++-
> 3 files changed, 21 insertions(+), 5 deletions(-)
>
The patch itself looks ok to me now. Thanks!

Shiraz

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH v2] infiniband: i40iw: fix potential NULL pointer dereferences
  2019-03-15  6:57       ` [PATCH v2] " Kangjie Lu
  2019-03-18 13:56         ` Saleem, Shiraz
@ 2019-03-27 13:20         ` Jason Gunthorpe
  1 sibling, 0 replies; 8+ messages in thread
From: Jason Gunthorpe @ 2019-03-27 13:20 UTC (permalink / raw)
  To: Kangjie Lu
  Cc: Faisal Latif, Shiraz Saleem, Doug Ledford, linux-rdma, linux-kernel

On Fri, Mar 15, 2019 at 01:57:14AM -0500, Kangjie Lu wrote:
> alloc_ordered_workqueue may fail and return NULL.
> The fix captures the failure and handles it properly to avoid
> potential NULL pointer dereferences.
> 
> Signed-off-by: Kangjie Lu <kjlu@umn.edu>
> ---
> V2: add return value to capture the error code
> ---
>  drivers/infiniband/hw/i40iw/i40iw.h      |  2 +-
>  drivers/infiniband/hw/i40iw/i40iw_cm.c   | 19 ++++++++++++++++---
>  drivers/infiniband/hw/i40iw/i40iw_main.c |  5 ++++-
>  3 files changed, 21 insertions(+), 5 deletions(-)

applied to for-next thanks
 
> diff --git a/drivers/infiniband/hw/i40iw/i40iw.h b/drivers/infiniband/hw/i40iw/i40iw.h
> index 2f2b4426ded7..8feec35f95a7 100644
> --- a/drivers/infiniband/hw/i40iw/i40iw.h
> +++ b/drivers/infiniband/hw/i40iw/i40iw.h
> @@ -552,7 +552,7 @@ enum i40iw_status_code i40iw_obj_aligned_mem(struct i40iw_device *iwdev,
>  
>  void i40iw_request_reset(struct i40iw_device *iwdev);
>  void i40iw_destroy_rdma_device(struct i40iw_ib_device *iwibdev);
> -void i40iw_setup_cm_core(struct i40iw_device *iwdev);
> +int i40iw_setup_cm_core(struct i40iw_device *iwdev);
>  void i40iw_cleanup_cm_core(struct i40iw_cm_core *cm_core);
>  void i40iw_process_ceq(struct i40iw_device *, struct i40iw_ceq *iwceq);
>  void i40iw_process_aeq(struct i40iw_device *);
> diff --git a/drivers/infiniband/hw/i40iw/i40iw_cm.c b/drivers/infiniband/hw/i40iw/i40iw_cm.c
> index 206cfb0016f8..2e20786b9a57 100644
> --- a/drivers/infiniband/hw/i40iw/i40iw_cm.c
> +++ b/drivers/infiniband/hw/i40iw/i40iw_cm.c
> @@ -3237,7 +3237,7 @@ void i40iw_receive_ilq(struct i40iw_sc_vsi *vsi, struct i40iw_puda_buf *rbuf)
>   * core
>   * @iwdev: iwarp device structure
>   */
> -void i40iw_setup_cm_core(struct i40iw_device *iwdev)
> +int i40iw_setup_cm_core(struct i40iw_device *iwdev)
>  {
>  	struct i40iw_cm_core *cm_core = &iwdev->cm_core;
>  
> @@ -3256,9 +3256,20 @@ void i40iw_setup_cm_core(struct i40iw_device *iwdev)
>  
>  	cm_core->event_wq = alloc_ordered_workqueue("iwewq",
>  						    WQ_MEM_RECLAIM);
> +	if (!cm_core->event_wq)
> +		goto error;
>  
>  	cm_core->disconn_wq = alloc_ordered_workqueue("iwdwq",
>  						      WQ_MEM_RECLAIM);
> +	if (!cm_core->disconn_wq)
> +		goto error;
> +
> +	return 0;
> +error:
> +	i40iw_cleanup_cm_core(&iwdev->cm_core);
> +	i40iw_pr_err("fail to setup CM core");

But I deleted this print, memory allocation failures already print
enough

Jason

^ permalink raw reply	[flat|nested] 8+ messages in thread

end of thread, other threads:[~2019-03-27 13:20 UTC | newest]

Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-03-09  5:27 [PATCH] infiniband: i40iw: fix potential NULL pointer dereferences Kangjie Lu
2019-03-12 15:01 ` Jason Gunthorpe
2019-03-12 15:36 ` Saleem, Shiraz
2019-03-14  5:37   ` Kangjie Lu
2019-03-14 18:31     ` Saleem, Shiraz
2019-03-15  6:57       ` [PATCH v2] " Kangjie Lu
2019-03-18 13:56         ` Saleem, Shiraz
2019-03-27 13:20         ` Jason Gunthorpe

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).