[for-next] iw_cxgb4: fix fill_res_ep_entry() stack abuse

Commit Message

Steve Wise May 24, 2018, 2:56 p.m. UTC

Too many unnecessary structs on the stack.  Just allocate one.

Fixes: 13b35021c7d6 ("iw_cxgb4: provide detailed provider-specific CM_ID information")
Signed-off-by: Steve Wise <[e-mail]>
---
 drivers/infiniband/hw/cxgb4/restrack.c | 56 +++++++++++++++++++++-------------
 1 file changed, 34 insertions(+), 22 deletions(-)

Comments

Jason Gunthorpe May 24, 2018, 7:25 p.m. UTC | #1

On Thu, May 24, 2018 at 07:56:06AM -0700, Steve Wise wrote:
> Too many unnecessary structs on the stack.  Just allocate one.
> 
> Fixes: 13b35021c7d6 ("iw_cxgb4: provide detailed provider-specific CM_ID information")
> Signed-off-by: Steve Wise <[e-mail]>
> ---
>  drivers/infiniband/hw/cxgb4/restrack.c | 56 +++++++++++++++++++++-------------
>  1 file changed, 34 insertions(+), 22 deletions(-)

I squashed this into the noted commit and fixed everything up, it is
sitting in wip/jgg-for-next for now

Jason
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to [e-mail]
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Steve Wise May 24, 2018, 7:39 p.m. UTC | #2

On 5/24/2018 2:25 PM, Jason Gunthorpe wrote:
> On Thu, May 24, 2018 at 07:56:06AM -0700, Steve Wise wrote:
>> Too many unnecessary structs on the stack.  Just allocate one.
>>
>> Fixes: 13b35021c7d6 ("iw_cxgb4: provide detailed provider-specific CM_ID information")
>> Signed-off-by: Steve Wise <[e-mail]>
>> ---
>>  drivers/infiniband/hw/cxgb4/restrack.c | 56 +++++++++++++++++++++-------------
>>  1 file changed, 34 insertions(+), 22 deletions(-)
> I squashed this into the noted commit and fixed everything up, it is
> sitting in wip/jgg-for-next for now
>
> Jason

Great.  Thanks Jason.

Steve.
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to [e-mail]
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Patch

diff --git a/drivers/infiniband/hw/cxgb4/restrack.c b/drivers/infiniband/hw/cxgb4/restrack.c
index 694ea65..9a7520e 100644
--- a/drivers/infiniband/hw/cxgb4/restrack.c
+++ b/drivers/infiniband/hw/cxgb4/restrack.c
@@ -190,15 +190,21 @@  static int fill_res_qp_entry(struct sk_buff *msg,
 	return -EMSGSIZE;
 }
 
+union union_ep {
+	struct c4iw_listen_ep lep;
+	struct c4iw_ep ep;
+};
+
 static int fill_res_ep_entry(struct sk_buff *msg,
 			     struct rdma_restrack_entry *res)
 {
 	struct rdma_cm_id *cm_id = rdma_res_to_id(res);
 	struct nlattr *table_attr;
-	struct c4iw_ep_common epc, *epcp;
-	struct c4iw_listen_ep listen_ep;
+	struct c4iw_ep_common *epcp;
+	struct c4iw_listen_ep *listen_ep = NULL;
+	struct c4iw_ep *ep = NULL;
 	struct iw_cm_id *iw_cm_id;
-	struct c4iw_ep ep;
+	union union_ep *uep;
 
 	iw_cm_id = rdma_iw_cm_id(cm_id);
 	if (!iw_cm_id)
@@ -206,56 +212,62 @@  static int fill_res_ep_entry(struct sk_buff *msg,
 	epcp = (struct c4iw_ep_common *)iw_cm_id->provider_data;
 	if (!epcp)
 		return 0;
+	uep = kcalloc(1, sizeof(*uep), GFP_KERNEL);
+	if (!uep)
+		return 0;
 
 	table_attr = nla_nest_start(msg, RDMA_NLDEV_ATTR_DRIVER);
 	if (!table_attr)
-		goto err;
+		goto err_free_uep;
 
 	/* Get a consistent snapshot */
 	mutex_lock(&epcp->mutex);
 	if (epcp->state == LISTEN) {
-		listen_ep = *(struct c4iw_listen_ep *)epcp;
+		uep->lep = *(struct c4iw_listen_ep *)epcp;
 		mutex_unlock(&epcp->mutex);
-		epcp = &listen_ep.com;
+		listen_ep = &uep->lep;
+		epcp = &listen_ep->com;
 	} else {
-		ep = *(struct c4iw_ep *)epcp;
+		uep->ep = *(struct c4iw_ep *)epcp;
 		mutex_unlock(&epcp->mutex);
-		epcp = &ep.com;
+		ep = &uep->ep;
+		epcp = &ep->com;
 	}
-	epc = *epcp;
 
-	if (rdma_nl_put_driver_u32(msg, "state", epc.state))
+	if (rdma_nl_put_driver_u32(msg, "state", epcp->state))
 		goto err_cancel_table;
-	if (rdma_nl_put_driver_u64_hex(msg, "flags", epc.flags))
+	if (rdma_nl_put_driver_u64_hex(msg, "flags", epcp->flags))
 		goto err_cancel_table;
-	if (rdma_nl_put_driver_u64_hex(msg, "history", epc.history))
+	if (rdma_nl_put_driver_u64_hex(msg, "history", epcp->history))
 		goto err_cancel_table;
 
-	if (epc.state == LISTEN) {
-		if (rdma_nl_put_driver_u32(msg, "stid", listen_ep.stid))
+	if (epcp->state == LISTEN) {
+		if (rdma_nl_put_driver_u32(msg, "stid", listen_ep->stid))
 			goto err_cancel_table;
-		if (rdma_nl_put_driver_u32(msg, "backlog", listen_ep.backlog))
+		if (rdma_nl_put_driver_u32(msg, "backlog", listen_ep->backlog))
 			goto err_cancel_table;
 	} else {
-		if (rdma_nl_put_driver_u32(msg, "hwtid", ep.hwtid))
+		if (rdma_nl_put_driver_u32(msg, "hwtid", ep->hwtid))
 			goto err_cancel_table;
-		if (rdma_nl_put_driver_u32(msg, "ord", ep.ord))
+		if (rdma_nl_put_driver_u32(msg, "ord", ep->ord))
 			goto err_cancel_table;
-		if (rdma_nl_put_driver_u32(msg, "ird", ep.ird))
+		if (rdma_nl_put_driver_u32(msg, "ird", ep->ird))
 			goto err_cancel_table;
-		if (rdma_nl_put_driver_u32(msg, "emss", ep.emss))
+		if (rdma_nl_put_driver_u32(msg, "emss", ep->emss))
 			goto err_cancel_table;
 
-		if (!ep.parent_ep && rdma_nl_put_driver_u32(msg, "atid",
-							      ep.atid))
+		if (!ep->parent_ep && rdma_nl_put_driver_u32(msg, "atid",
+							     ep->atid))
 			goto err_cancel_table;
 	}
 	nla_nest_end(msg, table_attr);
+	kfree(uep);
 	return 0;
 
 err_cancel_table:
 	nla_nest_cancel(msg, table_attr);
-err:
+err_free_uep:
+	kfree(uep);
 	return -EMSGSIZE;
 }