* [PATCH] depmod: fix errorpath memleaks in report cycles logic
@ 2017-03-20 10:09 Yauheni Kaliuta
2017-03-20 10:28 ` Yauheni Kaliuta
0 siblings, 1 reply; 3+ messages in thread
From: Yauheni Kaliuta @ 2017-03-20 10:09 UTC (permalink / raw)
To: linux-modules; +Cc: Lucas De Marchi
From: Yauheni Kaliuta <y.kaliuta@gmail.com>
The c7ce9f0c80f3d561078a78205a14c5ba7663cfdd commit (depmod:
handle nested loops) introduced a bunch of possible memory leaks
in error path. In the real world scenario it is not a problem,
since the utility quits if it detects any of the errors, but from
the programming point of view, it is not nice. So, add the
cleanups.
Signed-off-by: Yauheni Kaliuta <yauheni.kaliuta@redhat.com>
---
tools/depmod.c | 54 +++++++++++++++++++++++++++++++++++++-----------------
1 file changed, 37 insertions(+), 17 deletions(-)
diff --git a/tools/depmod.c b/tools/depmod.c
index 116adbeb14a0..7f9e9804787e 100644
--- a/tools/depmod.c
+++ b/tools/depmod.c
@@ -1481,10 +1481,10 @@ static void depmod_list_remove_data(struct kmod_list **list, void *data)
*list = l;
}
-static void depmod_report_one_cycle(struct depmod *depmod,
- struct vertex *vertex,
- struct kmod_list **roots,
- struct hash *loop_set)
+static int depmod_report_one_cycle(struct depmod *depmod,
+ struct vertex *vertex,
+ struct kmod_list **roots,
+ struct hash *loop_set)
{
const char sep[] = " -> ";
size_t sz;
@@ -1493,6 +1493,7 @@ static void depmod_report_one_cycle(struct depmod *depmod,
int i;
int n;
struct vertex *v;
+ int rc;
array_init(&reverse, 3);
@@ -1503,7 +1504,10 @@ static void depmod_report_one_cycle(struct depmod *depmod,
sz += v->mod->modnamesz - 1;
array_append(&reverse, v);
- hash_add(loop_set, v->mod->modname, NULL);
+ rc = hash_add(loop_set, v->mod->modname, NULL);
+ if (rc != 0)
+ return rc;
+ /* the hash will be freed where created */
}
sz += vertex->mod->modnamesz - 1;
@@ -1528,6 +1532,8 @@ static void depmod_report_one_cycle(struct depmod *depmod,
free(buf);
array_free_array(&reverse);
+
+ return 0;
}
static int depmod_report_cycles_from_root(struct depmod *depmod,
@@ -1545,17 +1551,18 @@ static int depmod_report_cycles_from_root(struct depmod *depmod,
struct mod *m;
struct mod **itr, **itr_end;
size_t is;
+ int ret = -ENOMEM;
root = vertex_new(root_mod, NULL);
if (root == NULL) {
ERR("No memory to report cycles\n");
- return -ENOMEM;
+ goto out;
}
l = kmod_list_append(free_list, root);
if (l == NULL) {
ERR("No memory to report cycles\n");
- return -ENOMEM;
+ goto out;
}
free_list = l;
@@ -1570,8 +1577,13 @@ static int depmod_report_cycles_from_root(struct depmod *depmod,
* from part of a loop or from a branch after a loop
*/
if (m->visited && m == root->mod) {
- depmod_report_one_cycle(depmod, vertex,
- roots, loop_set);
+ int rc;
+ rc = depmod_report_one_cycle(depmod, vertex,
+ roots, loop_set);
+ if (rc != 0) {
+ ret = rc;
+ goto out;
+ }
continue;
}
@@ -1598,7 +1610,7 @@ static int depmod_report_cycles_from_root(struct depmod *depmod,
v = vertex_new(dep, vertex);
if (v == NULL) {
ERR("No memory to report cycles\n");
- return -ENOMEM;
+ goto out;
}
assert(is < stack_size);
stack[is++] = v;
@@ -1606,12 +1618,15 @@ static int depmod_report_cycles_from_root(struct depmod *depmod,
l = kmod_list_append(free_list, v);
if (l == NULL) {
ERR("No memory to report cycles\n");
- return -ENOMEM;
+ goto out;
}
free_list = l;
}
}
+ ret = 0;
+
+out:
while (free_list) {
v = free_list->data;
l = kmod_list_remove(free_list);
@@ -1619,7 +1634,7 @@ static int depmod_report_cycles_from_root(struct depmod *depmod,
free(v);
}
- return 0;
+ return ret;
}
static void depmod_report_cycles(struct depmod *depmod, uint16_t n_mods,
@@ -1643,7 +1658,7 @@ static void depmod_report_cycles(struct depmod *depmod, uint16_t n_mods,
l = kmod_list_append(roots, m);
if (l == NULL) {
ERR("No memory to report cycles\n");
- return;
+ goto out_list;
}
roots = l;
n_r++;
@@ -1652,13 +1667,13 @@ static void depmod_report_cycles(struct depmod *depmod, uint16_t n_mods,
stack = malloc(n_r * sizeof(void *));
if (stack == NULL) {
ERR("No memory to report cycles\n");
- return;
+ goto out_list;
}
loop_set = hash_new(16, NULL);
if (loop_set == NULL) {
ERR("No memory to report cycles\n");
- return;
+ goto out_list;
}
while (roots != NULL) {
@@ -1670,14 +1685,19 @@ static void depmod_report_cycles(struct depmod *depmod, uint16_t n_mods,
&roots,
stack, n_r, loop_set);
if (err < 0)
- goto err;
+ goto out_hash;
}
num_cyclic = hash_get_count(loop_set);
ERR("Found %d modules in dependency cycles!\n", num_cyclic);
-err:
+out_hash:
hash_free(loop_set);
+out_list:
+ while (roots != NULL) {
+ /* no need to free data, come from outside */
+ roots = kmod_list_remove(roots);
+ }
}
static int depmod_calculate_dependencies(struct depmod *depmod)
--
2.9.2.368.g08bb350
^ permalink raw reply related [flat|nested] 3+ messages in thread
* Re: [PATCH] depmod: fix errorpath memleaks in report cycles logic
2017-03-20 10:09 [PATCH] depmod: fix errorpath memleaks in report cycles logic Yauheni Kaliuta
@ 2017-03-20 10:28 ` Yauheni Kaliuta
2017-05-09 6:48 ` Lucas De Marchi
0 siblings, 1 reply; 3+ messages in thread
From: Yauheni Kaliuta @ 2017-03-20 10:28 UTC (permalink / raw)
To: linux-modules; +Cc: Lucas De Marchi
Hi!
>>>>> On Mon, 20 Mar 2017 12:09:51 +0200, Yauheni Kaliuta wrote:
> From: Yauheni Kaliuta <y.kaliuta@gmail.com>
Ooops. Could you please remove the line? ;)
> The c7ce9f0c80f3d561078a78205a14c5ba7663cfdd commit (depmod:
> handle nested loops) introduced a bunch of possible memory leaks
> in error path. In the real world scenario it is not a problem,
> since the utility quits if it detects any of the errors, but from
> the programming point of view, it is not nice. So, add the
> cleanups.
> Signed-off-by: Yauheni Kaliuta <yauheni.kaliuta@redhat.com>
> ---
> tools/depmod.c | 54 +++++++++++++++++++++++++++++++++++++-----------------
> 1 file changed, 37 insertions(+), 17 deletions(-)
> diff --git a/tools/depmod.c b/tools/depmod.c
> index 116adbeb14a0..7f9e9804787e 100644
> --- a/tools/depmod.c
> +++ b/tools/depmod.c
> @@ -1481,10 +1481,10 @@ static void depmod_list_remove_data(struct kmod_list **list, void *data)
> *list = l;
> }
> -static void depmod_report_one_cycle(struct depmod *depmod,
> - struct vertex *vertex,
> - struct kmod_list **roots,
> - struct hash *loop_set)
> +static int depmod_report_one_cycle(struct depmod *depmod,
> + struct vertex *vertex,
> + struct kmod_list **roots,
> + struct hash *loop_set)
> {
> const char sep[] = " -> ";
> size_t sz;
> @@ -1493,6 +1493,7 @@ static void depmod_report_one_cycle(struct depmod *depmod,
> int i;
> int n;
> struct vertex *v;
> + int rc;
> array_init(&reverse, 3);
> @@ -1503,7 +1504,10 @@ static void depmod_report_one_cycle(struct depmod *depmod,
> sz += v->mod->modnamesz - 1;
> array_append(&reverse, v);
> - hash_add(loop_set, v->mod->modname, NULL);
> + rc = hash_add(loop_set, v->mod->modname, NULL);
> + if (rc != 0)
> + return rc;
> + /* the hash will be freed where created */
> }
> sz += vertex->mod->modnamesz - 1;
> @@ -1528,6 +1532,8 @@ static void depmod_report_one_cycle(struct depmod *depmod,
> free(buf);
> array_free_array(&reverse);
> +
> + return 0;
> }
> static int depmod_report_cycles_from_root(struct depmod *depmod,
> @@ -1545,17 +1551,18 @@ static int depmod_report_cycles_from_root(struct depmod *depmod,
> struct mod *m;
> struct mod **itr, **itr_end;
> size_t is;
> + int ret = -ENOMEM;
> root = vertex_new(root_mod, NULL);
> if (root == NULL) {
> ERR("No memory to report cycles\n");
> - return -ENOMEM;
> + goto out;
> }
> l = kmod_list_append(free_list, root);
> if (l == NULL) {
> ERR("No memory to report cycles\n");
> - return -ENOMEM;
> + goto out;
> }
> free_list = l;
> @@ -1570,8 +1577,13 @@ static int depmod_report_cycles_from_root(struct depmod *depmod,
> * from part of a loop or from a branch after a loop
> */
> if (m->visited && m == root->mod) {
> - depmod_report_one_cycle(depmod, vertex,
> - roots, loop_set);
> + int rc;
> + rc = depmod_report_one_cycle(depmod, vertex,
> + roots, loop_set);
> + if (rc != 0) {
> + ret = rc;
> + goto out;
> + }
> continue;
> }
> @@ -1598,7 +1610,7 @@ static int depmod_report_cycles_from_root(struct depmod *depmod,
> v = vertex_new(dep, vertex);
> if (v == NULL) {
> ERR("No memory to report cycles\n");
> - return -ENOMEM;
> + goto out;
> }
> assert(is < stack_size);
> stack[is++] = v;
> @@ -1606,12 +1618,15 @@ static int depmod_report_cycles_from_root(struct depmod *depmod,
> l = kmod_list_append(free_list, v);
> if (l == NULL) {
> ERR("No memory to report cycles\n");
> - return -ENOMEM;
> + goto out;
> }
> free_list = l;
> }
> }
> + ret = 0;
> +
> +out:
> while (free_list) {
> v = free_list->data;
> l = kmod_list_remove(free_list);
> @@ -1619,7 +1634,7 @@ static int depmod_report_cycles_from_root(struct depmod *depmod,
> free(v);
> }
> - return 0;
> + return ret;
> }
> static void depmod_report_cycles(struct depmod *depmod, uint16_t n_mods,
> @@ -1643,7 +1658,7 @@ static void depmod_report_cycles(struct depmod *depmod, uint16_t n_mods,
> l = kmod_list_append(roots, m);
> if (l == NULL) {
> ERR("No memory to report cycles\n");
> - return;
> + goto out_list;
> }
> roots = l;
> n_r++;
> @@ -1652,13 +1667,13 @@ static void depmod_report_cycles(struct depmod *depmod, uint16_t n_mods,
> stack = malloc(n_r * sizeof(void *));
> if (stack == NULL) {
> ERR("No memory to report cycles\n");
> - return;
> + goto out_list;
> }
> loop_set = hash_new(16, NULL);
> if (loop_set == NULL) {
> ERR("No memory to report cycles\n");
> - return;
> + goto out_list;
> }
> while (roots != NULL) {
> @@ -1670,14 +1685,19 @@ static void depmod_report_cycles(struct depmod *depmod, uint16_t n_mods,
> &roots,
> stack, n_r, loop_set);
> if (err < 0)
> - goto err;
> + goto out_hash;
> }
> num_cyclic = hash_get_count(loop_set);
> ERR("Found %d modules in dependency cycles!\n", num_cyclic);
> -err:
> +out_hash:
> hash_free(loop_set);
> +out_list:
> + while (roots != NULL) {
> + /* no need to free data, come from outside */
> + roots = kmod_list_remove(roots);
> + }
> }
> static int depmod_calculate_dependencies(struct depmod *depmod)
> --
> 2.9.2.368.g08bb350
> --
> To unsubscribe from this list: send the line "unsubscribe linux-modules" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
--
WBR,
Yauheni Kaliuta
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [PATCH] depmod: fix errorpath memleaks in report cycles logic
2017-03-20 10:28 ` Yauheni Kaliuta
@ 2017-05-09 6:48 ` Lucas De Marchi
0 siblings, 0 replies; 3+ messages in thread
From: Lucas De Marchi @ 2017-05-09 6:48 UTC (permalink / raw)
To: Yauheni Kaliuta; +Cc: linux-modules
On Mon, Mar 20, 2017 at 3:28 AM, Yauheni Kaliuta
<yauheni.kaliuta@redhat.com> wrote:
> Hi!
>
>>>>>> On Mon, 20 Mar 2017 12:09:51 +0200, Yauheni Kaliuta wrote:
>
> > From: Yauheni Kaliuta <y.kaliuta@gmail.com>
>
> Ooops. Could you please remove the line? ;)
Yep. Applied, thanks.
Lucas De Marchi
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2017-05-09 6:48 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-03-20 10:09 [PATCH] depmod: fix errorpath memleaks in report cycles logic Yauheni Kaliuta
2017-03-20 10:28 ` Yauheni Kaliuta
2017-05-09 6:48 ` Lucas De Marchi
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).