Linux-Modules Archive on lore.kernel.org
 help / Atom feed
* [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	[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, back to index

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

Linux-Modules Archive on lore.kernel.org

Archives are clonable:
	git clone --mirror https://lore.kernel.org/linux-modules/0 linux-modules/git/0.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 linux-modules linux-modules/ https://lore.kernel.org/linux-modules \
		linux-modules@vger.kernel.org linux-modules@archiver.kernel.org
	public-inbox-index linux-modules


Newsgroup available over NNTP:
	nntp://nntp.lore.kernel.org/org.kernel.vger.linux-modules


AGPL code for this site: git clone https://public-inbox.org/ public-inbox