[libnftnl PATCH 0/4] Fix covscan-detected issues

[libnftnl PATCH 0/4] Fix covscan-detected issues

[Phil Sutter]

These are merely memleaks in error paths related to multi device support
in flowtables and netdev family chains.

Phil Sutter (4):
  flowtable: Fix memleak in error path of nftnl_flowtable_parse_devs()
  chain: Fix memleak in error path of nftnl_chain_parse_devs()
  flowtable: Correctly check realloc() call
  chain: Correctly check realloc() call

 src/chain.c     | 12 ++++++------
 src/flowtable.c | 12 ++++++------
 2 files changed, 12 insertions(+), 12 deletions(-)

-- 
2.24.0

[libnftnl PATCH 1/4] flowtable: Fix memleak in error path of nftnl_flowtable_parse_devs()

[Phil Sutter]

In error case, allocated dev_array is not freed.

Fixes: 7f99639dd9217 ("flowtable: device array dynamic allocation")
Signed-off-by: Phil Sutter <phil@nwl.cc>
---
 src/flowtable.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/src/flowtable.c b/src/flowtable.c
index 324e80f7e6ad6..db319434b51c0 100644
--- a/src/flowtable.c
+++ b/src/flowtable.c
@@ -419,6 +419,7 @@ static int nftnl_flowtable_parse_devs(struct nlattr *nest,
 err:
 	while (len--)
 		xfree(dev_array[len]);
+	xfree(dev_array);
 	return -1;
 }
 
-- 
2.24.0

[libnftnl PATCH 2/4] chain: Fix memleak in error path of nftnl_chain_parse_devs()

[Phil Sutter]

In error case, dev_array is not freed when it should.

Fixes: e3ac19b5ec162 ("chain: multi-device support")
Signed-off-by: Phil Sutter <phil@nwl.cc>
---
 src/chain.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/src/chain.c b/src/chain.c
index d4050d28e77d0..9cc8735a4936f 100644
--- a/src/chain.c
+++ b/src/chain.c
@@ -636,6 +636,7 @@ static int nftnl_chain_parse_devs(struct nlattr *nest, struct nftnl_chain *c)
 err:
 	while (len--)
 		xfree(dev_array[len]);
+	xfree(dev_array);
 	return -1;
 }
 
-- 
2.24.0

[libnftnl PATCH 3/4] flowtable: Correctly check realloc() call

[Phil Sutter]

If realloc() fails, it returns NULL but the original pointer is
untouchted and therefore still has to be freed. Unconditionally
overwriting the old pointer is therefore a bad idea, use a temporary
variable instead.

Fixes: 7f99639dd9217 ("flowtable: device array dynamic allocation")
Signed-off-by: Phil Sutter <phil@nwl.cc>
---
 src/flowtable.c | 11 +++++------
 1 file changed, 5 insertions(+), 6 deletions(-)

diff --git a/src/flowtable.c b/src/flowtable.c
index db319434b51c0..9ba3b6d9a3404 100644
--- a/src/flowtable.c
+++ b/src/flowtable.c
@@ -388,7 +388,7 @@ static int nftnl_flowtable_parse_hook_cb(const struct nlattr *attr, void *data)
 static int nftnl_flowtable_parse_devs(struct nlattr *nest,
 				      struct nftnl_flowtable *c)
 {
-	const char **dev_array;
+	const char **dev_array, **tmp;
 	int len = 0, size = 8;
 	struct nlattr *attr;
 
@@ -401,14 +401,13 @@ static int nftnl_flowtable_parse_devs(struct nlattr *nest,
 			goto err;
 		dev_array[len++] = strdup(mnl_attr_get_str(attr));
 		if (len >= size) {
-			dev_array = realloc(dev_array,
-					    size * 2 * sizeof(char *));
-			if (!dev_array)
+			tmp = realloc(dev_array, size * 2 * sizeof(char *));
+			if (!tmp)
 				goto err;
 
 			size *= 2;
-			memset(&dev_array[len], 0,
-			       (size - len) * sizeof(char *));
+			memset(&tmp[len], 0, (size - len) * sizeof(char *));
+			dev_array = tmp;
 		}
 	}
 
-- 
2.24.0

[libnftnl PATCH 4/4] chain: Correctly check realloc() call

[Phil Sutter]

If realloc() fails, it returns NULL but the original pointer is
untouchted and therefore still has to be freed. Unconditionally
overwriting the old pointer is therefore a bad idea, use a temporary
variable instead.

Fixes: e3ac19b5ec162 ("chain: multi-device support")
Signed-off-by: Phil Sutter <phil@nwl.cc>
---
 src/chain.c | 11 +++++------
 1 file changed, 5 insertions(+), 6 deletions(-)

diff --git a/src/chain.c b/src/chain.c
index 9cc8735a4936f..b9a16fc9b42df 100644
--- a/src/chain.c
+++ b/src/chain.c
@@ -605,7 +605,7 @@ static int nftnl_chain_parse_hook_cb(const struct nlattr *attr, void *data)
 
 static int nftnl_chain_parse_devs(struct nlattr *nest, struct nftnl_chain *c)
 {
-	const char **dev_array;
+	const char **dev_array, **tmp;
 	int len = 0, size = 8;
 	struct nlattr *attr;
 
@@ -618,14 +618,13 @@ static int nftnl_chain_parse_devs(struct nlattr *nest, struct nftnl_chain *c)
 			goto err;
 		dev_array[len++] = strdup(mnl_attr_get_str(attr));
 		if (len >= size) {
-			dev_array = realloc(dev_array,
-					    size * 2 * sizeof(char *));
-			if (!dev_array)
+			tmp = realloc(dev_array, size * 2 * sizeof(char *));
+			if (!tmp)
 				goto err;
 
 			size *= 2;
-			memset(&dev_array[len], 0,
-			       (size - len) * sizeof(char *));
+			memset(&tmp[len], 0, (size - len) * sizeof(char *));
+			dev_array = tmp;
 		}
 	}
 
-- 
2.24.0

Re: [libnftnl PATCH 0/4] Fix covscan-detected issues

[Pablo Neira Ayuso]

On Mon, Dec 02, 2019 at 11:23:57PM +0100, Phil Sutter wrote:
> These are merely memleaks in error paths related to multi device support
> in flowtables and netdev family chains.

Acked-by: Pablo Neira Ayuso <pablo@netfilter.org>

Thanks for fixing up these, Phil.