diff mbox

[3/5] ib/mad: Add pr_notice messages for various umad/mad registration failures

Message ID 1399340690-27677-4-git-send-email-ira.weiny@intel.com (mailing list archive)
State Superseded, archived
Headers show

Commit Message

Ira Weiny May 6, 2014, 1:44 a.m. UTC 
From: Ira Weiny <ira.weiny@intel.com>

Reviewed-by: Sean Hefty <sean.hefty@intel.com>
Reviewed-by: Hal Rosenstock <hal@dev.mellanox.co.il>
Signed-off-by: Ira Weiny <ira.weiny@intel.com>
---
 drivers/infiniband/core/mad.c      | 47 ++++++++++++++++++++++++++++++--------
 drivers/infiniband/core/user_mad.c |  5 ++++
 2 files changed, 43 insertions(+), 9 deletions(-)

Comments

Jason Gunthorpe May 6, 2014, 2:57 a.m. UTC | #1 
On Mon, May 05, 2014 at 09:44:48PM -0400, ira.weiny@intel.com wrote:
> From: Ira Weiny <ira.weiny@intel.com>

An empty commit message is not good enough for the mainline kernel,
please describe motivation at least.

I am worried about log DOS potential here, it looks like many of these
messages could be triggered by userspace? Have you thought about this?

Generally moving this stuff to a pr_debug might be more appropriate,
it is very unusual to have logging like this for malformed syscall
arguments.

Jason
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Ira Weiny May 7, 2014, 2:35 p.m. UTC | #2 
> 
> On Mon, May 05, 2014 at 09:44:48PM -0400, ira.weiny@intel.com wrote:
> > From: Ira Weiny <ira.weiny@intel.com>
> 
> An empty commit message is not good enough for the mainline kernel,
> please describe motivation at least.

Will do.

> 
> I am worried about log DOS potential here, it looks like many of these
> messages could be triggered by userspace? Have you thought about this?

Yes I did think about it, see below.

> 
> Generally moving this stuff to a pr_debug might be more appropriate, it is
> very unusual to have logging like this for malformed syscall arguments.

I debated using debug.  The problem with debug is that it requires a kernel recompile.  This may not be acceptable for some users.  Ib_umad devices should  only be accessible from a privileged user which I felt was sufficient protection against a DOS attack.  But if it would be more acceptable I can change to [pr|dev]_debug.

Ira

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

Patch

