[IB] ib_umad: various cleanups

Simplify user_mad.c code in a few places, and convert from kmalloc() +
memset() to kzalloc().  This also fixes a theoretical race window by
not accessing packet->length after posting the send buffer (the send
could complete and packet could be freed before we get to the return
statement at the end of ib_umad_write()).

Signed-off-by: Sean Hefty <sean.hefty@intel.com>
Signed-off-by: Roland Dreier <rolandd@cisco.com>

1 files changed, 21 insertions, 52 deletions

diff --git a/drivers/infiniband/core/user_mad.c b/drivers/infiniband/core/user_mad.c
index a48166a8e04b..17ec0a19dbc0 100644
--- a/drivers/infiniband/core/user_mad.c
+++ b/drivers/infiniband/core/user_mad.c
@@ -99,7 +99,6 @@ struct ib_umad_packet {
 	struct ib_mad_send_buf *msg;
 	struct list_head   list;
 	int		   length;
-	DECLARE_PCI_UNMAP_ADDR(mapping)
 	struct ib_user_mad mad;
 };
 
@@ -145,15 +144,12 @@ static void send_handler(struct ib_mad_agent *agent,
 	ib_free_send_mad(packet->msg);
 
 	if (send_wc->status == IB_WC_RESP_TIMEOUT_ERR) {
-		timeout = kmalloc(sizeof *timeout + sizeof (struct ib_mad_hdr),
-				  GFP_KERNEL);
+		timeout = kzalloc(sizeof *timeout + IB_MGMT_MAD_HDR, GFP_KERNEL);
 		if (!timeout)
 			goto out;
 
-		memset(timeout, 0, sizeof *timeout + sizeof (struct ib_mad_hdr));
-
-		timeout->length = sizeof (struct ib_mad_hdr);
-		timeout->mad.hdr.id = packet->mad.hdr.id;
+		timeout->length 	= IB_MGMT_MAD_HDR;
+		timeout->mad.hdr.id 	= packet->mad.hdr.id;
 		timeout->mad.hdr.status = ETIMEDOUT;
 		memcpy(timeout->mad.data, packet->mad.data,
 		       sizeof (struct ib_mad_hdr));
@@ -176,11 +172,10 @@ static void recv_handler(struct ib_mad_agent *agent,
 		goto out;
 
 	length = mad_recv_wc->mad_len;
-	packet = kmalloc(sizeof *packet + length, GFP_KERNEL);
+	packet = kzalloc(sizeof *packet + length, GFP_KERNEL);
 	if (!packet)
 		goto out;
 
-	memset(packet, 0, sizeof *packet + length);
 	packet->length = length;
 
 	ib_coalesce_recv_mad(mad_recv_wc, packet->mad.data);
@@ -246,7 +241,7 @@ static ssize_t ib_umad_read(struct file *filp, char __user *buf,
 		else
 			ret = -ENOSPC;
 	} else if (copy_to_user(buf, &packet->mad,
-			      packet->length + sizeof (struct ib_user_mad)))
+				packet->length + sizeof (struct ib_user_mad)))
 		ret = -EFAULT;
 	else
 		ret = packet->length + sizeof (struct ib_user_mad);
@@ -271,22 +266,19 @@ static ssize_t ib_umad_write(struct file *filp, const char __user *buf,
 	struct ib_rmpp_mad *rmpp_mad;
 	u8 method;
 	__be64 *tid;
-	int ret, length, hdr_len, rmpp_hdr_size;
+	int ret, length, hdr_len, copy_offset;
 	int rmpp_active = 0;
 
 	if (count < sizeof (struct ib_user_mad))
 		return -EINVAL;
 
 	length = count - sizeof (struct ib_user_mad);
-	packet = kmalloc(sizeof *packet + sizeof(struct ib_mad_hdr) +
-			 sizeof (struct ib_rmpp_hdr), GFP_KERNEL);
+	packet = kmalloc(sizeof *packet + IB_MGMT_RMPP_HDR, GFP_KERNEL);
 	if (!packet)
 		return -ENOMEM;
 
 	if (copy_from_user(&packet->mad, buf,
-			    sizeof (struct ib_user_mad) +
-			    sizeof (struct ib_mad_hdr) +
-			    sizeof (struct ib_rmpp_hdr))) {
+			    sizeof (struct ib_user_mad) + IB_MGMT_RMPP_HDR)) {
 		ret = -EFAULT;
 		goto err;
 	}
@@ -297,8 +289,6 @@ static ssize_t ib_umad_write(struct file *filp, const char __user *buf,
 		goto err;
 	}
 
-	packet->length = length;
-
 	down_read(&file->agent_mutex);
 
 	agent = file->agent[packet->mad.hdr.id];
@@ -345,12 +335,10 @@ static ssize_t ib_umad_write(struct file *filp, const char __user *buf,
 			goto err_ah;
 		}
 		rmpp_active = 1;
+		copy_offset = IB_MGMT_RMPP_HDR;
 	} else {
-		if (length > sizeof (struct ib_mad)) {
-			ret = -EINVAL;
-			goto err_ah;
-		}
 		hdr_len = IB_MGMT_MAD_HDR;
+		copy_offset = IB_MGMT_MAD_HDR;
 	}
 
 	packet->msg = ib_create_send_mad(agent,
@@ -368,28 +356,14 @@ static ssize_t ib_umad_write(struct file *filp, const char __user *buf,
 	packet->msg->retries 	= packet->mad.hdr.retries;
 	packet->msg->context[0] = packet;
 
-	if (!rmpp_active) {
-		/* Copy message from user into send buffer */
-		if (copy_from_user(packet->msg->mad,
-				   buf + sizeof (struct ib_user_mad), length)) {
-			ret = -EFAULT;
-			goto err_msg;
-		}
-	} else {
-		rmpp_hdr_size = sizeof (struct ib_mad_hdr) +
-				sizeof (struct ib_rmpp_hdr);
-
-		/* Only copy MAD headers (RMPP header in place) */
-		memcpy(packet->msg->mad, packet->mad.data,
-		       sizeof (struct ib_mad_hdr));
-
-		/* Now, copy rest of message from user into send buffer */
-		if (copy_from_user(((struct ib_rmpp_mad *) packet->msg->mad)->data,
-				   buf + sizeof (struct ib_user_mad) + rmpp_hdr_size,
-				   length - rmpp_hdr_size)) {
-			ret = -EFAULT;
-			goto err_msg;
-		}
+	/* Copy MAD headers (RMPP header in place) */
+	memcpy(packet->msg->mad, packet->mad.data, IB_MGMT_MAD_HDR);
+	/* Now, copy rest of message from user into send buffer */
+	if (copy_from_user(packet->msg->mad + copy_offset,
+			   buf + sizeof (struct ib_user_mad) + copy_offset,
+			   length - copy_offset)) {
+		ret = -EFAULT;
+		goto err_msg;
 	}
 
 	/*
@@ -414,7 +388,7 @@ static ssize_t ib_umad_write(struct file *filp, const char __user *buf,
 
 	up_read(&file->agent_mutex);
 
-	return sizeof (struct ib_user_mad_hdr) + packet->length;
+	return count;
 
 err_msg:
 	ib_free_send_mad(packet->msg);
@@ -564,12 +538,10 @@ static int ib_umad_open(struct inode *inode, struct file *filp)
 		container_of(inode->i_cdev, struct ib_umad_port, dev);
 	struct ib_umad_file *file;
 
-	file = kmalloc(sizeof *file, GFP_KERNEL);
+	file = kzalloc(sizeof *file, GFP_KERNEL);
 	if (!file)
 		return -ENOMEM;
 
-	memset(file, 0, sizeof *file);
-
 	spin_lock_init(&file->recv_lock);
 	init_rwsem(&file->agent_mutex);
 	INIT_LIST_HEAD(&file->recv_list);
@@ -814,15 +786,12 @@ static void ib_umad_add_one(struct ib_device *device)
 		e = device->phys_port_cnt;
 	}
 
-	umad_dev = kmalloc(sizeof *umad_dev +
+	umad_dev = kzalloc(sizeof *umad_dev +
 			   (e - s + 1) * sizeof (struct ib_umad_port),
 			   GFP_KERNEL);
 	if (!umad_dev)
 		return;
 
-	memset(umad_dev, 0, sizeof *umad_dev +
-	       (e - s + 1) * sizeof (struct ib_umad_port));
-
 	kref_init(&umad_dev->ref);
 
 	umad_dev->start_port = s;