linux-mtd.lists.infradead.org archive mirror
 help / color / mirror / Atom feed
* [PATCH 00/15] mtd-utils: cleanup resource leaks
@ 2019-11-10 15:30 David Oberhollenzer
  2019-11-10 15:30 ` [PATCH 01/15] mkfs.ubifs: close file descriptor in add_file error path David Oberhollenzer
                   ` (14 more replies)
  0 siblings, 15 replies; 16+ messages in thread
From: David Oberhollenzer @ 2019-11-10 15:30 UTC (permalink / raw)
  To: linux-mtd; +Cc: richard

Hi,

back in 2016 or so I already fixed almost all of the warnings
reported by gcc for mtd-utils. Since then, gcc has gotten better
[citation needed] and with its colorfull diagnostics, compiling
mtd-utils finally makes your terminal look like a slot machine
again.

Furthermore, mtd-utils is also on coverity scan since at least 2017:

https://scan.coverity.com/projects/mtd-utils

I've been procrastinating lately and finally got around to looking
into this again.

This patch set tries to eliminate the largest single group of issues
that generate a lot of noise in the diagnostics: resource leaks.

It's not *that* critical for mtd-utils, it should be easy to fix and
it's also the largest chunk of issues since mtd-utils historically
didn't care at all and leak resources left and right.

Please tell me if my fixes break something elsewhere or my assumptions
about the intended behaviour are flawed. Otherwise I will merge this
at the end of next week and move on to the next batch.

Thanks,

David


______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/

^ permalink raw reply	[flat|nested] 16+ messages in thread

* [PATCH 01/15] mkfs.ubifs: close file descriptor in add_file error path
  2019-11-10 15:30 [PATCH 00/15] mtd-utils: cleanup resource leaks David Oberhollenzer
@ 2019-11-10 15:30 ` David Oberhollenzer
  2019-11-10 15:30 ` [PATCH 02/15] mkfs.ubifs: don't leak copied command line arguments David Oberhollenzer
                   ` (13 subsequent siblings)
  14 siblings, 0 replies; 16+ messages in thread
From: David Oberhollenzer @ 2019-11-10 15:30 UTC (permalink / raw)
  To: linux-mtd; +Cc: richard, David Oberhollenzer

Signed-off-by: David Oberhollenzer <david.oberhollenzer@sigma-star.at>
---
 ubifs-utils/mkfs.ubifs/mkfs.ubifs.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/ubifs-utils/mkfs.ubifs/mkfs.ubifs.c b/ubifs-utils/mkfs.ubifs/mkfs.ubifs.c
index 72ae1b4..bf1290f 100644
--- a/ubifs-utils/mkfs.ubifs/mkfs.ubifs.c
+++ b/ubifs-utils/mkfs.ubifs/mkfs.ubifs.c
@@ -1873,8 +1873,10 @@ static int add_file(const char *path_name, struct stat *st, ino_t inum,
 			dn->compr_size = 0;
 		} else {
 			ret = encrypt_data_node(fctx, block_no, dn, out_len);
-			if (ret < 0)
+			if (ret < 0) {
+				close(fd);
 				return ret;
+			}
 			out_len = ret;
 		}
 
-- 
2.21.0


______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/

^ permalink raw reply related	[flat|nested] 16+ messages in thread

* [PATCH 02/15] mkfs.ubifs: don't leak copied command line arguments
  2019-11-10 15:30 [PATCH 00/15] mtd-utils: cleanup resource leaks David Oberhollenzer
  2019-11-10 15:30 ` [PATCH 01/15] mkfs.ubifs: close file descriptor in add_file error path David Oberhollenzer
@ 2019-11-10 15:30 ` David Oberhollenzer
  2019-11-10 15:30 ` [PATCH 03/15] mkfs.ubifs: free derived fscrypt context in add_directory error paths David Oberhollenzer
                   ` (12 subsequent siblings)
  14 siblings, 0 replies; 16+ messages in thread
From: David Oberhollenzer @ 2019-11-10 15:30 UTC (permalink / raw)
  To: linux-mtd; +Cc: richard, David Oberhollenzer

For some command line flags, the argument string is copied. Simply
writing over the buffer leads to a resource leak if the same flag
is specified on the command line more than once.

This patch adds a free() call to the old buffer before overwriting
it with the new copy.

Signed-off-by: David Oberhollenzer <david.oberhollenzer@sigma-star.at>
---
 ubifs-utils/mkfs.ubifs/mkfs.ubifs.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/ubifs-utils/mkfs.ubifs/mkfs.ubifs.c b/ubifs-utils/mkfs.ubifs/mkfs.ubifs.c
index bf1290f..ea0afee 100644
--- a/ubifs-utils/mkfs.ubifs/mkfs.ubifs.c
+++ b/ubifs-utils/mkfs.ubifs/mkfs.ubifs.c
@@ -580,6 +580,7 @@ static int get_options(int argc, char**argv)
 		switch (opt) {
 		case 'r':
 		case 'd':
+			free(root);
 			root_len = strlen(optarg);
 			root = xmalloc(root_len + 2);
 
@@ -726,6 +727,7 @@ static int get_options(int argc, char**argv)
 			do_create_inum_attr = 1;
 			break;
 		case 's':
+			free(context);
 			context_len = strlen(optarg);
 			context = (char *) xmalloc(context_len + 1);
 			if (!context)
-- 
2.21.0


______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/

^ permalink raw reply related	[flat|nested] 16+ messages in thread

* [PATCH 03/15] mkfs.ubifs: free derived fscrypt context in add_directory error paths
  2019-11-10 15:30 [PATCH 00/15] mtd-utils: cleanup resource leaks David Oberhollenzer
  2019-11-10 15:30 ` [PATCH 01/15] mkfs.ubifs: close file descriptor in add_file error path David Oberhollenzer
  2019-11-10 15:30 ` [PATCH 02/15] mkfs.ubifs: don't leak copied command line arguments David Oberhollenzer
@ 2019-11-10 15:30 ` David Oberhollenzer
  2019-11-10 15:30 ` [PATCH 04/15] mkfs.ubifs: don't leak hastable iterators David Oberhollenzer
                   ` (11 subsequent siblings)
  14 siblings, 0 replies; 16+ messages in thread
