diff mbox

[U-Boot] mtdparts: Fix various issues reported by Coverity

Message ID 1502757747-17303-1-git-send-email-trini@konsulko.com
State Accepted
Delegated to: Tom Rini
Headers show

Commit Message

Tom Rini Aug. 15, 2017, 12:42 a.m. UTC
Now that sandbox is building cmd/mtdparts.c Coverity has looked at the
code and found a number of issues.  In index_partitions() it is possible
that part will be NULL, so re-work the checks and debug statements to
take this into account.  We have a number of string buffers that we
print to in the exact size of, and use string functions on, so we need
to ensure they are large enough to be NULL terminated.  In
device_parse() it is not possible for num_partitions to be 0 (we would
have hit a different error first) so remove logically dead code.
Finally, in parse_mtdparts() if we have an error we need to free the
memory allocated to dev.

Cc: Lothar Waßmann <LW@KARO-electronics.de>
Cc: Maxime Ripard <maxime.ripard@free-electrons.com>
Reported-by: Coverity (CID: 166334, 166333, 166332, 166329, 166328)
Signed-off-by: Tom Rini <trini@konsulko.com>
---
 cmd/mtdparts.c | 33 ++++++++++++++++++---------------
 1 file changed, 18 insertions(+), 15 deletions(-)

Comments

Tom Rini Aug. 20, 2017, 11:29 p.m. UTC | #1
On Mon, Aug 14, 2017 at 08:42:27PM -0400, Tom Rini wrote:

> Now that sandbox is building cmd/mtdparts.c Coverity has looked at the
> code and found a number of issues.  In index_partitions() it is possible
> that part will be NULL, so re-work the checks and debug statements to
> take this into account.  We have a number of string buffers that we
> print to in the exact size of, and use string functions on, so we need
> to ensure they are large enough to be NULL terminated.  In
> device_parse() it is not possible for num_partitions to be 0 (we would
> have hit a different error first) so remove logically dead code.
> Finally, in parse_mtdparts() if we have an error we need to free the
> memory allocated to dev.
> 
> Cc: Lothar Waßmann <LW@KARO-electronics.de>
> Cc: Maxime Ripard <maxime.ripard@free-electrons.com>
> Reported-by: Coverity (CID: 166334, 166333, 166332, 166329, 166328)
> Signed-off-by: Tom Rini <trini@konsulko.com>

Applied to u-boot/master, thanks!
diff mbox

Patch

diff --git a/cmd/mtdparts.c b/cmd/mtdparts.c
index 683c48bdad1b..93fd2f3767b8 100644
--- a/cmd/mtdparts.c
+++ b/cmd/mtdparts.c
@@ -133,9 +133,9 @@  static const char *mtdparts_default = MTDPARTS_DEFAULT;
 #define MTDIDS_MAXLEN		128
 #define MTDPARTS_MAXLEN		512
 #define PARTITION_MAXLEN	16
-static char last_ids[MTDIDS_MAXLEN];
-static char last_parts[MTDPARTS_MAXLEN];
-static char last_partition[PARTITION_MAXLEN];
+static char last_ids[MTDIDS_MAXLEN + 1];
+static char last_parts[MTDPARTS_MAXLEN + 1];
+static char last_partition[PARTITION_MAXLEN + 1];
 
 /* low level jffs2 cache cleaning routine */
 extern void jffs2_free_cache(struct part_info *part);
@@ -240,15 +240,22 @@  static void index_partitions(void)
 			if (dev == current_mtd_dev) {
 				mtddevnum += current_mtd_partnum;
 				setenv_ulong("mtddevnum", mtddevnum);
+				debug("=> mtddevnum %d,\n", mtddevnum);
 				break;
 			}
 			mtddevnum += dev->num_parts;
 		}
 
 		part = mtd_part_info(current_mtd_dev, current_mtd_partnum);
-		setenv("mtddevname", part->name);
+		if (part) {
+			setenv("mtddevname", part->name);
+
+			debug("=> mtddevname %s\n", part->name);
+		} else {
+			setenv("mtddevname", NULL);
 
-		debug("=> mtddevnum %d,\n=> mtddevname %s\n", mtddevnum, part->name);
+			debug("=> mtddevname NULL\n");
+		}
 	} else {
 		setenv("mtddevnum", NULL);
 		setenv("mtddevname", NULL);
@@ -912,12 +919,6 @@  static int device_parse(const char *const mtd_dev, const char **ret, struct mtd_
 		return 1;
 	}
 
-	if (num_parts == 0) {
-		printf("no partitions for device %s%d (%s)\n",
-				MTD_DEV_TYPE(id->type), id->num, id->mtd_id);
-		return 1;
-	}
-
 	debug("\ntotal partitions: %d\n", num_parts);
 
 	/* check for next device presence */
@@ -1593,8 +1594,10 @@  static int parse_mtdparts(const char *const mtdparts)
 		list_add_tail(&dev->link, &devices);
 		err = 0;
 	}
-	if (err == 1)
+	if (err == 1) {
+		free(dev);
 		device_delall(&devices);
+	}
 
 	return err;
 }
@@ -1730,9 +1733,9 @@  int mtdparts_init(void)
 	if (!initialized) {
 		INIT_LIST_HEAD(&mtdids);
 		INIT_LIST_HEAD(&devices);
-		memset(last_ids, 0, MTDIDS_MAXLEN);
-		memset(last_parts, 0, MTDPARTS_MAXLEN);
-		memset(last_partition, 0, PARTITION_MAXLEN);
+		memset(last_ids, 0, sizeof(last_ids));
+		memset(last_parts, 0, sizeof(last_parts));
+		memset(last_partition, 0, sizeof(last_partition));
 #if defined(CONFIG_SYS_MTDPARTS_RUNTIME)
 		board_mtdparts_default(&mtdids_default, &mtdparts_default);
 #endif