diff --git a/drivers/infiniband/core/mad.c b/drivers/infiniband/core/mad.c
index 7395e23..c48e9a8 100644
--- a/drivers/infiniband/core/mad.c
+++ b/drivers/infiniband/core/mad.c
@@ -214,58 +214,85 @@  struct ib_mad_agent *ib_register_mad_agent(struct ib_device *device,
 
 	/* Validate parameters */
 	qpn = get_spl_qp_index(qp_type);
-	if (qpn == -1)
+	if (qpn == -1) {
+		pr_notice("ib_register_mad_agent: invalid QP Type %d\n",
+			qp_type);
 		goto error1;
+	}
 
-	if (rmpp_version && rmpp_version != IB_MGMT_RMPP_VERSION)
+	if (rmpp_version && rmpp_version != IB_MGMT_RMPP_VERSION) {
+		pr_notice("ib_register_mad_agent: invalid RMPP Version %u\n",
+			rmpp_version);
 		goto error1;
+	}
 
 	/* Validate MAD registration request if supplied */
 	if (mad_reg_req) {
-		if (mad_reg_req->mgmt_class_version >= MAX_MGMT_VERSION)
+		if (mad_reg_req->mgmt_class_version >= MAX_MGMT_VERSION) {
+			pr_notice("ib_register_mad_agent: invalid Class Version %u\n",
+				mad_reg_req->mgmt_class_version);
 			goto error1;
-		if (!recv_handler)
+		}
+		if (!recv_handler) {
+			pr_notice("ib_register_mad_agent: no recv_handler\n");
 			goto error1;
+		}
 		if (mad_reg_req->mgmt_class >= MAX_MGMT_CLASS) {
 			/*
 			 * IB_MGMT_CLASS_SUBN_DIRECTED_ROUTE is the only
 			 * one in this range currently allowed
 			 */
 			if (mad_reg_req->mgmt_class !=
-			    IB_MGMT_CLASS_SUBN_DIRECTED_ROUTE)
+			    IB_MGMT_CLASS_SUBN_DIRECTED_ROUTE) {
+				pr_notice("ib_register_mad_agent: Invalid Mgmt Class 0x%x\n",
+					mad_reg_req->mgmt_class);
 				goto error1;
+			}
 		} else if (mad_reg_req->mgmt_class == 0) {
 			/*
 			 * Class 0 is reserved in IBA and is used for
 			 * aliasing of IB_MGMT_CLASS_SUBN_DIRECTED_ROUTE
 			 */
+			pr_notice("ib_register_mad_agent: Invalid Mgmt Class 0\n");
 			goto error1;
 		} else if (is_vendor_class(mad_reg_req->mgmt_class)) {
 			/*
 			 * If class is in "new" vendor range,
 			 * ensure supplied OUI is not zero
 			 */
-			if (!is_vendor_oui(mad_reg_req->oui))
+			if (!is_vendor_oui(mad_reg_req->oui)) {
+				pr_notice("ib_register_mad_agent: No OUI specified for class 0x%x\n",
+					mad_reg_req->mgmt_class);
 				goto error1;
+			}
 		}
 		/* Make sure class supplied is consistent with RMPP */
 		if (!ib_is_mad_class_rmpp(mad_reg_req->mgmt_class)) {
-			if (rmpp_version)
+			if (rmpp_version) {
+				pr_notice("ib_register_mad_agent: RMPP version for non-RMPP class 0x%x\n",
+					mad_reg_req->mgmt_class);
 				goto error1;
+			}
 		}
 		/* Make sure class supplied is consistent with QP type */
 		if (qp_type == IB_QPT_SMI) {
 			if ((mad_reg_req->mgmt_class !=
 					IB_MGMT_CLASS_SUBN_LID_ROUTED) &&
 			    (mad_reg_req->mgmt_class !=
-					IB_MGMT_CLASS_SUBN_DIRECTED_ROUTE))
+					IB_MGMT_CLASS_SUBN_DIRECTED_ROUTE)) {
+				pr_notice("ib_register_mad_agent: Invalid SM QP type: class 0x%x\n",
+					mad_reg_req->mgmt_class);
 				goto error1;
+			}
 		} else {
 			if ((mad_reg_req->mgmt_class ==
 					IB_MGMT_CLASS_SUBN_LID_ROUTED) ||
 			    (mad_reg_req->mgmt_class ==
-					IB_MGMT_CLASS_SUBN_DIRECTED_ROUTE))
+					IB_MGMT_CLASS_SUBN_DIRECTED_ROUTE)) {
+				pr_notice("ib_register_mad_agent: Invalid GS QP type: class 0x%x\n",
+					mad_reg_req->mgmt_class);
 				goto error1;
+			}
 		}
 	} else {
 		/* No registration request supplied */
@@ -276,6 +303,7 @@  struct ib_mad_agent *ib_register_mad_agent(struct ib_device *device,
 	/* Validate device and port */
 	port_priv = ib_get_mad_port(device, port_num);
 	if (!port_priv) {
+		pr_notice("ib_register_mad_agent: Invalid port\n");
 		ret = ERR_PTR(-ENODEV);
 		goto error1;
 	}
@@ -283,6 +311,7 @@  struct ib_mad_agent *ib_register_mad_agent(struct ib_device *device,
 	/* Verify the QP requested is supported.  For example, Ethernet devices
 	 * will not have QP0 */
 	if (!port_priv->qp_info[qpn].qp) {
+		pr_notice("ib_register_mad_agent: QP %d not supported\n", qpn);
 		ret = ERR_PTR(-EPROTONOSUPPORT);
 		goto error1;
 	}
diff --git a/drivers/infiniband/core/user_mad.c b/drivers/infiniband/core/user_mad.c
index 0df292a..1341282 100644
--- a/drivers/infiniband/core/user_mad.c
+++ b/drivers/infiniband/core/user_mad.c
@@ -612,6 +612,7 @@  static int ib_umad_reg_agent(struct ib_umad_file *file, void __user *arg,
 	mutex_lock(&file->mutex);
 
 	if (!file->port->ib_dev) {
+		pr_notice("ib_umad_reg_agent: invalid device\n");
 		ret = -EPIPE;
 		goto out;
 	}
@@ -622,6 +623,8 @@  static int ib_umad_reg_agent(struct ib_umad_file *file, void __user *arg,
 	}
 
 	if (ureq.qpn != 0 && ureq.qpn != 1) {
+		pr_notice("ib_umad_reg_agent: invalid QPN %d specified\n",
+			ureq.qpn);
 		ret = -EINVAL;
 		goto out;
 	}
@@ -630,6 +633,8 @@  static int ib_umad_reg_agent(struct ib_umad_file *file, void __user *arg,
 		if (!__get_agent(file, agent_id))
 			goto found;
 
+	pr_notice("ib_umad_reg_agent: Max Agents (%u) reached\n",
+		IB_UMAD_MAX_AGENTS);
 	ret = -ENOMEM;
 	goto out;