From: David Oberhollenzer @ 2019-11-10 15:30 UTC (permalink / raw)
  To: linux-mtd; +Cc: richard, David Oberhollenzer

Signed-off-by: David Oberhollenzer <david.oberhollenzer@sigma-star.at>
---
 ubifs-utils/mkfs.ubifs/mkfs.ubifs.c | 33 ++++++++++++++++++++++-------
 1 file changed, 25 insertions(+), 8 deletions(-)

diff --git a/ubifs-utils/mkfs.ubifs/mkfs.ubifs.c b/ubifs-utils/mkfs.ubifs/mkfs.ubifs.c
index ea0afee..87f7b4f 100644
--- a/ubifs-utils/mkfs.ubifs/mkfs.ubifs.c
+++ b/ubifs-utils/mkfs.ubifs/mkfs.ubifs.c
@@ -2099,24 +2099,32 @@ static int add_directory(const char *dir_name, ino_t dir_inum, struct stat *st,
 
 		if (S_ISDIR(dent_st.st_mode)) {
 			err = add_directory(name, inum, &dent_st, 1, new_fctx);
-			if (err)
+			if (err) {
+				free_fscrypt_context(new_fctx);
 				goto out_free;
+			}
 			nlink += 1;
 			type = UBIFS_ITYPE_DIR;
 		} else {
 			err = add_non_dir(name, &inum, 0, &type,
 					  &dent_st, new_fctx);
-			if (err)
+			if (err) {
+				free_fscrypt_context(new_fctx);
 				goto out_free;
+			}
 		}
 
 		err = create_inum_attr(inum, name);
-		if (err)
+		if (err) {
+			free_fscrypt_context(new_fctx);
 			goto out_free;
+		}
 
 		err = add_dent_node(dir_inum, entry->d_name, inum, type, fctx);
-		if (err)
+		if (err) {
+			free_fscrypt_context(new_fctx);
 			goto out_free;
+		}
 		size += ALIGN(UBIFS_DENT_NODE_SZ + strlen(entry->d_name) + 1,
 			      8);
 
@@ -2162,24 +2170,33 @@ static int add_directory(const char *dir_name, ino_t dir_inum, struct stat *st,
 
 		if (S_ISDIR(nh_elt->mode)) {
 			err = add_directory(name, inum, &fake_st, 0, new_fctx);
-			if (err)
+			if (err) {
+				free_fscrypt_context(new_fctx);
 				goto out_free;
+			}
 			nlink += 1;
 			type = UBIFS_ITYPE_DIR;
 		} else {
 			err = add_non_dir(name, &inum, 0, &type,
 					  &fake_st, new_fctx);
-			if (err)
+			if (err) {
+				free_fscrypt_context(new_fctx);
 				goto out_free;
+			}
 		}
 
 		err = create_inum_attr(inum, name);
-		if (err)
+		if (err) {
+			free_fscrypt_context(new_fctx);
 			goto out_free;
+		}
 
 		err = add_dent_node(dir_inum, nh_elt->name, inum, type, fctx);
-		if (err)
+		if (err) {
+			free_fscrypt_context(new_fctx);
 			goto out_free;
+		}
+
 		size += ALIGN(UBIFS_DENT_NODE_SZ + strlen(nh_elt->name) + 1, 8);
 
 		nh_elt = next_name_htbl_element(ph_elt, &itr);
-- 
2.21.0


______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/

^ permalink raw reply related	[flat|nested] 16+ messages in thread

* [PATCH 04/15] mkfs.ubifs: don't leak hastable iterators
  2019-11-10 15:30 [PATCH 00/15] mtd-utils: cleanup resource leaks David Oberhollenzer
                   ` (2 preceding siblings ...)
  2019-11-10 15:30 ` [PATCH 03/15] mkfs.ubifs: free derived fscrypt context in add_directory error paths David Oberhollenzer
@ 2019-11-10 15:30 ` David Oberhollenzer
  2019-11-10 15:30 ` [PATCH 05/15] mkfs.ubifs: don't leak temporary buffers David Oberhollenzer
                   ` (10 subsequent siblings)
  14 siblings, 0 replies; 16+ messages in thread
From: David Oberhollenzer @ 2019-11-10 15:30 UTC (permalink / raw)
  To: linux-mtd; +Cc: richard, David Oberhollenzer

Signed-off-by: David Oberhollenzer <david.oberhollenzer@sigma-star.at>
---
 ubifs-utils/mkfs.ubifs/devtable.c   | 1 +
 ubifs-utils/mkfs.ubifs/mkfs.ubifs.c | 3 ++-
 2 files changed, 3 insertions(+), 1 deletion(-)

diff --git a/ubifs-utils/mkfs.ubifs/devtable.c b/ubifs-utils/mkfs.ubifs/devtable.c
index 10faaca..0afea90 100644
--- a/ubifs-utils/mkfs.ubifs/devtable.c
+++ b/ubifs-utils/mkfs.ubifs/devtable.c
@@ -525,6 +525,7 @@ void free_devtable_info(void)
 			 */
 			hashtable_destroy(ph_elt->name_htbl, 1);
 		} while (hashtable_iterator_advance(ph_itr));
+		free(ph_itr);
 	}
 	hashtable_destroy(path_htbl, 1);
 }
diff --git a/ubifs-utils/mkfs.ubifs/mkfs.ubifs.c b/ubifs-utils/mkfs.ubifs/mkfs.ubifs.c
index 87f7b4f..4247270 100644
--- a/ubifs-utils/mkfs.ubifs/mkfs.ubifs.c
+++ b/ubifs-utils/mkfs.ubifs/mkfs.ubifs.c
@@ -2013,7 +2013,7 @@ static int add_directory(const char *dir_name, ino_t dir_inum, struct stat *st,
 	unsigned int nlink = 2;
 	struct path_htbl_element *ph_elt;
 	struct name_htbl_element *nh_elt = NULL;
-	struct hashtable_itr *itr;
+	struct hashtable_itr *itr = NULL;
 	ino_t inum;
 	unsigned char type;
 	unsigned long long dir_creat_sqnum = ++c->max_sqnum;
@@ -2218,6 +2218,7 @@ static int add_directory(const char *dir_name, ino_t dir_inum, struct stat *st,
 	return 0;
 
 out_free:
+	free(itr);
 	free(name);
 	if (existing)
 		closedir(dir);
-- 
2.21.0


______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/

^ permalink raw reply related	[flat|nested] 16+ messages in thread

* [PATCH 05/15] mkfs.ubifs: don't leak temporary buffers
  2019-11-10 15:30 [PATCH 00/15] mtd-utils: cleanup resource leaks David Oberhollenzer
                   ` (3 preceding siblings ...)
  2019-11-10 15:30 ` [PATCH 04/15] mkfs.ubifs: don't leak hastable iterators David Oberhollenzer
@ 2019-11-10 15:30 ` David Oberhollenzer
  2019-11-10 15:30 ` [PATCH 06/15] mkfs.ubifs: propperly cleanup in ALL interpret_table_entry error paths David Oberhollenzer
                   ` (9 subsequent siblings)
  14 siblings, 0 replies; 16+ messages in thread
From: David Oberhollenzer @ 2019-11-10 15:30 UTC (permalink / raw)
  To: linux-mtd; +Cc: richard, David Oberhollenzer

Signed-off-by: David Oberhollenzer <david.oberhollenzer@sigma-star.at>
---
 ubifs-utils/mkfs.ubifs/crypto.c  |  9 ++++++---
 ubifs-utils/mkfs.ubifs/fscrypt.c | 18 +++++++++++++++---
 2 files changed, 21 insertions(+), 6 deletions(-)

diff --git a/ubifs-utils/mkfs.ubifs/crypto.c b/ubifs-utils/mkfs.ubifs/crypto.c
index d31bd2a..19c445e 100644
--- a/ubifs-utils/mkfs.ubifs/crypto.c
+++ b/ubifs-utils/mkfs.ubifs/crypto.c
@@ -118,23 +118,26 @@ static ssize_t gen_essiv_salt(const void *iv, size_t iv_len, const void *key, si
 	cipher = EVP_aes_256_ecb();
 	if (!cipher) {
 		errmsg("OpenSSL: Cipher AES-256-ECB is not supported");
-		return -1;
+		goto fail;
 	}
 
 	if (do_hash(EVP_sha256(), key, key_len, sha256) != 0) {
 		errmsg("sha256 failed");
-		return -1;
+		goto fail;
 	}
 
 	ret = do_encrypt(cipher, iv, iv_len, sha256, EVP_MD_size(EVP_sha256()), NULL, 0, salt);
 	if (ret != iv_len) {
 		errmsg("Unable to compute ESSIV salt, return value %zi instead of %zi", ret, iv_len);
-		return -1;
+		goto fail;
 	}
 
 	free(sha256);
 
 	return ret;
+fail:
+	free(sha256);
+	return -1;
 }
 
 static ssize_t encrypt_block(const void *plaintext, size_t size,
diff --git a/ubifs-utils/mkfs.ubifs/fscrypt.c b/ubifs-utils/mkfs.ubifs/fscrypt.c
index 118c11c..b75bdf7 100644
--- a/ubifs-utils/mkfs.ubifs/fscrypt.c
+++ b/ubifs-utils/mkfs.ubifs/fscrypt.c
@@ -106,13 +106,19 @@ int encrypt_path(void **outbuf, void *data, unsigned int data_len,
 	memcpy(inbuf, data, data_len);
 
 	crypt_key = calc_fscrypt_subkey(fctx);
-	if (!crypt_key)
+	if (!crypt_key) {
+		free(inbuf);
+		free(*outbuf);
 		return err_msg("could not compute subkey");
+	}
 
 	ret = fscrypt_cipher->encrypt_fname(inbuf, cryptlen,
 					    crypt_key, *outbuf);
-	if (ret < 0)
+	if (ret < 0) {
+		free(inbuf);
+		free(*outbuf);
 		return err_msg("could not encrypt filename");
+	}
 
 	free(crypt_key);
 	free(inbuf);
@@ -133,13 +139,19 @@ int encrypt_data_node(struct fscrypt_context *fctx, unsigned int block_no,
 	memcpy(inbuf, &dn->data, length);
 
 	crypt_key = calc_fscrypt_subkey(fctx);
-	if (!crypt_key)
+	if (!crypt_key) {
+		free(inbuf);
+		free(outbuf);
 		return err_msg("could not compute subkey");
+	}
 
 	ret = fscrypt_cipher->encrypt_block(inbuf, pad_len,
 					    crypt_key, block_no,
 					    outbuf);
 	if (ret != pad_len) {
+		free(inbuf);
+		free(outbuf);
+		free(crypt_key);
 		return err_msg("encrypt_block returned %zi "
 				"instead of %zi", ret, pad_len);
 	}
-- 
2.21.0


______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/

^ permalink raw reply related	[flat|nested] 16+ messages in thread

* [PATCH 06/15] mkfs.ubifs: propperly cleanup in ALL interpret_table_entry error paths
  2019-11-10 15:30 [PATCH 00/15] mtd-utils: cleanup resource leaks David Oberhollenzer
                   ` (4 preceding siblings ...)
  2019-11-10 15:30 ` [PATCH 05/15] mkfs.ubifs: don't leak temporary buffers David Oberhollenzer
@ 2019-11-10 15:30 ` David Oberhollenzer
  2019-11-10 15:30 ` [PATCH 07/15] mkfs.jffs2: don't leak temporary buffer if readlink fails David Oberhollenzer
                   ` (8 subsequent siblings)
  14 siblings, 0 replies; 16+ messages in thread
From: David Oberhollenzer @ 2019-11-10 15:30 UTC (permalink / raw)
  To: linux-mtd; +Cc: richard, David Oberhollenzer

Signed-off-by: David Oberhollenzer <david.oberhollenzer@sigma-star.at>
---
 ubifs-utils/mkfs.ubifs/devtable.c | 12 ++++++++----
 1 file changed, 8 insertions(+), 4 deletions(-)

diff --git a/ubifs-utils/mkfs.ubifs/devtable.c b/ubifs-utils/mkfs.ubifs/devtable.c
index 0afea90..aa815fb 100644
--- a/ubifs-utils/mkfs.ubifs/devtable.c
+++ b/ubifs-utils/mkfs.ubifs/devtable.c
@@ -219,8 +219,10 @@ static int interpret_table_entry(const char *line)
 		}
 	}
 
-	if (increment != 0 && count == 0)
-		return err_msg("count cannot be zero if increment is non-zero");
+	if (increment != 0 && count == 0) {
+		err_msg("count cannot be zero if increment is non-zero");
+		goto out_free;
+	}
 
 	/*
 	 * Add the file/directory/device node (last component of the path) to
@@ -245,8 +247,10 @@ static int interpret_table_entry(const char *line)
 		dbg_msg(3, "inserting '%s' into name hash table (major %d, minor %d)",
 			name, major(nh_elt->dev), minor(nh_elt->dev));
 
-		if (hashtable_search(ph_elt->name_htbl, name))
-			return err_msg("'%s' is referred twice", buf);
+		if (hashtable_search(ph_elt->name_htbl, name)) {
+			err_msg("'%s' is referred twice", buf);
+			goto out_free;
+		}
 
 		nh_elt->name = name;
 		if (!hashtable_insert(ph_elt->name_htbl, name, nh_elt)) {
-- 
2.21.0


______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/

^ permalink raw reply related	[flat|nested] 16+ messages in thread

* [PATCH 07/15] mkfs.jffs2: don't leak temporary buffer if readlink fails
  2019-11-10 15:30 [PATCH 00/15] mtd-utils: cleanup resource leaks David Oberhollenzer
                   ` (5 preceding siblings ...)
  2019-11-10 15:30 ` [PATCH 06/15] mkfs.ubifs: propperly cleanup in ALL interpret_table_entry error paths David Oberhollenzer
@ 2019-11-10 15:30 ` David Oberhollenzer
  2019-11-10 15:30 ` [PATCH 08/15] libmtd: don't leak temporary buffers David Oberhollenzer
                   ` (7 subsequent siblings)
  14 siblings, 0 replies; 16+ messages in thread
From: David Oberhollenzer @ 2019-11-10 15:30 UTC (permalink / raw)
  To: linux-mtd; +Cc: richard, David Oberhollenzer

Signed-off-by: David Oberhollenzer <david.oberhollenzer@sigma-star.at>
---
 jffsX-utils/mkfs.jffs2.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/jffsX-utils/mkfs.jffs2.c b/jffsX-utils/mkfs.jffs2.c
index 0661786..f46cc22 100644
--- a/jffsX-utils/mkfs.jffs2.c
+++ b/jffsX-utils/mkfs.jffs2.c
@@ -151,6 +151,7 @@ static char *xreadlink(const char *path)
 		readsize = readlink(path, buf, bufsize); /* 1st try */
 		if (readsize == -1) {
 			sys_errmsg("%s:%s", PROGRAM_NAME, path);
+			free(buf);
 			return NULL;
 		}
 	}
-- 
2.21.0


______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/

^ permalink raw reply related	[flat|nested] 16+ messages in thread

* [PATCH 08/15] libmtd: don't leak temporary buffers
  2019-11-10 15:30 [PATCH 00/15] mtd-utils: cleanup resource leaks David Oberhollenzer
                   ` (6 preceding siblings ...)
  2019-11-10 15:30 ` [PATCH 07/15] mkfs.jffs2: don't leak temporary buffer if readlink fails David Oberhollenzer
@ 2019-11-10 15:30 ` David Oberhollenzer
  2019-11-10 15:30 ` [PATCH 09/15] ftl_check: " David Oberhollenzer
                   ` (6 subsequent siblings)
  14 siblings, 0 replies; 16+ messages in thread
From: David Oberhollenzer @ 2019-11-10 15:30 UTC (permalink / raw)
  To: linux-mtd; +Cc: richard, David Oberhollenzer

Signed-off-by: David Oberhollenzer <david.oberhollenzer@sigma-star.at>
---
 lib/libmtd.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/lib/libmtd.c b/lib/libmtd.c
index 5e3ac50..564e5c0 100644
--- a/lib/libmtd.c
+++ b/lib/libmtd.c
@@ -1140,6 +1140,7 @@ static int legacy_auto_oob_layout(const struct mtd_dev_info *mtd, int fd,
 		memcpy(oob + start, tmp_buf + start, len);
 	}
 
+	free(tmp_buf);
 	return 0;
 }
 
-- 
2.21.0


______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/

^ permalink raw reply related	[flat|nested] 16+ messages in thread

* [PATCH 09/15] ftl_check: don't leak temporary buffers
  2019-11-10 15:30 [PATCH 00/15] mtd-utils: cleanup resource leaks David Oberhollenzer
                   ` (7 preceding siblings ...)
  2019-11-10 15:30 ` [PATCH 08/15] libmtd: don't leak temporary buffers David Oberhollenzer
@ 2019-11-10 15:30 ` David Oberhollenzer
  2019-11-10 15:30 ` [PATCH 10/15] ftl_format: " David Oberhollenzer
                   ` (5 subsequent siblings)
  14 siblings, 0 replies; 16+ messages in thread
From: David Oberhollenzer @ 2019-11-10 15:30 UTC (permalink / raw)
  To: linux-mtd; +Cc: richard, David Oberhollenzer

Signed-off-by: David Oberhollenzer <david.oberhollenzer@sigma-star.at>
---
 misc-utils/ftl_check.c | 10 ++++++----
 1 file changed, 6 insertions(+), 4 deletions(-)

diff --git a/misc-utils/ftl_check.c b/misc-utils/ftl_check.c
index e854922..5a04155 100644
--- a/misc-utils/ftl_check.c
+++ b/misc-utils/ftl_check.c
@@ -75,7 +75,7 @@ static void check_partition(int fd)
 	erase_unit_header_t hdr, hdr2;
 	off_t i;
 	u_int j, nbam, *bam;
-	int control, data, free, deleted;
+	int control, data, blk_free, deleted;
 
 	/* Get partition size, block size */
 	if (ioctl(fd, MEMGETINFO, &mtd) != 0) {
@@ -150,10 +150,10 @@ static void check_partition(int fd)
 				perror("read failed");
 				break;
 			}
-			free = deleted = control = data = 0;
+			blk_free = deleted = control = data = 0;
 			for (j = 0; j < nbam; j++) {
 				if (BLOCK_FREE(le32_to_cpu(bam[j])))
-					free++;
+					blk_free++;
 				else if (BLOCK_DELETED(le32_to_cpu(bam[j])))
 					deleted++;
 				else switch (BLOCK_TYPE(le32_to_cpu(bam[j]))) {
@@ -163,9 +163,11 @@ static void check_partition(int fd)
 				}
 			}
 			printf("  Block allocation: %d control, %d data, %d free,"
-					" %d deleted\n", control, data, free, deleted);
+					" %d deleted\n", control, data, blk_free, deleted);
 		}
 	}
+
+	free(bam);
 } /* format_partition */
 
 /* Show usage information */
