[PATCH RE-RESEND V2 for-next 0/5] IB core fixes 29-Jan-2015

[PATCH RE-RESEND V2 for-next 0/5] IB core fixes 29-Jan-2015

[Or Gerlitz]

-- re-resent due to Gerrit IDs that were left by mistake, sorry for spamming you over (and over)

Hi Roland,

This is a batch with small IB core fixes, please apply for 3.20

There's one fix which touches few HW drivers to make sure
they invoke ib_unregister_mad_agent() only for valid cases
in their error flows.

There's a pending CMA patch we sent for 3.19 and was approved
by Sean, please pick it too "RDMA/CMA: Mark IPv4 addresses correctly 
when the listener is IPv6" https://patchwork.kernel.org/patch/5298971/

Majs and Or.

Ilya Nelkenbaum (1):
  IB/core: When marshaling ucma path from user-space, clear unused fields

Majd Dibbiny (3):
  IB/core: Fixes in ib_sa_add_one error flow
  IB/core: Call ib_unregister_mad_agent() only for valid agents
  IB/core: Make sure that the PSN does not overflow

Moshe Lazer (1):
  IB/core: Fix deadlock on uverbs modify_qp error flow

 drivers/infiniband/core/cma.c           |    1 +
 drivers/infiniband/core/sa_query.c      |   10 +++++++---
 drivers/infiniband/core/ucma.c          |    3 +++
 drivers/infiniband/core/uverbs_cmd.c    |   13 +++++++------
 drivers/infiniband/hw/mlx4/mad.c        |    2 +-
 drivers/infiniband/hw/mthca/mthca_mad.c |    2 +-
 drivers/infiniband/hw/qib/qib_mad.c     |    2 +-
 7 files changed, 21 insertions(+), 12 deletions(-)

--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH RE-RESEND V2 for-next 1/5] IB/core: When marshaling ucma path from user-space, clear unused fields

[Or Gerlitz]

From: Ilya Nelkenbaum <ilyan-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>

When marsheling a user path to the kernel struct ib_sa_path, need
to zero smac, dmac and set the vlan id to the "no vlan" value.

This is to ensure that Ethernet attributes are not used with
InfiniBand QPs.

Fixes: dd5f03beb4f7 ("IB/core: Ethernet L2 attributes in verbs/cm structures")
Signed-off-by: Ilya Nelkenbaum <ilyan-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>
Signed-off-by: Or Gerlitz <ogerlitz-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>
---
 drivers/infiniband/core/ucma.c |    3 +++
 1 files changed, 3 insertions(+), 0 deletions(-)

diff --git a/drivers/infiniband/core/ucma.c b/drivers/infiniband/core/ucma.c
index 56a4b7c..45d67e9 100644
--- a/drivers/infiniband/core/ucma.c
+++ b/drivers/infiniband/core/ucma.c
@@ -1124,6 +1124,9 @@ static int ucma_set_ib_path(struct ucma_context *ctx,
 	if (!optlen)
 		return -EINVAL;
 
+	memset(&sa_path, 0, sizeof(sa_path));
+	sa_path.vlan_id = 0xffff;
+
 	ib_sa_unpack_path(path_data->path_rec, &sa_path);
 	ret = rdma_set_ib_paths(ctx->cm_id, &sa_path, 1);
 	if (ret)
-- 
1.7.1

--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH RE-RESEND V2 for-next 2/5] IB/core: Fixes in ib_sa_add_one error flow

[Or Gerlitz]

From: Majd Dibbiny <majd-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>

Fixed off-by-one bug, we need to decrement the port number only after we
released the resources to the current port.

Call ib_unregister_mad_agent only for cases where ib_register_mad_agent succeeded.

Separate the ib_register_event_handler() call error flow from the loop error
flow. If the call to ib_register_event_handler fails, the client data must be
reset to NULL, (in case at some point ib_register_event_handler() is modified so
that it may return a non-zero (error) value).

Signed-off-by: Majd Dibbiny <majd-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>
Signed-off-by: Or Gerlitz <ogerlitz-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>
---
 drivers/infiniband/core/sa_query.c |   10 +++++++---
 1 files changed, 7 insertions(+), 3 deletions(-)

diff --git a/drivers/infiniband/core/sa_query.c b/drivers/infiniband/core/sa_query.c
index c38f030..80461af 100644
--- a/drivers/infiniband/core/sa_query.c
+++ b/drivers/infiniband/core/sa_query.c
@@ -1202,7 +1202,7 @@ static void ib_sa_add_one(struct ib_device *device)
 
 	INIT_IB_EVENT_HANDLER(&sa_dev->event_handler, device, ib_sa_event);
 	if (ib_register_event_handler(&sa_dev->event_handler))
-		goto err;
+		goto reg_err;
 
 	for (i = 0; i <= e - s; ++i)
 		if (rdma_port_get_link_layer(device, i + 1) == IB_LINK_LAYER_INFINIBAND)
@@ -1210,9 +1210,13 @@ static void ib_sa_add_one(struct ib_device *device)
 
 	return;
 
+reg_err:
+	ib_set_client_data(device, &sa_client, NULL);
+	i = e - s;
 err:
