[PATCH] Return EPROTONOSUPPORT when an RDMA device lacks QP0

[PATCH] Return EPROTONOSUPPORT when an RDMA device lacks QP0

[Weiny, Ira K.]

[-- Attachment #1: Type: text/plain, Size: 229 bytes --]

We had a script which was looping through the devices returned from ibstat and attempted to register a SMI agent on an ethernet device.  This caused a kernel panic.  We have fixed our script.  This patch prevents the panic.



[-- Attachment #2: 0001-Return-EPROTONOSUPPORT-when-an-RDMA-device-lacks-QP0.patch --]
[-- Type: application/octet-stream, Size: 1079 bytes --]

From 82e627f392fc928f55354d0dfc5e7868ac01ead1 Mon Sep 17 00:00:00 2001
From: Ira Weiny <weiny2-i2BcT+NCU+M@public.gmane.org>
Date: Thu, 19 May 2011 18:19:28 -0700
Subject: [PATCH] Return EPROTONOSUPPORT when an RDMA device lacks QP0

This prevents a NULL pointer dereference later which panics the kernel.

Signed-off-by: Ira Weiny <weiny2-i2BcT+NCU+M@public.gmane.org>
---
 drivers/infiniband/core/mad.c |    7 +++++++
 1 files changed, 7 insertions(+), 0 deletions(-)

diff --git a/drivers/infiniband/core/mad.c b/drivers/infiniband/core/mad.c
index b2dba21..e30652a 100644
--- a/drivers/infiniband/core/mad.c
+++ b/drivers/infiniband/core/mad.c
@@ -208,6 +208,13 @@ struct ib_mad_agent *ib_register_mad_agent(struct ib_device *device,
 	u8 mgmt_class, vclass;
 
 	/* Validate parameters */
+	if (qp_type == IB_QPT_SMI &&
+	    rdma_port_get_link_layer(device, port_num)!=
+	    IB_LINK_LAYER_INFINIBAND) {
+		ret = ERR_PTR(-EPROTONOSUPPORT);
+		goto error1;
+	}
+
 	qpn = get_spl_qp_index(qp_type);
 	if (qpn == -1)
 		goto error1;
-- 
1.7.1

RE: [PATCH] Return EPROTONOSUPPORT when an RDMA device lacks QP0

[Hefty, Sean]

> We had a script which was looping through the devices returned from ibstat
> and attempted to register a SMI agent on an ethernet device.  This caused
> a kernel panic.  We have fixed our script.  This patch prevents the panic.

Do you remember where the panic was?

I'm going to guess in ib_register_mad_agent() at this line:

	mad_agent_priv->agent.mr = ib_get_dma_mr(port_priv->qp_info[qpn].qp->pd,
						 IB_ACCESS_LOCAL_WRITE);

in which case it may be safer to check for the NULL pointer.  Can you confirm if this was the spot?

- Sean
--
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] Return EPROTONOSUPPORT when an RDMA device lacks QP0

[Weiny, Ira K.]

On May 20, 2011, at 12:23 PM, Hefty, Sean wrote:

>> We had a script which was looping through the devices returned from ibstat
>> and attempted to register a SMI agent on an ethernet device.  This caused
>> a kernel panic.  We have fixed our script.  This patch prevents the panic.
> 
> Do you remember where the panic was?
> 
> I'm going to guess in ib_register_mad_agent() at this line:
> 
> 	mad_agent_priv->agent.mr = ib_get_dma_mr(port_priv->qp_info[qpn].qp->pd,
> 						 IB_ACCESS_LOCAL_WRITE);
> 
> in which case it may be safer to check for the NULL pointer.  Can you confirm if this was the spot?

Yes it was that spot.  I did think of that today after I sent the patch.

Do you think it would be safer just to check for both pointers QP0 and 1 (depending on the registration)?

Ira

> 
> - Sean

--
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] Return EPROTONOSUPPORT when an RDMA device lacks QP0

[Hefty, Sean]

> > 	mad_agent_priv->agent.mr = ib_get_dma_mr(port_priv->qp_info[qpn].qp-
> >pd,
> > 						 IB_ACCESS_LOCAL_WRITE);
> >
> > in which case it may be safer to check for the NULL pointer.  Can you
> confirm if this was the spot?
> 
> Yes it was that spot.  I did think of that today after I sent the patch.
> 
> Do you think it would be safer just to check for both pointers QP0 and 1
> (depending on the registration)?

Yes, it seems safer and easier to maintain if we just validated the pointer.

- Sean
--
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] Return EPROTONOSUPPORT when an RDMA device lacks QP0