-- 
2.21.0


______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/

^ permalink raw reply related	[flat|nested] 16+ messages in thread

* [PATCH 10/15] ftl_format: don't leak temporary buffers
  2019-11-10 15:30 [PATCH 00/15] mtd-utils: cleanup resource leaks David Oberhollenzer
                   ` (8 preceding siblings ...)
  2019-11-10 15:30 ` [PATCH 09/15] ftl_check: " David Oberhollenzer
@ 2019-11-10 15:30 ` David Oberhollenzer
  2019-11-10 15:30 ` [PATCH 11/15] ubiformat: don't leak file descriptors David Oberhollenzer
                   ` (4 subsequent siblings)
  14 siblings, 0 replies; 16+ messages in thread
From: David Oberhollenzer @ 2019-11-10 15:30 UTC (permalink / raw)
  To: linux-mtd; +Cc: richard, David Oberhollenzer

Signed-off-by: David Oberhollenzer <david.oberhollenzer@sigma-star.at>
---
 misc-utils/ftl_format.c | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/misc-utils/ftl_format.c b/misc-utils/ftl_format.c
index 649984b..bf3c8f2 100644
--- a/misc-utils/ftl_format.c
+++ b/misc-utils/ftl_format.c
@@ -191,6 +191,7 @@ static int format_partition(int fd, int quiet, int interrogate,
 				fflush(stdout);
 			}
 			perror("block erase failed");
+			free(bam);
 			return -1;
 		}
 		erase.start += erase.length;
@@ -246,6 +247,9 @@ static int format_partition(int fd, int quiet, int interrogate,
 			break;
 		}
 	}
+
+	free(bam);
+
 	if (i < le16_to_cpu(hdr.NumEraseUnits))
 		return -1;
 	else
-- 
2.21.0


______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/

^ permalink raw reply related	[flat|nested] 16+ messages in thread

* [PATCH 11/15] ubiformat: don't leak file descriptors
  2019-11-10 15:30 [PATCH 00/15] mtd-utils: cleanup resource leaks David Oberhollenzer
                   ` (9 preceding siblings ...)
  2019-11-10 15:30 ` [PATCH 10/15] ftl_format: " David Oberhollenzer
@ 2019-11-10 15:30 ` David Oberhollenzer
  2019-11-10 15:30 ` [PATCH 12/15] nanddump: don't leak copied command line arguments David Oberhollenzer
                   ` (3 subsequent siblings)
  14 siblings, 0 replies; 16+ messages in thread
