Skip to content

Commit

Permalink
[IB] ib_umad: various cleanups
Browse files Browse the repository at this point in the history 
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>
  • Loading branch information
Sean Hefty authored and Roland Dreier committed Oct 28, 2005
1 parent 089a1be commit cb0f091
Showing 1 changed file with 21 additions and 52 deletions.
73 changes: 21 additions & 52 deletions drivers/infiniband/core/user_mad.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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;
};

Expand Down Expand Up @@ -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));
Expand All @@ -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);
Expand Down Expand Up @@ -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);
Expand All @@ -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;
}
Expand All @@ -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];
Expand Down Expand Up @@ -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,
Expand All @@ -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;
}

/*
Expand All @@ -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);
Expand Down Expand Up @@ -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);
Expand Down Expand Up @@ -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;
Expand Down

0 comments on commit cb0f091

Please sign in to comment.