[Weiny, Ira K.]

On May 20, 2011, at 4:23 PM, Hefty, Sean wrote:

>>> 	mad_agent_priv->agent.mr = ib_get_dma_mr(port_priv->qp_info[qpn].qp-
>>> pd,
>>> 						 IB_ACCESS_LOCAL_WRITE);
>>> 
>>> in which case it may be safer to check for the NULL pointer.  Can you
>> confirm if this was the spot?
>> 
>> Yes it was that spot.  I did think of that today after I sent the patch.
>> 
>> Do you think it would be safer just to check for both pointers QP0 and 1
>> (depending on the registration)?
> 
> Yes, it seems safer and easier to maintain if we just validated the pointer.

I will rework the patch.

Thanks,
Ira

> 
> - Sean

--
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 v2] Return EPROTONOSUPPORT when an RDMA device lacks QP0

[Weiny, Ira K.]

[-- Attachment #1: Type: text/plain, Size: 619 bytes --]


On May 20, 2011, at 4:23 PM, Hefty, Sean wrote:

>>> 	mad_agent_priv->agent.mr = ib_get_dma_mr(port_priv->qp_info[qpn].qp-
>>> pd,
>>> 						 IB_ACCESS_LOCAL_WRITE);
>>> 
>>> in which case it may be safer to check for the NULL pointer.  Can you
>> confirm if this was the spot?
>> 
>> Yes it was that spot.  I did think of that today after I sent the patch.
>> 
>> Do you think it would be safer just to check for both pointers QP0 and 1
>> (depending on the registration)?
> 
> Yes, it seems safer and easier to maintain if we just validated the pointer.
> 
> - Sean

New patch attached.

Ira


[-- Attachment #2: 0001-Return-EPROTONOSUPPORT-when-an-RDMA-device-lacks-the.patch --]
[-- Type: application/octet-stream, Size: 1319 bytes --]

From c4db4c727704cd94d37a57252e0636c2adf1e040 Mon Sep 17 00:00:00 2001
From: Ira Weiny <weiny2-i2BcT+NCU+M@public.gmane.org>
Date: Thu, 19 May 2011 18:19:28 -0700
Subject: [PATCH] Return EPROTONOSUPPORT when an RDMA device lacks the QP required.

Signed-off-by: Ira Weiny <weiny2-i2BcT+NCU+M@public.gmane.org>
---
 drivers/infiniband/core/mad.c |    8 ++++++++
 1 files changed, 8 insertions(+), 0 deletions(-)

diff --git a/drivers/infiniband/core/mad.c b/drivers/infiniband/core/mad.c
index b2dba21..fc97883 100644
--- a/drivers/infiniband/core/mad.c
+++ b/drivers/infiniband/core/mad.c
@@ -275,6 +275,13 @@ struct ib_mad_agent *ib_register_mad_agent(struct ib_device *device,
 		goto error1;
 	}
 
+	/* Verify the QP requested is supported.  For example, Ethernet devices
+	 * will not have QP0 */
+	if (!port_priv->qp_info[qpn].qp) {
+		ret = ERR_PTR(-EPROTONOSUPPORT);
+		goto error1;
+	}
+
 	/* Allocate structures */
 	mad_agent_priv = kzalloc(sizeof *mad_agent_priv, GFP_KERNEL);
 	if (!mad_agent_priv) {
@@ -2738,6 +2745,7 @@ static void init_mad_qp(struct ib_mad_port_private *port_priv,
 	spin_lock_init(&qp_info->snoop_lock);
 	qp_info->snoop_table = NULL;
 	qp_info->snoop_table_size = 0;
+	qp_info->qp = NULL;
 	atomic_set(&qp_info->snoop_count, 0);
 }
 
-- 
1.7.1

Re: [PATCH v2] Return EPROTONOSUPPORT when an RDMA device lacks QP0

[Roland Dreier]

 > @@ -2738,6 +2745,7 @@ static void init_mad_qp(struct
ib_mad_port_private *port_priv,
 >  	spin_lock_init(&qp_info->snoop_lock);
 >  	qp_info->snoop_table = NULL;
 >  	qp_info->snoop_table_size = 0;
 > +	qp_info->qp = NULL;
 >  	atomic_set(&qp_info->snoop_count, 0);
 >  }

This bit isn't needed, is it?  qp_info is in memory allocated with kzalloc.
(We could get rid of the other = 0 / = NULL lines too)

 - R.
--
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 v2] Return EPROTONOSUPPORT when an RDMA device lacks QP0

[Weiny, Ira K.]

[-- Attachment #1: Type: text/plain, Size: 502 bytes --]


On May 22, 2011, at 9:05 PM, Roland Dreier wrote:

>> @@ -2738,6 +2745,7 @@ static void init_mad_qp(struct
> ib_mad_port_private *port_priv,
>> 	spin_lock_init(&qp_info->snoop_lock);
>> 	qp_info->snoop_table = NULL;
>> 	qp_info->snoop_table_size = 0;
>> +	qp_info->qp = NULL;
>> 	atomic_set(&qp_info->snoop_count, 0);
>> }
> 
> This bit isn't needed, is it?  qp_info is in memory allocated with kzalloc.
> (We could get rid of the other = 0 / = NULL lines too)
> 

Yes, sorry, revised attached,
Ira



[-- Attachment #2: 0001-Return-EPROTONOSUPPORT-when-an-RDMA-device-lacks-the.patch --]
[-- Type: application/octet-stream, Size: 952 bytes --]

From: Ira Weiny <weiny2-i2BcT+NCU+M@public.gmane.org>
Date: Thu, 19 May 2011 18:19:28 -0700
Subject: [PATCH] Return EPROTONOSUPPORT when an RDMA device lacks the QP required.

Signed-off-by: Ira Weiny <weiny2-i2BcT+NCU+M@public.gmane.org>
---
 drivers/infiniband/core/mad.c |    7 +++++++
 1 files changed, 7 insertions(+), 0 deletions(-)

diff --git a/drivers/infiniband/core/mad.c b/drivers/infiniband/core/mad.c
index b2dba21..25ba485 100644
--- a/drivers/infiniband/core/mad.c
+++ b/drivers/infiniband/core/mad.c
@@ -275,6 +275,13 @@ struct ib_mad_agent *ib_register_mad_agent(struct ib_device *device,
 		goto error1;
 	}
 
+	/* Verify the QP requested is supported.  For example, Ethernet devices
+	 * will not have QP0 */
+	if (!port_priv->qp_info[qpn].qp) {
+		ret = ERR_PTR(-EPROTONOSUPPORT);
+		goto error1;
+	}
+
 	/* Allocate structures */
 	mad_agent_priv = kzalloc(sizeof *mad_agent_priv, GFP_KERNEL);
 	if (!mad_agent_priv) {
-- 
1.7.1

Re: [PATCH v2] Return EPROTONOSUPPORT when an RDMA device lacks QP0

[Roland Dreier]

> Yes, sorry, revised attached,

Thanks, this looks good to apply to me.

Sean, any objection?

 - R.
--
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 v2] Return EPROTONOSUPPORT when an RDMA device lacks QP0

[Hefty, Sean]

> Sean, any objection?

Looks good to me.  Thanks, Ira.

Acked-by: Sean Hefty <sean.hefty-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>
--
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