From: David Oberhollenzer @ 2019-11-10 15:30 UTC (permalink / raw)
  To: linux-mtd; +Cc: richard, David Oberhollenzer

The original code had a 'goto out_close' directly after a return error
code, which is obviously not what was intended.

Signed-off-by: David Oberhollenzer <david.oberhollenzer@sigma-star.at>
---
 ubi-utils/ubiformat.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/ubi-utils/ubiformat.c b/ubi-utils/ubiformat.c
index a90627c..64afad2 100644
--- a/ubi-utils/ubiformat.c
+++ b/ubi-utils/ubiformat.c
@@ -426,8 +426,8 @@ static int flash_image(libmtd_t libmtd, const struct mtd_dev_info *mtd,
 	}
 
 	if (st_size % mtd->eb_size) {
-		return sys_errmsg("file \"%s\" (size %lld bytes) is not multiple of ""eraseblock size (%d bytes)",
-				  args.image, (long long)st_size, mtd->eb_size);
+		sys_errmsg("file \"%s\" (size %lld bytes) is not multiple of ""eraseblock size (%d bytes)",
+			  args.image, (long long)st_size, mtd->eb_size);
 		goto out_close;
 	}
 
-- 
2.21.0


______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/

^ permalink raw reply related	[flat|nested] 16+ messages in thread