-	while (--i >= 0)
-		if (rdma_port_get_link_layer(device, i + 1) == IB_LINK_LAYER_INFINIBAND)
+	for (; i >= 0; --i)
+		if (rdma_port_get_link_layer(device, i + 1) == IB_LINK_LAYER_INFINIBAND &&
+		    !IS_ERR(sa_dev->port[i].agent))
 			ib_unregister_mad_agent(sa_dev->port[i].agent);
 
 	kfree(sa_dev);
-- 
1.7.1

--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH RE-RESEND V2 for-next 3/5] IB/core: Call ib_unregister_mad_agent() only for valid agents

[Or Gerlitz]

From: Majd Dibbiny <majd-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>

In some error flows, ib_mad_unregister_agent is being invoked also in cases where
the ib_mad_register_agent call failed (resulting in an illegal pointer in the
agent field).  This causes a kernel crash in the error flow.

Fix this by calling ib_unregister_mad_agent only for cases where
ib_register_mad_agent succeeded.

Signed-off-by: Majd Dibbiny <majd-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>
Signed-off-by: Or Gerlitz <ogerlitz-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>
---
 drivers/infiniband/hw/mlx4/mad.c        |    2 +-
 drivers/infiniband/hw/mthca/mthca_mad.c |    2 +-
 drivers/infiniband/hw/qib/qib_mad.c     |    2 +-
 3 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/infiniband/hw/mlx4/mad.c b/drivers/infiniband/hw/mlx4/mad.c
index 82a7dd8..6be0d2c 100644
--- a/drivers/infiniband/hw/mlx4/mad.c
+++ b/drivers/infiniband/hw/mlx4/mad.c
@@ -907,7 +907,7 @@ int mlx4_ib_mad_init(struct mlx4_ib_dev *dev)
 err:
 	for (p = 0; p < dev->num_ports; ++p)
 		for (q = 0; q <= 1; ++q)
-			if (dev->send_agent[p][q])
+			if (!IS_ERR(dev->send_agent[p][q]))
 				ib_unregister_mad_agent(dev->send_agent[p][q]);
 
 	return ret;
diff --git a/drivers/infiniband/hw/mthca/mthca_mad.c b/drivers/infiniband/hw/mthca/mthca_mad.c
index 8881fa3..5f1a7ce 100644
--- a/drivers/infiniband/hw/mthca/mthca_mad.c
+++ b/drivers/infiniband/hw/mthca/mthca_mad.c
@@ -317,7 +317,7 @@ int mthca_create_agents(struct mthca_dev *dev)
 err:
 	for (p = 0; p < dev->limits.num_ports; ++p)
 		for (q = 0; q <= 1; ++q)
-			if (dev->send_agent[p][q])
+			if (!IS_ERR(dev->send_agent[p][q]))
 				ib_unregister_mad_agent(dev->send_agent[p][q]);
 
 	return ret;
