Commit 1725d71d authored by Vasily Averin's avatar Vasily Averin Committed by Linus Torvalds

i2o_cfg_passthru cleanup

This patch fixes a number of issues in i2o_cfg_passthru{,32}:
- i2o_msg_get_wait() return vaile is not checked;
- i2o_message memory leaks on error paths;
- infinite loop to sg_list_cleanup in passthru32

It's important issue because of i2o_cfg_passthru is used by raidutils for
monitorig controllers state, and in case of memory shortage it leads to the
node crash or disk IO stall.

[akpm@linux-foundation.org: fix null-ptr deref]
Signed-off-by: default avatarVasily Averin <vvs@sw.ru>
Acked-by: default avatarAlan Cox <alan@lxorguk.ukuu.org.uk>
Cc: Markus Lidel <Markus.Lidel@shadowconnect.com>
Acked-by: default avatarKirill Korotaev <dev@openvz.org>
Signed-off-by: default avatarAndrew Morton <akpm@linux-foundation.org>
Signed-off-by: default avatarLinus Torvalds <torvalds@linux-foundation.org>
parent 2bf68a36
...@@ -554,8 +554,6 @@ static int i2o_cfg_passthru32(struct file *file, unsigned cmnd, ...@@ -554,8 +554,6 @@ static int i2o_cfg_passthru32(struct file *file, unsigned cmnd,
return -ENXIO; return -ENXIO;
} }
msg = i2o_msg_get_wait(c, I2O_TIMEOUT_MESSAGE_GET);
sb = c->status_block.virt; sb = c->status_block.virt;
if (get_user(size, &user_msg[0])) { if (get_user(size, &user_msg[0])) {
...@@ -573,24 +571,30 @@ static int i2o_cfg_passthru32(struct file *file, unsigned cmnd, ...@@ -573,24 +571,30 @@ static int i2o_cfg_passthru32(struct file *file, unsigned cmnd,
size <<= 2; // Convert to bytes size <<= 2; // Convert to bytes
msg = i2o_msg_get_wait(c, I2O_TIMEOUT_MESSAGE_GET);
if (IS_ERR(msg))
return PTR_ERR(msg);
rcode = -EFAULT;
/* Copy in the user's I2O command */ /* Copy in the user's I2O command */
if (copy_from_user(msg, user_msg, size)) { if (copy_from_user(msg, user_msg, size)) {
osm_warn("unable to copy user message\n"); osm_warn("unable to copy user message\n");
return -EFAULT; goto out;
} }
i2o_dump_message(msg); i2o_dump_message(msg);
if (get_user(reply_size, &user_reply[0]) < 0) if (get_user(reply_size, &user_reply[0]) < 0)
return -EFAULT; goto out;
reply_size >>= 16; reply_size >>= 16;
reply_size <<= 2; reply_size <<= 2;
rcode = -ENOMEM;
reply = kzalloc(reply_size, GFP_KERNEL); reply = kzalloc(reply_size, GFP_KERNEL);
if (!reply) { if (!reply) {
printk(KERN_WARNING "%s: Could not allocate reply buffer\n", printk(KERN_WARNING "%s: Could not allocate reply buffer\n",
c->name); c->name);
return -ENOMEM; goto out;
} }
sg_offset = (msg->u.head[0] >> 4) & 0x0f; sg_offset = (msg->u.head[0] >> 4) & 0x0f;
...@@ -661,13 +665,14 @@ static int i2o_cfg_passthru32(struct file *file, unsigned cmnd, ...@@ -661,13 +665,14 @@ static int i2o_cfg_passthru32(struct file *file, unsigned cmnd,
} }
rcode = i2o_msg_post_wait(c, msg, 60); rcode = i2o_msg_post_wait(c, msg, 60);
msg = NULL;
if (rcode) { if (rcode) {
reply[4] = ((u32) rcode) << 24; reply[4] = ((u32) rcode) << 24;
goto sg_list_cleanup; goto sg_list_cleanup;
} }
if (sg_offset) { if (sg_offset) {
u32 msg[I2O_OUTBOUND_MSG_FRAME_SIZE]; u32 rmsg[I2O_OUTBOUND_MSG_FRAME_SIZE];
/* Copy back the Scatter Gather buffers back to user space */ /* Copy back the Scatter Gather buffers back to user space */
u32 j; u32 j;
// TODO 64bit fix // TODO 64bit fix
...@@ -675,7 +680,7 @@ static int i2o_cfg_passthru32(struct file *file, unsigned cmnd, ...@@ -675,7 +680,7 @@ static int i2o_cfg_passthru32(struct file *file, unsigned cmnd,
int sg_size; int sg_size;
// re-acquire the original message to handle correctly the sg copy operation // re-acquire the original message to handle correctly the sg copy operation
memset(&msg, 0, I2O_OUTBOUND_MSG_FRAME_SIZE * 4); memset(&rmsg, 0, I2O_OUTBOUND_MSG_FRAME_SIZE * 4);
// get user msg size in u32s // get user msg size in u32s
if (get_user(size, &user_msg[0])) { if (get_user(size, &user_msg[0])) {
rcode = -EFAULT; rcode = -EFAULT;
...@@ -684,7 +689,7 @@ static int i2o_cfg_passthru32(struct file *file, unsigned cmnd, ...@@ -684,7 +689,7 @@ static int i2o_cfg_passthru32(struct file *file, unsigned cmnd,
size = size >> 16; size = size >> 16;
size *= 4; size *= 4;
/* Copy in the user's I2O command */ /* Copy in the user's I2O command */
if (copy_from_user(msg, user_msg, size)) { if (copy_from_user(rmsg, user_msg, size)) {
rcode = -EFAULT; rcode = -EFAULT;
goto sg_list_cleanup; goto sg_list_cleanup;
} }
...@@ -692,7 +697,7 @@ static int i2o_cfg_passthru32(struct file *file, unsigned cmnd, ...@@ -692,7 +697,7 @@ static int i2o_cfg_passthru32(struct file *file, unsigned cmnd,
(size - sg_offset * 4) / sizeof(struct sg_simple_element); (size - sg_offset * 4) / sizeof(struct sg_simple_element);
// TODO 64bit fix // TODO 64bit fix
sg = (struct sg_simple_element *)(msg + sg_offset); sg = (struct sg_simple_element *)(rmsg + sg_offset);
for (j = 0; j < sg_count; j++) { for (j = 0; j < sg_count; j++) {
/* Copy out the SG list to user's buffer if necessary */ /* Copy out the SG list to user's buffer if necessary */
if (! if (!
...@@ -714,7 +719,7 @@ static int i2o_cfg_passthru32(struct file *file, unsigned cmnd, ...@@ -714,7 +719,7 @@ static int i2o_cfg_passthru32(struct file *file, unsigned cmnd,
} }
} }
sg_list_cleanup: sg_list_cleanup:
/* Copy back the reply to user space */ /* Copy back the reply to user space */
if (reply_size) { if (reply_size) {
// we wrote our own values for context - now restore the user supplied ones // we wrote our own values for context - now restore the user supplied ones
...@@ -723,7 +728,6 @@ static int i2o_cfg_passthru32(struct file *file, unsigned cmnd, ...@@ -723,7 +728,6 @@ static int i2o_cfg_passthru32(struct file *file, unsigned cmnd,
"%s: Could not copy message context FROM user\n", "%s: Could not copy message context FROM user\n",
c->name); c->name);
rcode = -EFAULT; rcode = -EFAULT;
goto sg_list_cleanup;
} }
if (copy_to_user(user_reply, reply, reply_size)) { if (copy_to_user(user_reply, reply, reply_size)) {
printk(KERN_WARNING printk(KERN_WARNING
...@@ -731,12 +735,14 @@ static int i2o_cfg_passthru32(struct file *file, unsigned cmnd, ...@@ -731,12 +735,14 @@ static int i2o_cfg_passthru32(struct file *file, unsigned cmnd,
rcode = -EFAULT; rcode = -EFAULT;
} }
} }
for (i = 0; i < sg_index; i++) for (i = 0; i < sg_index; i++)
i2o_dma_free(&c->pdev->dev, &sg_list[i]); i2o_dma_free(&c->pdev->dev, &sg_list[i]);
cleanup: cleanup:
kfree(reply); kfree(reply);
out:
if (msg)
i2o_msg_nop(c, msg);
return rcode; return rcode;
} }
...@@ -793,8 +799,6 @@ static int i2o_cfg_passthru(unsigned long arg) ...@@ -793,8 +799,6 @@ static int i2o_cfg_passthru(unsigned long arg)
return -ENXIO; return -ENXIO;
} }
msg = i2o_msg_get_wait(c, I2O_TIMEOUT_MESSAGE_GET);
sb = c->status_block.virt; sb = c->status_block.virt;
if (get_user(size, &user_msg[0])) if (get_user(size, &user_msg[0]))
...@@ -810,12 +814,17 @@ static int i2o_cfg_passthru(unsigned long arg) ...@@ -810,12 +814,17 @@ static int i2o_cfg_passthru(unsigned long arg)
size <<= 2; // Convert to bytes size <<= 2; // Convert to bytes
msg = i2o_msg_get_wait(c, I2O_TIMEOUT_MESSAGE_GET);
if (IS_ERR(msg))
return PTR_ERR(msg);
rcode = -EFAULT;
/* Copy in the user's I2O command */ /* Copy in the user's I2O command */
if (copy_from_user(msg, user_msg, size)) if (copy_from_user(msg, user_msg, size))
return -EFAULT; goto out;
if (get_user(reply_size, &user_reply[0]) < 0) if (get_user(reply_size, &user_reply[0]) < 0)
return -EFAULT; goto out;
reply_size >>= 16; reply_size >>= 16;
reply_size <<= 2; reply_size <<= 2;
...@@ -824,7 +833,8 @@ static int i2o_cfg_passthru(unsigned long arg) ...@@ -824,7 +833,8 @@ static int i2o_cfg_passthru(unsigned long arg)
if (!reply) { if (!reply) {
printk(KERN_WARNING "%s: Could not allocate reply buffer\n", printk(KERN_WARNING "%s: Could not allocate reply buffer\n",
c->name); c->name);
return -ENOMEM; rcode = -ENOMEM;
goto out;
} }
sg_offset = (msg->u.head[0] >> 4) & 0x0f; sg_offset = (msg->u.head[0] >> 4) & 0x0f;
...@@ -891,13 +901,14 @@ static int i2o_cfg_passthru(unsigned long arg) ...@@ -891,13 +901,14 @@ static int i2o_cfg_passthru(unsigned long arg)
} }
rcode = i2o_msg_post_wait(c, msg, 60); rcode = i2o_msg_post_wait(c, msg, 60);
msg = NULL;
if (rcode) { if (rcode) {
reply[4] = ((u32) rcode) << 24; reply[4] = ((u32) rcode) << 24;
goto sg_list_cleanup; goto sg_list_cleanup;
} }
if (sg_offset) { if (sg_offset) {
u32 msg[128]; u32 rmsg[128];
/* Copy back the Scatter Gather buffers back to user space */ /* Copy back the Scatter Gather buffers back to user space */
u32 j; u32 j;
// TODO 64bit fix // TODO 64bit fix
...@@ -905,7 +916,7 @@ static int i2o_cfg_passthru(unsigned long arg) ...@@ -905,7 +916,7 @@ static int i2o_cfg_passthru(unsigned long arg)
int sg_size; int sg_size;
// re-acquire the original message to handle correctly the sg copy operation // re-acquire the original message to handle correctly the sg copy operation
memset(&msg, 0, I2O_OUTBOUND_MSG_FRAME_SIZE * 4); memset(&rmsg, 0, I2O_OUTBOUND_MSG_FRAME_SIZE * 4);
// get user msg size in u32s // get user msg size in u32s
if (get_user(size, &user_msg[0])) { if (get_user(size, &user_msg[0])) {
rcode = -EFAULT; rcode = -EFAULT;
...@@ -914,7 +925,7 @@ static int i2o_cfg_passthru(unsigned long arg) ...@@ -914,7 +925,7 @@ static int i2o_cfg_passthru(unsigned long arg)
size = size >> 16; size = size >> 16;
size *= 4; size *= 4;
/* Copy in the user's I2O command */ /* Copy in the user's I2O command */
if (copy_from_user(msg, user_msg, size)) { if (copy_from_user(rmsg, user_msg, size)) {
rcode = -EFAULT; rcode = -EFAULT;
goto sg_list_cleanup; goto sg_list_cleanup;
} }
...@@ -922,7 +933,7 @@ static int i2o_cfg_passthru(unsigned long arg) ...@@ -922,7 +933,7 @@ static int i2o_cfg_passthru(unsigned long arg)
(size - sg_offset * 4) / sizeof(struct sg_simple_element); (size - sg_offset * 4) / sizeof(struct sg_simple_element);
// TODO 64bit fix // TODO 64bit fix
sg = (struct sg_simple_element *)(msg + sg_offset); sg = (struct sg_simple_element *)(rmsg + sg_offset);
for (j = 0; j < sg_count; j++) { for (j = 0; j < sg_count; j++) {
/* Copy out the SG list to user's buffer if necessary */ /* Copy out the SG list to user's buffer if necessary */
if (! if (!
...@@ -944,7 +955,7 @@ static int i2o_cfg_passthru(unsigned long arg) ...@@ -944,7 +955,7 @@ static int i2o_cfg_passthru(unsigned long arg)
} }
} }
sg_list_cleanup: sg_list_cleanup:
/* Copy back the reply to user space */ /* Copy back the reply to user space */
if (reply_size) { if (reply_size) {
// we wrote our own values for context - now restore the user supplied ones // we wrote our own values for context - now restore the user supplied ones
...@@ -964,8 +975,11 @@ static int i2o_cfg_passthru(unsigned long arg) ...@@ -964,8 +975,11 @@ static int i2o_cfg_passthru(unsigned long arg)
for (i = 0; i < sg_index; i++) for (i = 0; i < sg_index; i++)
kfree(sg_list[i]); kfree(sg_list[i]);
cleanup: cleanup:
kfree(reply); kfree(reply);
out:
if (msg)
i2o_msg_nop(c, msg);
return rcode; return rcode;
} }
#endif #endif
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment