From cb0f0910f4b41772a6771bdb4fb2d419b27bcd77 Mon Sep 17 00:00:00 2001 From: Sean Hefty Date: Thu, 27 Oct 2005 20:48:11 -0700 Subject: [PATCH] [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 Signed-off-by: Roland Dreier --- drivers/infiniband/core/user_mad.c | 73 +++++++++--------------------- 1 file changed, 21 insertions(+), 52 deletions(-) diff --git a/drivers/infiniband/core/user_mad.c b/drivers/infiniband/core/user_mad.c index a48166a8e0..17ec0a19db 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; -- 2.39.5