Skip to content

Commit

Permalink
iscsi-target: Fix leak on failure in iscsi_copy_param_list()
Browse files Browse the repository at this point in the history
We leak memory if the allocations for 'new_param->name' or
'new_param->value' fail in iscsi_target_parameters.c::iscsi_copy_param_list()

We also do a lot of variable assignments that are completely pointless
if the allocations fail.

So, let's move the allocations before the assignments and also make
sure that we free whatever was allocated to one if the allocation fail.

There's also some small CodingStyle fixups in there (curly braces on
both branches of if statement, only one variable per line) since I was
in the area anyway. And finally, error messages in the function are
put on a single line for easy grep'abillity.

Signed-off-by: Jesper Juhl <jj@chaosbits.net>
Signed-off-by: Nicholas Bellinger <nab@linux-iscsi.org>
  • Loading branch information
Jesper Juhl authored and Nicholas Bellinger committed Aug 22, 2011
1 parent e1750ba commit 9be08c5
Showing 1 changed file with 16 additions and 27 deletions.
43 changes: 16 additions & 27 deletions drivers/target/iscsi/iscsi_target_parameters.c
Original file line number Diff line number Diff line change
Expand Up @@ -545,13 +545,13 @@ int iscsi_copy_param_list(
struct iscsi_param_list *src_param_list,
int leading)
{
struct iscsi_param *new_param = NULL, *param = NULL;
struct iscsi_param *param = NULL;
struct iscsi_param *new_param = NULL;
struct iscsi_param_list *param_list = NULL;

param_list = kzalloc(sizeof(struct iscsi_param_list), GFP_KERNEL);
if (!param_list) {
pr_err("Unable to allocate memory for"
" struct iscsi_param_list.\n");
pr_err("Unable to allocate memory for struct iscsi_param_list.\n");
goto err_out;
}
INIT_LIST_HEAD(&param_list->param_list);
Expand All @@ -567,8 +567,17 @@ int iscsi_copy_param_list(

new_param = kzalloc(sizeof(struct iscsi_param), GFP_KERNEL);
if (!new_param) {
pr_err("Unable to allocate memory for"
" struct iscsi_param.\n");
pr_err("Unable to allocate memory for struct iscsi_param.\n");
goto err_out;
}

new_param->name = kstrdup(param->name, GFP_KERNEL);
new_param->value = kstrdup(param->value, GFP_KERNEL);
if (!new_param->value || !new_param->name) {
kfree(new_param->value);
kfree(new_param->name);
kfree(new_param);
pr_err("Unable to allocate memory for parameter name/value.\n");
goto err_out;
}

Expand All @@ -580,32 +589,12 @@ int iscsi_copy_param_list(
new_param->use = param->use;
new_param->type_range = param->type_range;

new_param->name = kzalloc(strlen(param->name) + 1, GFP_KERNEL);
if (!new_param->name) {
pr_err("Unable to allocate memory for"
" parameter name.\n");
goto err_out;
}

new_param->value = kzalloc(strlen(param->value) + 1,
GFP_KERNEL);
if (!new_param->value) {
pr_err("Unable to allocate memory for"
" parameter value.\n");
goto err_out;
}

memcpy(new_param->name, param->name, strlen(param->name));
new_param->name[strlen(param->name)] = '\0';
memcpy(new_param->value, param->value, strlen(param->value));
new_param->value[strlen(param->value)] = '\0';

list_add_tail(&new_param->p_list, &param_list->param_list);
}

if (!list_empty(&param_list->param_list))
if (!list_empty(&param_list->param_list)) {
*dst_param_list = param_list;
else {
} else {
pr_err("No parameters allocated.\n");
goto err_out;
}
Expand Down

0 comments on commit 9be08c5

Please sign in to comment.