* [PATCH 12/15] nanddump: don't leak copied command line arguments
  2019-11-10 15:30 [PATCH 00/15] mtd-utils: cleanup resource leaks David Oberhollenzer
                   ` (10 preceding siblings ...)
  2019-11-10 15:30 ` [PATCH 11/15] ubiformat: don't leak file descriptors David Oberhollenzer
@ 2019-11-10 15:30 ` David Oberhollenzer
  2019-11-10 15:30 ` [PATCH 13/15] mtd_debug: cleanup error handling in flash_to_file David Oberhollenzer
                   ` (2 subsequent siblings)
  14 siblings, 0 replies; 16+ messages in thread
From: David Oberhollenzer @ 2019-11-10 15:30 UTC (permalink / raw)
  To: linux-mtd; +Cc: richard, David Oberhollenzer

For some command line flags, the argument string is copied. Simply
writing over the buffer leads to a resource leak if the same flag
is specified on the command line more than once.

This patch adds a free() call to the old buffer before overwriting
it with the new copy.

Signed-off-by: David Oberhollenzer <david.oberhollenzer@sigma-star.at>
---
 nand-utils/nanddump.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/nand-utils/nanddump.c b/nand-utils/nanddump.c
index 2f167bb..841ed67 100644
--- a/nand-utils/nanddump.c
+++ b/nand-utils/nanddump.c
@@ -162,6 +162,7 @@ static void process_options(int argc, char * const argv[])
 				start_addr = simple_strtoll(optarg, &error);
 				break;
 			case 'f':
+				free(dumpfile);
 				dumpfile = xstrdup(optarg);
 				break;
 			case 'l':
-- 
2.21.0


______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/

^ permalink raw reply related	[flat|nested] 16+ messages in thread

* [PATCH 13/15] mtd_debug: cleanup error handling in flash_to_file
  2019-11-10 15:30 [PATCH 00/15] mtd-utils: cleanup resource leaks David Oberhollenzer
                   ` (11 preceding siblings ...)
  2019-11-10 15:30 ` [PATCH 12/15] nanddump: don't leak copied command line arguments David Oberhollenzer
@ 2019-11-10 15:30 ` David Oberhollenzer
  2019-11-10 15:30 ` [PATCH 14/15] jittertest: fix error check for open system call David Oberhollenzer
  2019-11-10 15:30 ` [PATCH 15/15] fs-tests: don't leak temporary buffers David Oberhollenzer
  14 siblings, 0 replies; 16+ messages in thread
