[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>
This commit is contained in:
committed by
Roland Dreier
parent
089a1bedd8
commit
cb0f0910f4
@@ -99,7 +99,6 @@ struct ib_umad_packet {
|
|||||||
struct ib_mad_send_buf *msg;
|
struct ib_mad_send_buf *msg;
|
||||||
struct list_head list;
|
struct list_head list;
|
||||||
int length;
|
int length;
|
||||||
DECLARE_PCI_UNMAP_ADDR(mapping)
|
|
||||||
struct ib_user_mad mad;
|
struct ib_user_mad mad;
|
||||||
};
|
};
|
||||||
|
|
||||||
@@ -145,14 +144,11 @@ static void send_handler(struct ib_mad_agent *agent,
|
|||||||
ib_free_send_mad(packet->msg);
|
ib_free_send_mad(packet->msg);
|
||||||
|
|
||||||
if (send_wc->status == IB_WC_RESP_TIMEOUT_ERR) {
|
if (send_wc->status == IB_WC_RESP_TIMEOUT_ERR) {
|
||||||
timeout = kmalloc(sizeof *timeout + sizeof (struct ib_mad_hdr),
|
timeout = kzalloc(sizeof *timeout + IB_MGMT_MAD_HDR, GFP_KERNEL);
|
||||||
GFP_KERNEL);
|
|
||||||
if (!timeout)
|
if (!timeout)
|
||||||
goto out;
|
goto out;
|
||||||
|
|
||||||
memset(timeout, 0, sizeof *timeout + sizeof (struct ib_mad_hdr));
|
timeout->length = IB_MGMT_MAD_HDR;
|
||||||
|
|
||||||
timeout->length = sizeof (struct ib_mad_hdr);
|
|
||||||
timeout->mad.hdr.id = packet->mad.hdr.id;
|
timeout->mad.hdr.id = packet->mad.hdr.id;
|
||||||
timeout->mad.hdr.status = ETIMEDOUT;
|
timeout->mad.hdr.status = ETIMEDOUT;
|
||||||
memcpy(timeout->mad.data, packet->mad.data,
|
memcpy(timeout->mad.data, packet->mad.data,
|
||||||
@@ -176,11 +172,10 @@ static void recv_handler(struct ib_mad_agent *agent,
|
|||||||
goto out;
|
goto out;
|
||||||
|
|
||||||
length = mad_recv_wc->mad_len;
|
length = mad_recv_wc->mad_len;
|
||||||
packet = kmalloc(sizeof *packet + length, GFP_KERNEL);
|
packet = kzalloc(sizeof *packet + length, GFP_KERNEL);
|
||||||
if (!packet)
|
if (!packet)
|
||||||
goto out;
|
goto out;
|
||||||
|
|
||||||
memset(packet, 0, sizeof *packet + length);
|
|
||||||
packet->length = length;
|
packet->length = length;
|
||||||
|
|
||||||
ib_coalesce_recv_mad(mad_recv_wc, packet->mad.data);
|
ib_coalesce_recv_mad(mad_recv_wc, packet->mad.data);
|
||||||
@@ -271,22 +266,19 @@ static ssize_t ib_umad_write(struct file *filp, const char __user *buf,
|
|||||||
struct ib_rmpp_mad *rmpp_mad;
|
struct ib_rmpp_mad *rmpp_mad;
|
||||||
u8 method;
|
u8 method;
|
||||||
__be64 *tid;
|
__be64 *tid;
|
||||||
int ret, length, hdr_len, rmpp_hdr_size;
|
int ret, length, hdr_len, copy_offset;
|
||||||
int rmpp_active = 0;
|
int rmpp_active = 0;
|
||||||
|
|
||||||
if (count < sizeof (struct ib_user_mad))
|
if (count < sizeof (struct ib_user_mad))
|
||||||
return -EINVAL;
|
return -EINVAL;
|
||||||
|
|
||||||
length = count - sizeof (struct ib_user_mad);
|
length = count - sizeof (struct ib_user_mad);
|
||||||
packet = kmalloc(sizeof *packet + sizeof(struct ib_mad_hdr) +
|
packet = kmalloc(sizeof *packet + IB_MGMT_RMPP_HDR, GFP_KERNEL);
|
||||||
sizeof (struct ib_rmpp_hdr), GFP_KERNEL);
|
|
||||||
if (!packet)
|
if (!packet)
|
||||||
return -ENOMEM;
|
return -ENOMEM;
|
||||||
|
|
||||||
if (copy_from_user(&packet->mad, buf,
|
if (copy_from_user(&packet->mad, buf,
|
||||||
sizeof (struct ib_user_mad) +
|
sizeof (struct ib_user_mad) + IB_MGMT_RMPP_HDR)) {
|
||||||
sizeof (struct ib_mad_hdr) +
|
|
||||||
sizeof (struct ib_rmpp_hdr))) {
|
|
||||||
ret = -EFAULT;
|
ret = -EFAULT;
|
||||||
goto err;
|
goto err;
|
||||||
}
|
}
|
||||||
@@ -297,8 +289,6 @@ static ssize_t ib_umad_write(struct file *filp, const char __user *buf,
|
|||||||
goto err;
|
goto err;
|
||||||
}
|
}
|
||||||
|
|
||||||
packet->length = length;
|
|
||||||
|
|
||||||
down_read(&file->agent_mutex);
|
down_read(&file->agent_mutex);
|
||||||
|
|
||||||
agent = file->agent[packet->mad.hdr.id];
|
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;
|
goto err_ah;
|
||||||
}
|
}
|
||||||
rmpp_active = 1;
|
rmpp_active = 1;
|
||||||
|
copy_offset = IB_MGMT_RMPP_HDR;
|
||||||
} else {
|
} else {
|
||||||
if (length > sizeof (struct ib_mad)) {
|
|
||||||
ret = -EINVAL;
|
|
||||||
goto err_ah;
|
|
||||||
}
|
|
||||||
hdr_len = IB_MGMT_MAD_HDR;
|
hdr_len = IB_MGMT_MAD_HDR;
|
||||||
|
copy_offset = IB_MGMT_MAD_HDR;
|
||||||
}
|
}
|
||||||
|
|
||||||
packet->msg = ib_create_send_mad(agent,
|
packet->msg = ib_create_send_mad(agent,
|
||||||
@@ -368,29 +356,15 @@ static ssize_t ib_umad_write(struct file *filp, const char __user *buf,
|
|||||||
packet->msg->retries = packet->mad.hdr.retries;
|
packet->msg->retries = packet->mad.hdr.retries;
|
||||||
packet->msg->context[0] = packet;
|
packet->msg->context[0] = packet;
|
||||||
|
|
||||||
if (!rmpp_active) {
|
/* Copy MAD headers (RMPP header in place) */
|
||||||
/* Copy message from user into send buffer */
|
memcpy(packet->msg->mad, packet->mad.data, IB_MGMT_MAD_HDR);
|
||||||
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 */
|
/* Now, copy rest of message from user into send buffer */
|
||||||
if (copy_from_user(((struct ib_rmpp_mad *) packet->msg->mad)->data,
|
if (copy_from_user(packet->msg->mad + copy_offset,
|
||||||
buf + sizeof (struct ib_user_mad) + rmpp_hdr_size,
|
buf + sizeof (struct ib_user_mad) + copy_offset,
|
||||||
length - rmpp_hdr_size)) {
|
length - copy_offset)) {
|
||||||
ret = -EFAULT;
|
ret = -EFAULT;
|
||||||
goto err_msg;
|
goto err_msg;
|
||||||
}
|
}
|
||||||
}
|
|
||||||
|
|
||||||
/*
|
/*
|
||||||
* If userspace is generating a request that will generate a
|
* If userspace is generating a request that will generate a
|
||||||
@@ -414,7 +388,7 @@ static ssize_t ib_umad_write(struct file *filp, const char __user *buf,
|
|||||||
|
|
||||||
up_read(&file->agent_mutex);
|
up_read(&file->agent_mutex);
|
||||||
|
|
||||||
return sizeof (struct ib_user_mad_hdr) + packet->length;
|
return count;
|
||||||
|
|
||||||
err_msg:
|
err_msg:
|
||||||
ib_free_send_mad(packet->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);
|
container_of(inode->i_cdev, struct ib_umad_port, dev);
|
||||||
struct ib_umad_file *file;
|
struct ib_umad_file *file;
|
||||||
|
|
||||||
file = kmalloc(sizeof *file, GFP_KERNEL);
|
file = kzalloc(sizeof *file, GFP_KERNEL);
|
||||||
if (!file)
|
if (!file)
|
||||||
return -ENOMEM;
|
return -ENOMEM;
|
||||||
|
|
||||||
memset(file, 0, sizeof *file);
|
|
||||||
|
|
||||||
spin_lock_init(&file->recv_lock);
|
spin_lock_init(&file->recv_lock);
|
||||||
init_rwsem(&file->agent_mutex);
|
init_rwsem(&file->agent_mutex);
|
||||||
INIT_LIST_HEAD(&file->recv_list);
|
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;
|
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),
|
(e - s + 1) * sizeof (struct ib_umad_port),
|
||||||
GFP_KERNEL);
|
GFP_KERNEL);
|
||||||
if (!umad_dev)
|
if (!umad_dev)
|
||||||
return;
|
return;
|
||||||
|
|
||||||
memset(umad_dev, 0, sizeof *umad_dev +
|
|
||||||
(e - s + 1) * sizeof (struct ib_umad_port));
|
|
||||||
|
|
||||||
kref_init(&umad_dev->ref);
|
kref_init(&umad_dev->ref);
|
||||||
|
|
||||||
umad_dev->start_port = s;
|
umad_dev->start_port = s;
|
||||||
|
Reference in New Issue
Block a user