diff --git a/drivers/infiniband/hw/qib/qib_mad.c b/drivers/infiniband/hw/qib/qib_mad.c
index 636be11..6c7cc80 100644
--- a/drivers/infiniband/hw/qib/qib_mad.c
+++ b/drivers/infiniband/hw/qib/qib_mad.c
@@ -2499,7 +2499,7 @@ int qib_create_agents(struct qib_ibdev *dev)
 err:
 	for (p = 0; p < dd->num_pports; p++) {
 		ibp = &dd->pport[p].ibport_data;
-		if (ibp->send_agent) {
+		if (!IS_ERR(ibp->send_agent)) {
 			agent = ibp->send_agent;
 			ibp->send_agent = NULL;
 			ib_unregister_mad_agent(agent);
-- 
1.7.1

--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH RE-RESEND V2 for-next 4/5] IB/core: Make sure that the PSN does not overflow

[Or Gerlitz]

From: Majd Dibbiny <majd-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>

The rq/sq->psn is 24 bits as defined in the IB spec, therefore ULPs and User
space applications shouldn't use the 8 most significant bits in the 32 bits
variables to avoid overflow in modify_qp.

Fixed the PSN generation by the RDMA-CM to mask out the 8 most significant bits,
also mask out these bits in uverbs for attributes provided by user-space.

Signed-off-by: Majd Dibbiny <majd-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>
Signed-off-by: Or Gerlitz <ogerlitz-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>
---
 drivers/infiniband/core/cma.c        |    1 +
 drivers/infiniband/core/uverbs_cmd.c |    4 ++--
 2 files changed, 3 insertions(+), 2 deletions(-)

diff --git a/drivers/infiniband/core/cma.c b/drivers/infiniband/core/cma.c
index d570030..fab0ee5 100644
--- a/drivers/infiniband/core/cma.c
+++ b/drivers/infiniband/core/cma.c
@@ -512,6 +512,7 @@ struct rdma_cm_id *rdma_create_id(rdma_cm_event_handler event_handler,
 	INIT_LIST_HEAD(&id_priv->listen_list);
 	INIT_LIST_HEAD(&id_priv->mc_list);
 	get_random_bytes(&id_priv->seq_num, sizeof id_priv->seq_num);
+	id_priv->seq_num &= 0xffffff;
 
 	return &id_priv->id;
 }
diff --git a/drivers/infiniband/core/uverbs_cmd.c b/drivers/infiniband/core/uverbs_cmd.c
index 532d8eb..ecb6430 100644
--- a/drivers/infiniband/core/uverbs_cmd.c
+++ b/drivers/infiniband/core/uverbs_cmd.c
@@ -2053,8 +2053,8 @@ ssize_t ib_uverbs_modify_qp(struct ib_uverbs_file *file,
 	attr->path_mtu 		  = cmd.path_mtu;
 	attr->path_mig_state 	  = cmd.path_mig_state;
 	attr->qkey 		  = cmd.qkey;
-	attr->rq_psn 		  = cmd.rq_psn;
-	attr->sq_psn 		  = cmd.sq_psn;
+	attr->rq_psn              = cmd.rq_psn & 0xffffff;
+	attr->sq_psn              = cmd.sq_psn & 0xffffff;
 	attr->dest_qp_num 	  = cmd.dest_qp_num;
 	attr->qp_access_flags 	  = cmd.qp_access_flags;
 	attr->pkey_index 	  = cmd.pkey_index;
-- 
1.7.1

--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH RE-RESEND V2 for-next 5/5] IB/core: Fix deadlock on uverbs modify_qp error flow

[Or Gerlitz]

From: Moshe Lazer <moshel-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>

The deadlock occurs on __uverbs_modify_qp, we take a lock (idr_read_qp)
and in case of failure in ib_resolve_eth_l2_attrs we don't release
it (put_qp_read). Fix that.

Issue: 355606
Fixes: ed4c54e5b4ba ("IB/core: Resolve Ethernet L2 addresses when modifying QP")
Signed-off-by: Moshe Lazer <moshel-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>
Signed-off-by: Or Gerlitz <ogerlitz-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>
---
 drivers/infiniband/core/uverbs_cmd.c |    9 +++++----
 1 files changed, 5 insertions(+), 4 deletions(-)

diff --git a/drivers/infiniband/core/uverbs_cmd.c b/drivers/infiniband/core/uverbs_cmd.c
index ecb6430..a602ce9 100644
--- a/drivers/infiniband/core/uverbs_cmd.c
+++ b/drivers/infiniband/core/uverbs_cmd.c
@@ -2097,20 +2097,21 @@ ssize_t ib_uverbs_modify_qp(struct ib_uverbs_file *file,
 	if (qp->real_qp == qp) {
 		ret = ib_resolve_eth_l2_attrs(qp, attr, &cmd.attr_mask);
 		if (ret)
-			goto out;
+			goto release_qp;
 		ret = qp->device->modify_qp(qp, attr,
 			modify_qp_mask(qp->qp_type, cmd.attr_mask), &udata);
 	} else {
 		ret = ib_modify_qp(qp, attr, modify_qp_mask(qp->qp_type, cmd.attr_mask));
 	}
 
-	put_qp_read(qp);
-
 	if (ret)
-		goto out;
+		goto release_qp;
 
 	ret = in_len;
 
+release_qp:
+	put_qp_read(qp);
+
 out:
 	kfree(attr);
 
-- 
1.7.1

--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

RE: [PATCH RE-RESEND V2 for-next 4/5] IB/core: Make sure that the PSN does not overflow

[Weiny, Ira]

> Subject: [PATCH RE-RESEND V2 for-next 4/5] IB/core: Make sure that the PSN
> does not overflow

Is there a particular bug  which this fixes?

Ira

> 
> From: Majd Dibbiny <majd-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>
> 
> The rq/sq->psn is 24 bits as defined in the IB spec, therefore ULPs and User
> space applications shouldn't use the 8 most significant bits in the 32 bits
> variables to avoid overflow in modify_qp.
> 
> Fixed the PSN generation by the RDMA-CM to mask out the 8 most significant
> bits, also mask out these bits in uverbs for attributes provided by user-space.
> 
> Signed-off-by: Majd Dibbiny <majd-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>
> Signed-off-by: Or Gerlitz <ogerlitz-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>
> ---
>  drivers/infiniband/core/cma.c        |    1 +
>  drivers/infiniband/core/uverbs_cmd.c |    4 ++--
>  2 files changed, 3 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/infiniband/core/cma.c b/drivers/infiniband/core/cma.c
> index d570030..fab0ee5 100644
> --- a/drivers/infiniband/core/cma.c
> +++ b/drivers/infiniband/core/cma.c
> @@ -512,6 +512,7 @@ struct rdma_cm_id
> *rdma_create_id(rdma_cm_event_handler event_handler,
>  	INIT_LIST_HEAD(&id_priv->listen_list);
>  	INIT_LIST_HEAD(&id_priv->mc_list);
>  	get_random_bytes(&id_priv->seq_num, sizeof id_priv->seq_num);
> +	id_priv->seq_num &= 0xffffff;
> 
>  	return &id_priv->id;
>  }
> diff --git a/drivers/infiniband/core/uverbs_cmd.c
> b/drivers/infiniband/core/uverbs_cmd.c
> index 532d8eb..ecb6430 100644
> --- a/drivers/infiniband/core/uverbs_cmd.c
> +++ b/drivers/infiniband/core/uverbs_cmd.c
> @@ -2053,8 +2053,8 @@ ssize_t ib_uverbs_modify_qp(struct ib_uverbs_file
> *file,
>  	attr->path_mtu 		  = cmd.path_mtu;
>  	attr->path_mig_state 	  = cmd.path_mig_state;
>  	attr->qkey 		  = cmd.qkey;
> -	attr->rq_psn 		  = cmd.rq_psn;
> -	attr->sq_psn 		  = cmd.sq_psn;
> +	attr->rq_psn              = cmd.rq_psn & 0xffffff;
> +	attr->sq_psn              = cmd.sq_psn & 0xffffff;
>  	attr->dest_qp_num 	  = cmd.dest_qp_num;
>  	attr->qp_access_flags 	  = cmd.qp_access_flags;
>  	attr->pkey_index 	  = cmd.pkey_index;
> --
> 1.7.1
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-rdma" in the body
> of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org More majordomo info at
> http://vger.kernel.org/majordomo-info.html
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH RE-RESEND V2 for-next 4/5] IB/core: Make sure that the PSN does not overflow

[Or Gerlitz]

On Fri, Feb 6, 2015 at 12:27 AM, Weiny, Ira <ira.weiny-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org> wrote:
>> Subject: [PATCH RE-RESEND V2 for-next 4/5] IB/core: Make sure that the PSN
>> does not overflow
>
> Is there a particular bug  which this fixes?

Not something that I am aware too (but we can take a look @ our
regression data-base - Majd, please do), more of maintenance fix.
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH RE-RESEND V2 for-next 5/5] IB/core: Fix deadlock on uverbs modify_qp error flow

[Yann Droneaud]

Le jeudi 05 février 2015 à 13:53 +0200, Or Gerlitz a écrit :
> From: Moshe Lazer <moshel-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>
> 
> The deadlock occurs on __uverbs_modify_qp, we take a lock (idr_read_qp)
> and in case of failure in ib_resolve_eth_l2_attrs we don't release
> it (put_qp_read). Fix that.
> 
> Issue: 355606
 ^^^^^^^^^^^^^^
I'm not sure this is relevant

> Fixes: ed4c54e5b4ba ("IB/core: Resolve Ethernet L2 addresses when modifying QP")
> Signed-off-by: Moshe Lazer <moshel-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>
> Signed-off-by: Or Gerlitz <ogerlitz-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>
> ---
>  drivers/infiniband/core/uverbs_cmd.c |    9 +++++----
>  1 files changed, 5 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/infiniband/core/uverbs_cmd.c b/drivers/infiniband/core/uverbs_cmd.c
> index ecb6430..a602ce9 100644
> --- a/drivers/infiniband/core/uverbs_cmd.c
> +++ b/drivers/infiniband/core/uverbs_cmd.c
> @@ -2097,20 +2097,21 @@ ssize_t ib_uverbs_modify_qp(struct ib_uverbs_file *file,
>  	if (qp->real_qp == qp) {
>  		ret = ib_resolve_eth_l2_attrs(qp, attr, &cmd.attr_mask);
>  		if (ret)
> -			goto out;
> +			goto release_qp;
>  		ret = qp->device->modify_qp(qp, attr,
>  			modify_qp_mask(qp->qp_type, cmd.attr_mask), &udata);
>  	} else {
>  		ret = ib_modify_qp(qp, attr, modify_qp_mask(qp->qp_type, cmd.attr_mask));
>  	}
>  
> -	put_qp_read(qp);
> -
>  	if (ret)
> -		goto out;
> +		goto release_qp;
>  
>  	ret = in_len;
>  
> +release_qp:
> +	put_qp_read(qp);
> +
>  out:
>  	kfree(attr);
>  

Regards.

-- 
Yann Droneaud
OPTEYA


--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH RE-RESEND V2 for-next 4/5] IB/core: Make sure that the PSN does not overflow

[Yann Droneaud]

Hi,

Le jeudi 05 février 2015 à 13:53 +0200, Or Gerlitz a écrit :
> From: Majd Dibbiny <majd-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>
> 
> The rq/sq->psn is 24 bits as defined in the IB spec, therefore ULPs and User
> space applications shouldn't use the 8 most significant bits in the 32 bits
> variables to avoid overflow in modify_qp.
> 
> Fixed the PSN generation by the RDMA-CM to mask out the 8 most significant bits,
> also mask out these bits in uverbs for attributes provided by user-space.
> 
> Signed-off-by: Majd Dibbiny <majd-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>
> Signed-off-by: Or Gerlitz <ogerlitz-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>
> ---
>  drivers/infiniband/core/cma.c        |    1 +
>  drivers/infiniband/core/uverbs_cmd.c |    4 ++--
>  2 files changed, 3 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/infiniband/core/cma.c b/drivers/infiniband/core/cma.c
> index d570030..fab0ee5 100644
> --- a/drivers/infiniband/core/cma.c
> +++ b/drivers/infiniband/core/cma.c
> @@ -512,6 +512,7 @@ struct rdma_cm_id *rdma_create_id(rdma_cm_event_handler event_handler,
>  	INIT_LIST_HEAD(&id_priv->listen_list);
>  	INIT_LIST_HEAD(&id_priv->mc_list);
>  	get_random_bytes(&id_priv->seq_num, sizeof id_priv->seq_num);
> +	id_priv->seq_num &= 0xffffff;
>  
>  	return &id_priv->id;
>  }
> diff --git a/drivers/infiniband/core/uverbs_cmd.c b/drivers/infiniband/core/uverbs_cmd.c
> index 532d8eb..ecb6430 100644
> --- a/drivers/infiniband/core/uverbs_cmd.c
> +++ b/drivers/infiniband/core/uverbs_cmd.c
> @@ -2053,8 +2053,8 @@ ssize_t ib_uverbs_modify_qp(struct ib_uverbs_file *file,
>  	attr->path_mtu 		  = cmd.path_mtu;
>  	attr->path_mig_state 	  = cmd.path_mig_state;
>  	attr->qkey 		  = cmd.qkey;
> -	attr->rq_psn 		  = cmd.rq_psn;
> -	attr->sq_psn 		  = cmd.sq_psn;
> +	attr->rq_psn              = cmd.rq_psn & 0xffffff;
> +	attr->sq_psn              = cmd.sq_psn & 0xffffff;
>  	attr->dest_qp_num 	  = cmd.dest_qp_num;
>  	attr->qp_access_flags 	  = cmd.qp_access_flags;
>  	attr->pkey_index 	  = cmd.pkey_index;

0xffffff could be made a #define

Question: userspace is allowed to ask for a PSN on 32bits, but it will 
be silently truncated, is it going to puzzle applications ?

Anyway, it would have been better to return an error in
the first place ... not sure if we can do it now ...

Regards.

-- 
Yann Droneaud
OPTEYA


--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH RE-RESEND V2 for-next 3/5] IB/core: Call ib_unregister_mad_agent() only for valid agents

[Yann Droneaud]

Le jeudi 05 février 2015 à 13:53 +0200, Or Gerlitz a écrit :
> From: Majd Dibbiny <majd-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>
> 
> In some error flows, ib_mad_unregister_agent is being invoked also in cases where
                      ^^^^^^^^^^^^^^^^^^^^^^^
                      ib_unregister_mad_agent()

> the ib_mad_register_agent call failed (resulting in an illegal pointer in the
     ^^^^^^^^^^^^^^^^^^^^^
     ib_register_mad_agent()

> agent field).  This causes a kernel crash in the error flow.
> 
> Fix this by calling ib_unregister_mad_agent only for cases where
> ib_register_mad_agent succeeded.
> 

The code was checking for struct ib_mad_agent *agent not being NULL, 
while ib_register_mad_agent() returns a ERR_PTR() in case of error
... bad :(

> Signed-off-by: Majd Dibbiny <majd-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>
> Signed-off-by: Or Gerlitz <ogerlitz-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>
> ---
>  drivers/infiniband/hw/mlx4/mad.c        |    2 +-
>  drivers/infiniband/hw/mthca/mthca_mad.c |    2 +-
>  drivers/infiniband/hw/qib/qib_mad.c     |    2 +-
>  3 files changed, 3 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/infiniband/hw/mlx4/mad.c b/drivers/infiniband/hw/mlx4/mad.c
> index 82a7dd8..6be0d2c 100644
> --- a/drivers/infiniband/hw/mlx4/mad.c
> +++ b/drivers/infiniband/hw/mlx4/mad.c
> @@ -907,7 +907,7 @@ int mlx4_ib_mad_init(struct mlx4_ib_dev *dev)
>  err:
>  	for (p = 0; p < dev->num_ports; ++p)
>  		for (q = 0; q <= 1; ++q)
> -			if (dev->send_agent[p][q])
> +			if (!IS_ERR(dev->send_agent[p][q]))
>  				ib_unregister_mad_agent(dev->send_agent[p][q]);
>  
>  	return ret;
> diff --git a/drivers/infiniband/hw/mthca/mthca_mad.c b/drivers/infiniband/hw/mthca/mthca_mad.c
> index 8881fa3..5f1a7ce 100644
> --- a/drivers/infiniband/hw/mthca/mthca_mad.c
> +++ b/drivers/infiniband/hw/mthca/mthca_mad.c
> @@ -317,7 +317,7 @@ int mthca_create_agents(struct mthca_dev *dev)
>  err:
>  	for (p = 0; p < dev->limits.num_ports; ++p)
>  		for (q = 0; q <= 1; ++q)
> -			if (dev->send_agent[p][q])
> +			if (!IS_ERR(dev->send_agent[p][q]))
>  				ib_unregister_mad_agent(dev->send_agent[p][q]);
>  
>  	return ret;
> diff --git a/drivers/infiniband/hw/qib/qib_mad.c b/drivers/infiniband/hw/qib/qib_mad.c
> index 636be11..6c7cc80 100644
> --- a/drivers/infiniband/hw/qib/qib_mad.c
> +++ b/drivers/infiniband/hw/qib/qib_mad.c
> @@ -2499,7 +2499,7 @@ int qib_create_agents(struct qib_ibdev *dev)
>  err:
>  	for (p = 0; p < dd->num_pports; p++) {
>  		ibp = &dd->pport[p].ibport_data;
> -		if (ibp->send_agent) {
> +		if (!IS_ERR(ibp->send_agent)) {
>  			agent = ibp->send_agent;
>  			ibp->send_agent = NULL;
>  			ib_unregister_mad_agent(agent);

Regards.

-- 
Yann Droneaud
OPTEYA


--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

RE: [PATCH RE-RESEND V2 for-next 4/5] IB/core: Make sure that the PSN does not overflow

[Weiny, Ira]

> 
> 0xffffff could be made a #define
> 
> Question: userspace is allowed to ask for a PSN on 32bits, but it will be silently
> truncated, is it going to puzzle applications ?
> 
> Anyway, it would have been better to return an error in the first place ... not
> sure if we can do it now ...
> 

Yes I'm still not sure what bug this fixes.  Or did you ever find out?

Ira
 

Re: [PATCH RE-RESEND V2 for-next 3/5] IB/core: Call ib_unregister_mad_agent() only for valid agents

[Yann Droneaud]

Hi,

Le lundi 16 février 2015 à 22:28 +0100, Yann Droneaud a écrit :
> Le jeudi 05 février 2015 à 13:53 +0200, Or Gerlitz a écrit :
> > From: Majd Dibbiny <majd-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>
> > 
> > In some error flows, ib_mad_unregister_agent is being invoked also in cases where
>                       ^^^^^^^^^^^^^^^^^^^^^^^
>                       ib_unregister_mad_agent()
> 
> > the ib_mad_register_agent call failed (resulting in an illegal pointer in the
>      ^^^^^^^^^^^^^^^^^^^^^
>      ib_register_mad_agent()
> 
> > agent field).  This causes a kernel crash in the error flow.
> > 
> > Fix this by calling ib_unregister_mad_agent only for cases where
> > ib_register_mad_agent succeeded.
> > 
> 
> The code was checking for struct ib_mad_agent *agent not being NULL, 
> while ib_register_mad_agent() returns a ERR_PTR() in case of error
> ... bad :(
> 

After reading Jason's comments [1], I don't understand the purpose of
this patch. How can an ERR_PTR() value be present in the arrays ?

A quick review makes me think that mthca and mlx4 error paths lacks NULL
assignment after calling ib_unregister_mad_agent(). In other words, qib
might be correct while the others should be fixed.

[1] http://mid.gmane.org/20150205174337.GB31711-ePGOBjL8dl3ta4EC/59zMFaTQe2KTcn/@public.gmane.org

> > Signed-off-by: Majd Dibbiny <majd-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>
> > Signed-off-by: Or Gerlitz <ogerlitz-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>
> > ---
> >  drivers/infiniband/hw/mlx4/mad.c        |    2 +-
> >  drivers/infiniband/hw/mthca/mthca_mad.c |    2 +-
> >  drivers/infiniband/hw/qib/qib_mad.c     |    2 +-
> >  3 files changed, 3 insertions(+), 3 deletions(-)
> > 
> > diff --git a/drivers/infiniband/hw/mlx4/mad.c b/drivers/infiniband/hw/mlx4/mad.c
> > index 82a7dd8..6be0d2c 100644
> > --- a/drivers/infiniband/hw/mlx4/mad.c
> > +++ b/drivers/infiniband/hw/mlx4/mad.c
> > @@ -907,7 +907,7 @@ int mlx4_ib_mad_init(struct mlx4_ib_dev *dev)
> >  err:
> >  	for (p = 0; p < dev->num_ports; ++p)
> >  		for (q = 0; q <= 1; ++q)
> > -			if (dev->send_agent[p][q])
> > +			if (!IS_ERR(dev->send_agent[p][q]))
> >  				ib_unregister_mad_agent(dev->send_agent[p][q]);
> >  
> >  	return ret;
> > diff --git a/drivers/infiniband/hw/mthca/mthca_mad.c b/drivers/infiniband/hw/mthca/mthca_mad.c
> > index 8881fa3..5f1a7ce 100644
> > --- a/drivers/infiniband/hw/mthca/mthca_mad.c
> > +++ b/drivers/infiniband/hw/mthca/mthca_mad.c
> > @@ -317,7 +317,7 @@ int mthca_create_agents(struct mthca_dev *dev)
> >  err:
> >  	for (p = 0; p < dev->limits.num_ports; ++p)
> >  		for (q = 0; q <= 1; ++q)
> > -			if (dev->send_agent[p][q])
> > +			if (!IS_ERR(dev->send_agent[p][q]))
> >  				ib_unregister_mad_agent(dev->send_agent[p][q]);
> >  
> >  	return ret;
> > diff --git a/drivers/infiniband/hw/qib/qib_mad.c b/drivers/infiniband/hw/qib/qib_mad.c
> > index 636be11..6c7cc80 100644
> > --- a/drivers/infiniband/hw/qib/qib_mad.c
> > +++ b/drivers/infiniband/hw/qib/qib_mad.c
> > @@ -2499,7 +2499,7 @@ int qib_create_agents(struct qib_ibdev *dev)
> >  err:
> >  	for (p = 0; p < dd->num_pports; p++) {
> >  		ibp = &dd->pport[p].ibport_data;
> > -		if (ibp->send_agent) {
> > +		if (!IS_ERR(ibp->send_agent)) {
> >  			agent = ibp->send_agent;
> >  			ibp->send_agent = NULL;
> >  			ib_unregister_mad_agent(agent);
> 

Regards.

-- 
Yann Droneaud
OPTEYA


--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH RE-RESEND V2 for-next 1/5] IB/core: When marshaling ucma path from user-space, clear unused fields

[Or Gerlitz]

On Thu, Feb 5, 2015 at 6:53 AM, Or Gerlitz <ogerlitz-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org> wrote:
> From: Ilya Nelkenbaum <ilyan-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>
>
> When marsheling a user path to the kernel struct ib_sa_path, need
> to zero smac, dmac and set the vlan id to the "no vlan" value.

Other patches in the series had some comments and on-going discussion,
this one is critical fix for the apps that hit the bug, please pick it
up

>
> This is to ensure that Ethernet attributes are not used with
> InfiniBand QPs.
>
> Fixes: dd5f03beb4f7 ("IB/core: Ethernet L2 attributes in verbs/cm structures")
> Signed-off-by: Ilya Nelkenbaum <ilyan-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>
> Signed-off-by: Or Gerlitz <ogerlitz-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>
> ---
>  drivers/infiniband/core/ucma.c |    3 +++
>  1 files changed, 3 insertions(+), 0 deletions(-)
>
> diff --git a/drivers/infiniband/core/ucma.c b/drivers/infiniband/core/ucma.c
> index 56a4b7c..45d67e9 100644
> --- a/drivers/infiniband/core/ucma.c
> +++ b/drivers/infiniband/core/ucma.c
> @@ -1124,6 +1124,9 @@ static int ucma_set_ib_path(struct ucma_context *ctx,
>         if (!optlen)
>                 return -EINVAL;
>
> +       memset(&sa_path, 0, sizeof(sa_path));
> +       sa_path.vlan_id = 0xffff;
> +
>         ib_sa_unpack_path(path_data->path_rec, &sa_path);
>         ret = rdma_set_ib_paths(ctx->cm_id, &sa_path, 1);
>         if (ret)
> --
> 1.7.1
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
> the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH RE-RESEND V2 for-next 5/5] IB/core: Fix deadlock on uverbs modify_qp error flow

[Or Gerlitz]

On 2/5/2015 6:53 AM, Or Gerlitz wrote:
> From: Moshe Lazer <moshel-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>
>
> The deadlock occurs on __uverbs_modify_qp, we take a lock (idr_read_qp)
> and in case of failure in ib_resolve_eth_l2_attrs we don't release
> it (put_qp_read). Fix that.
>
> Issue: 355606
> Fixes: ed4c54e5b4ba ("IB/core: Resolve Ethernet L2 addresses when modifying QP")
> Signed-off-by: Moshe Lazer <moshel-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>
> Signed-off-by: Or Gerlitz <ogerlitz-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>

Roland, can you please pick this one too, it's a bug fix and no comments 
were raised against it.
I just failed to remove the Gerrit Issue: XXX line,  so if you can do 
that, will be great

> ---
>   drivers/infiniband/core/uverbs_cmd.c |    9 +++++----
>   1 files changed, 5 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/infiniband/core/uverbs_cmd.c b/drivers/infiniband/core/uverbs_cmd.c
> index ecb6430..a602ce9 100644
> --- a/drivers/infiniband/core/uverbs_cmd.c
> +++ b/drivers/infiniband/core/uverbs_cmd.c
> @@ -2097,20 +2097,21 @@ ssize_t ib_uverbs_modify_qp(struct ib_uverbs_file *file,
>   	if (qp->real_qp == qp) {
>   		ret = ib_resolve_eth_l2_attrs(qp, attr, &cmd.attr_mask);
>   		if (ret)
> -			goto out;
> +			goto release_qp;
>   		ret = qp->device->modify_qp(qp, attr,
>   			modify_qp_mask(qp->qp_type, cmd.attr_mask), &udata);
>   	} else {
>   		ret = ib_modify_qp(qp, attr, modify_qp_mask(qp->qp_type, cmd.attr_mask));
>   	}
>   
> -	put_qp_read(qp);
> -
>   	if (ret)
> -		goto out;
> +		goto release_qp;
>   
>   	ret = in_len;
>   
> +release_qp:
> +	put_qp_read(qp);
> +
>   out:
>   	kfree(attr);
>   

--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH RE-RESEND V2 for-next 5/5] IB/core: Fix deadlock on uverbs modify_qp error flow

[Roland Dreier]

On Tue, Feb 17, 2015 at 9:03 PM, Or Gerlitz <ogerlitz-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org> wrote:
> Roland, can you please pick this one too, it's a bug fix and no comments
> were raised against it.
> I just failed to remove the Gerrit Issue: XXX line,  so if you can do that,
> will be great


OK.
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH RE-RESEND V2 for-next 5/5] IB/core: Fix deadlock on uverbs modify_qp error flow

[Or Gerlitz]

On 2/18/2015 8:07 AM, Roland Dreier wrote:
> On Tue, Feb 17, 2015 at 9:03 PM, Or Gerlitz <ogerlitz-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org> wrote:
>> Roland, can you please pick this one too, it's a bug fix and no comments
>> were raised against it.
>> I just failed to remove the Gerrit Issue: XXX line,  so if you can do that,
>> will be great
>
> OK.

I noted that these commits which are targeted to 3.20 are @ your master 
branch but not on for-next, just
dropping you a note to make sure they are not left behind.


IB/core: Add on demand paging caps to ib_uverbs_ex_query_device
IB/core: Add support for extended query device caps
IB/core: Fix deadlock on uverbs modify_qp error flow
IB/core: Properly handle registration of on-demand paging MRs after dereg
IB/mlx4: Bug fixes in mlx4_ib_resize_cq
IB/mlx4: Fix memory leak in __mlx4_ib_modify_qp
IB/mlx4: In mlx4_ib_demux_cm, print out GUID in host-endian order
IB/mlx5: Enable the ODP capability query verb
IB/mlx5: Update the dev in reg_create

--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH RE-RESEND V2 for-next 3/5] IB/core: Call ib_unregister_mad_agent() only for valid agents

[Majd Dibbiny]

On 17/2/2015 3:59 PM, Yann Droneaud wrote:
> Hi,
>
> Le lundi 16 février 2015 à 22:28 +0100, Yann Droneaud a écrit :
>> Le jeudi 05 février 2015 à 13:53 +0200, Or Gerlitz a écrit :
>>> From: Majd Dibbiny <majd-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>
>>>
>>> In some error flows, ib_mad_unregister_agent is being invoked also in cases where
>>                       ^^^^^^^^^^^^^^^^^^^^^^^
>>                       ib_unregister_mad_agent()
>>
>>> the ib_mad_register_agent call failed (resulting in an illegal pointer in the
>>      ^^^^^^^^^^^^^^^^^^^^^
>>      ib_register_mad_agent()
>>
>>> agent field).  This causes a kernel crash in the error flow.
>>>
>>> Fix this by calling ib_unregister_mad_agent only for cases where
>>> ib_register_mad_agent succeeded.
>>>
>> The code was checking for struct ib_mad_agent *agent not being NULL, 
>> while ib_register_mad_agent() returns a ERR_PTR() in case of error
>> ... bad :(
>>
> After reading Jason's comments [1], I don't understand the purpose of
> this patch. How can an ERR_PTR() value be present in the arrays ?
>
> A quick review makes me think that mthca and mlx4 error paths lacks NULL
> assignment after calling ib_unregister_mad_agent(). In other words, qib
> might be correct while the others should be fixed.
You are right. Seems like I missed things here.
I'll fix it and send a new version soon.
>
> [1] http://mid.gmane.org/20150205174337.GB31711-ePGOBjL8dl3ta4EC/59zMFaTQe2KTcn/@public.gmane.org
>
>>> Signed-off-by: Majd Dibbiny <majd-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>
>>> Signed-off-by: Or Gerlitz <ogerlitz-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>
>>> ---
>>>  drivers/infiniband/hw/mlx4/mad.c        |    2 +-
>>>  drivers/infiniband/hw/mthca/mthca_mad.c |    2 +-
>>>  drivers/infiniband/hw/qib/qib_mad.c     |    2 +-
>>>  3 files changed, 3 insertions(+), 3 deletions(-)
>>>
>>> diff --git a/drivers/infiniband/hw/mlx4/mad.c b/drivers/infiniband/hw/mlx4/mad.c
>>> index 82a7dd8..6be0d2c 100644
>>> --- a/drivers/infiniband/hw/mlx4/mad.c
>>> +++ b/drivers/infiniband/hw/mlx4/mad.c
>>> @@ -907,7 +907,7 @@ int mlx4_ib_mad_init(struct mlx4_ib_dev *dev)
>>>  err:
>>>  	for (p = 0; p < dev->num_ports; ++p)
>>>  		for (q = 0; q <= 1; ++q)
>>> -			if (dev->send_agent[p][q])
>>> +			if (!IS_ERR(dev->send_agent[p][q]))
>>>  				ib_unregister_mad_agent(dev->send_agent[p][q]);
>>>  
>>>  	return ret;
>>> diff --git a/drivers/infiniband/hw/mthca/mthca_mad.c b/drivers/infiniband/hw/mthca/mthca_mad.c
>>> index 8881fa3..5f1a7ce 100644
>>> --- a/drivers/infiniband/hw/mthca/mthca_mad.c
>>> +++ b/drivers/infiniband/hw/mthca/mthca_mad.c
>>> @@ -317,7 +317,7 @@ int mthca_create_agents(struct mthca_dev *dev)
>>>  err:
>>>  	for (p = 0; p < dev->limits.num_ports; ++p)
>>>  		for (q = 0; q <= 1; ++q)
>>> -			if (dev->send_agent[p][q])
>>> +			if (!IS_ERR(dev->send_agent[p][q]))
>>>  				ib_unregister_mad_agent(dev->send_agent[p][q]);
>>>  
>>>  	return ret;
>>> diff --git a/drivers/infiniband/hw/qib/qib_mad.c b/drivers/infiniband/hw/qib/qib_mad.c
>>> index 636be11..6c7cc80 100644
>>> --- a/drivers/infiniband/hw/qib/qib_mad.c
>>> +++ b/drivers/infiniband/hw/qib/qib_mad.c
>>> @@ -2499,7 +2499,7 @@ int qib_create_agents(struct qib_ibdev *dev)
>>>  err:
>>>  	for (p = 0; p < dd->num_pports; p++) {
>>>  		ibp = &dd->pport[p].ibport_data;
>>> -		if (ibp->send_agent) {
>>> +		if (!IS_ERR(ibp->send_agent)) {
>>>  			agent = ibp->send_agent;
>>>  			ibp->send_agent = NULL;
>>>  			ib_unregister_mad_agent(agent);
> Regards.
>

--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html