From: David Oberhollenzer @ 2019-11-10 15:30 UTC (permalink / raw)
  To: linux-mtd; +Cc: richard, David Oberhollenzer

The existing code had multiple error handling labels and did things
like checking if a buffer is not NULL before freeing it.

This patch collapses all of this into a single label. We can do this,
because the standard guarantees us that it is safe to call free() with
a NULL pointer.

This also has the side effect of removing the possibility of using the
wrong error label and accidentally leaking something.

Signed-off-by: David Oberhollenzer <david.oberhollenzer@sigma-star.at>
---
 misc-utils/mtd_debug.c | 20 ++++++++------------
 1 file changed, 8 insertions(+), 12 deletions(-)

diff --git a/misc-utils/mtd_debug.c b/misc-utils/mtd_debug.c
index d65ad36..c0b7109 100644
--- a/misc-utils/mtd_debug.c
+++ b/misc-utils/mtd_debug.c
@@ -112,12 +112,12 @@ static int flash_to_file(int fd, off_t offset, size_t len, const char *filename)
 
 	if (offset != lseek(fd, offset, SEEK_SET)) {
 		perror("lseek()");
-		goto err0;
+		return 1;
 	}
 	outfd = creat(filename, 0666);
 	if (outfd < 0) {
 		perror("creat()");
-		goto err1;
+		return 1;
 	}
 
 retry:
@@ -130,7 +130,7 @@ retry:
 			goto retry;
 		}
 		perror("malloc()");
-		goto err0;
+		goto fail;
 	}
 	do {
 		if (n <= size)
@@ -139,7 +139,7 @@ retry:
 		if (err < 0) {
 			fprintf(stderr, "%s: read, size %#x, n %#x\n", __func__, size, n);
 			perror("read()");
-			goto err2;
+			goto fail;
 		}
 		if (err < size) {
 			fprintf(stderr, "%s: short read, requested %#x, read %#x\n", __func__, size, err);
@@ -148,11 +148,11 @@ retry:
 		if (err < 0) {
 			fprintf(stderr, "%s: write, size %#x, n %#x\n", __func__, size, n);
 			perror("write()");
-			goto err2;
+			goto fail;
 		}
 		if (err != size) {
 			fprintf(stderr, "Couldn't copy entire buffer to %s. (%d/%d bytes copied)\n", filename, err, size);
-			goto err2;
+			goto fail;
 		}
 		n -= size;
 	} while (n > 0);
@@ -162,13 +162,9 @@ retry:
 	close(outfd);
 	printf("Copied %zu bytes from address 0x%.8llx in flash to %s\n", len, (unsigned long long)offset, filename);
 	return 0;
-
-err2:
+fail:
 	close(outfd);
-err1:
-	if (buf != NULL)
-		free(buf);
-err0:
+	free(buf);
 	return 1;
 }
 
-- 
2.21.0


______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/

^ permalink raw reply related	[flat|nested] 16+ messages in thread

* [PATCH 14/15] jittertest: fix error check for open system call
  2019-11-10 15:30 [PATCH 00/15] mtd-utils: cleanup resource leaks David Oberhollenzer
                   ` (12 preceding siblings ...)
  2019-11-10 15:30 ` [PATCH 13/15] mtd_debug: cleanup error handling in flash_to_file David Oberhollenzer
@ 2019-11-10 15:30 ` David Oberhollenzer
  2019-11-10 15:30 ` [PATCH 15/15] fs-tests: don't leak temporary buffers David Oberhollenzer
  14 siblings, 0 replies; 16+ messages in thread
From: David Oberhollenzer @ 2019-11-10 15:30 UTC (permalink / raw)
  To: linux-mtd; +Cc: richard, David Oberhollenzer

The value 0 is a valid file descriptor. The existing error handling
would not only treat that as an error, but subsequently leak the
file descriptor in the error handling path.

Signed-off-by: David Oberhollenzer <david.oberhollenzer@sigma-star.at>
---
 tests/jittertest/JitterTest.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/tests/jittertest/JitterTest.c b/tests/jittertest/JitterTest.c
index e109995..797035b 100644
--- a/tests/jittertest/JitterTest.c
+++ b/tests/jittertest/JitterTest.c
@@ -462,14 +462,14 @@ static void doGrabKProfile(int jitterusec, char *fileName)
 
     (void)jitterusec;
 
-    if((fdSnapshot = open(fileName, O_WRONLY | O_CREAT, S_IRWXU)) <= 0)
+    if((fdSnapshot = open(fileName, O_WRONLY | O_CREAT, S_IRWXU)) < 0)
     {
         fprintf(stderr, "Could not open file %s.\n", fileName);
         perror("Error:");
         return;
     }
 
-    if((fdProfile = open("/proc/profile", O_RDWR)) <= 0)
+    if((fdProfile = open("/proc/profile", O_RDWR)) < 0)
     {
         fprintf(stderr, "Could not open file /proc/profile. Make sure you booted with profile=2\n");
         close(fdSnapshot);
@@ -509,7 +509,7 @@ static void clearProfileBuf(void){
   char readBuf[10];
 
 
-  if((fdProfile = open("/proc/profile", O_RDWR)) <= 0)
+  if((fdProfile = open("/proc/profile", O_RDWR)) < 0)
     {
       fprintf(stderr, "Could not open file /proc/profile. Make sure you booted with profile=2\n");
       return;
-- 
2.21.0


______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/

^ permalink raw reply related	[flat|nested] 16+ messages in thread

* [PATCH 15/15] fs-tests: don't leak temporary buffers
  2019-11-10 15:30 [PATCH 00/15] mtd-utils: cleanup resource leaks David Oberhollenzer
                   ` (13 preceding siblings ...)
  2019-11-10 15:30 ` [PATCH 14/15] jittertest: fix error check for open system call David Oberhollenzer
@ 2019-11-10 15:30 ` David Oberhollenzer
  14 siblings, 0 replies; 16+ messages in thread
From: David Oberhollenzer @ 2019-11-10 15:30 UTC (permalink / raw)
  To: linux-mtd; +Cc: richard, David Oberhollenzer

Signed-off-by: David Oberhollenzer <david.oberhollenzer@sigma-star.at>
---
 tests/fs-tests/simple/perf.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/tests/fs-tests/simple/perf.c b/tests/fs-tests/simple/perf.c
index aee8226..224085f 100644
--- a/tests/fs-tests/simple/perf.c
+++ b/tests/fs-tests/simple/perf.c
@@ -151,6 +151,7 @@ static void perf(void)
 	printf("Write speed (KiB/s): %u\n", speed(actual_size, write_time));
 	printf("Read speed (KiB/s): %u\n", speed(actual_size, read_time));
 	printf("Test completed\n");
+	free(buf);
 }
 
 /* Title of this test */
-- 
2.21.0


______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/

^ permalink raw reply related	[flat|nested] 16+ messages in thread

end of thread, other threads:[~2019-11-10 15:36 UTC | newest]

Thread overview: 16+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-11-10 15:30 [PATCH 00/15] mtd-utils: cleanup resource leaks David Oberhollenzer
2019-11-10 15:30 ` [PATCH 01/15] mkfs.ubifs: close file descriptor in add_file error path David Oberhollenzer
2019-11-10 15:30 ` [PATCH 02/15] mkfs.ubifs: don't leak copied command line arguments David Oberhollenzer
2019-11-10 15:30 ` [PATCH 03/15] mkfs.ubifs: free derived fscrypt context in add_directory error paths David Oberhollenzer
2019-11-10 15:30 ` [PATCH 04/15] mkfs.ubifs: don't leak hastable iterators David Oberhollenzer
2019-11-10 15:30 ` [PATCH 05/15] mkfs.ubifs: don't leak temporary buffers David Oberhollenzer
2019-11-10 15:30 ` [PATCH 06/15] mkfs.ubifs: propperly cleanup in ALL interpret_table_entry error paths David Oberhollenzer
2019-11-10 15:30 ` [PATCH 07/15] mkfs.jffs2: don't leak temporary buffer if readlink fails David Oberhollenzer
2019-11-10 15:30 ` [PATCH 08/15] libmtd: don't leak temporary buffers David Oberhollenzer
2019-11-10 15:30 ` [PATCH 09/15] ftl_check: " David Oberhollenzer
2019-11-10 15:30 ` [PATCH 10/15] ftl_format: " David Oberhollenzer
2019-11-10 15:30 ` [PATCH 11/15] ubiformat: don't leak file descriptors David Oberhollenzer
2019-11-10 15:30 ` [PATCH 12/15] nanddump: don't leak copied command line arguments David Oberhollenzer
2019-11-10 15:30 ` [PATCH 13/15] mtd_debug: cleanup error handling in flash_to_file David Oberhollenzer
2019-11-10 15:30 ` [PATCH 14/15] jittertest: fix error check for open system call David Oberhollenzer
2019-11-10 15:30 ` [PATCH 15/15] fs-tests: don't leak temporary buffers David Oberhollenzer

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).