changeset 18835:273beebb7fa2

loader: Don't leak memory when displaying help. loader: commands.c should only use snprintf loader: commands.c use __unused illumos issues #9476 9480, 9481
author Toomas Soome <tsoome@me.com>
date Sat, 07 Apr 2018 10:06:30 +0300
parents 55534ee72366
children 1c6c78906a12
files usr/src/boot/sys/boot/common/commands.c usr/src/common/ficl/emu/loader_emu.c
diffstat 2 files changed, 49 insertions(+), 43 deletions(-) [+]
line wrap: on
line diff
--- a/usr/src/boot/sys/boot/common/commands.c	Tue Mar 13 12:43:00 2018 +0200
+++ b/usr/src/boot/sys/boot/common/commands.c	Sat Apr 07 10:06:30 2018 +0300
@@ -1,4 +1,4 @@
-/*-
+/*
  * Copyright (c) 1998 Michael Smith <msmith@freebsd.org>
  * All rights reserved.
  *
@@ -25,7 +25,6 @@
  */
 
 #include <sys/cdefs.h>
-__FBSDID("$FreeBSD$");
 
 #include <stand.h>
 #include <string.h>
@@ -64,7 +63,9 @@
 help_getnext(int fd, char **topic, char **subtopic, char **desc) 
 {
     char	line[81], *cp, *ep;
-    
+
+    /* Make sure we provide sane values. */
+    *topic = *subtopic = *desc = NULL;
     for (;;) {
 	if (fgetstr(line, 80, fd) < 0)
 	    return(0);
@@ -72,9 +73,8 @@
 	if ((strlen(line) < 3) || (line[0] != '#') || (line[1] != ' '))
 	    continue;
 
-	*topic = *subtopic = *desc = NULL;
 	cp = line + 2;
-	while((cp != NULL) && (*cp != 0)) {
+	while ((cp != NULL) && (*cp != 0)) {
 	    ep = strchr(cp, ' ');
 	    if ((*cp == 'T') && (*topic == NULL)) {
 		if (ep != NULL)
@@ -91,11 +91,10 @@
 	    cp = ep;
 	}
 	if (*topic == NULL) {
-	    if (*subtopic != NULL)
 		free(*subtopic);
-	    if (*desc != NULL)
 		free(*desc);
-	    continue;
+		*subtopic = *desc = NULL;
+		continue;
 	}
 	return(1);
     }
@@ -132,7 +131,7 @@
     char	*topic, *subtopic, *t, *s, *d;
 
     /* page the help text from our load path */
-    sprintf(buf, "%s/boot/loader.help", getenv("loaddev"));
+    snprintf(buf, sizeof (buf), "%s/boot/loader.help", getenv("loaddev"));
     if ((hfd = open(buf, O_RDONLY)) < 0) {
 	printf("Verbose help not available, use '?' to list commands\n");
 	return(CMD_OK);
@@ -161,7 +160,7 @@
     
     /* Scan the helpfile looking for help matching the request */
     pager_open();
-    while(help_getnext(hfd, &t, &s, &d)) {
+    while (help_getnext(hfd, &t, &s, &d)) {
 
 	if (doindex) {		/* dink around formatting */
 	    if (help_emitsummary(t, s, d))
@@ -169,7 +168,7 @@
 
 	} else if (strcmp(topic, t)) {
 	    /* topic mismatch */
-	    if(matched)		/* nothing more on this topic, stop scanning */
+	    if (matched)	/* nothing more on this topic, stop scanning */
 		break;
 
 	} else {
@@ -178,7 +177,7 @@
 	    if (((subtopic == NULL) && (s == NULL)) ||
 		((subtopic != NULL) && (s != NULL) && !strcmp(subtopic, s))) {
 		/* exact match, print text */
-		while((fgetstr(buf, 80, hfd) >= 0) && (buf[0] != '#')) {
+		while ((fgetstr(buf, 80, hfd) >= 0) && (buf[0] != '#')) {
 		    if (pager_output(buf))
 			break;
 		    if (pager_output("\n"))
@@ -193,29 +192,29 @@
 	free(t);
 	free(s);
 	free(d);
+	t = s = d = NULL;
     }
+    free(t);
+    free(s);
+    free(d);
     pager_close();
     close(hfd);
     if (!matched) {
 	snprintf(command_errbuf, sizeof (command_errbuf),
 	    "no help available for '%s'", topic);
 	free(topic);
-	if (subtopic)
-	    free(subtopic);
+	free(subtopic);
 	return(CMD_ERROR);
     }
     free(topic);
-    if (subtopic)
-	free(subtopic);
+    free(subtopic);
     return(CMD_OK);
 }
 
-
 COMMAND_SET(commandlist, "?", "list commands", command_commandlist);
 
 static int
-command_commandlist(int argc __attribute((unused)),
-    char *argv[] __attribute((unused)))
+command_commandlist(int argc __unused, char *argv[] __unused)
 {
     struct bootblk_command	**cmdp;
     int		res;
@@ -228,7 +227,7 @@
 	if (res)
 	    break;
 	if (((*cmdp)->c_name != NULL) && ((*cmdp)->c_desc != NULL)) {
-	    sprintf(name, "  %-15s  ", (*cmdp)->c_name);
+	    snprintf(name, sizeof (name), "  %-15s  ", (*cmdp)->c_name);
 	    pager_output(name);
 	    pager_output((*cmdp)->c_desc);
 	    res = pager_output("\n");
@@ -446,12 +445,12 @@
     res=0;
     pager_open();
     for (i = 1; (i < argc) && (res == 0); i++) {
-	sprintf(line, "*** FILE %s BEGIN ***\n", argv[i]);
+	snprintf(line, sizeof (line), "*** FILE %s BEGIN ***\n", argv[i]);
 	if (pager_output(line))
 		break;
         res = page_file(argv[i]);
 	if (!res) {
-	    sprintf(line, "*** FILE %s END ***\n", argv[i]);
+	    snprintf(line, sizeof (line), "*** FILE %s END ***\n", argv[i]);
 	    res = pager_output(line);
 	}
     }
@@ -512,7 +511,7 @@
 	    if (devsw[i]->dv_print(verbose))
 		break;
 	} else {
-	    sprintf(line, "%s: (unknown)\n", devsw[i]->dv_name);
+	    snprintf(line, sizeof (line), "%s: (unknown)\n", devsw[i]->dv_name);
 	    if (pager_output(line))
 		break;
 	}
--- a/usr/src/common/ficl/emu/loader_emu.c	Tue Mar 13 12:43:00 2018 +0200
+++ b/usr/src/common/ficl/emu/loader_emu.c	Sat Apr 07 10:06:30 2018 +0300
@@ -799,7 +799,8 @@
 			printf("error interpreting forth: %d\n", rv);
 			exit(1);
 		}
-		sprintf(create_buf, "builtin: %s", cmdp->c_name);
+		snprintf(create_buf, sizeof (create_buf), "builtin: %s",
+		    cmdp->c_name);
 		rv = ficlVmEvaluate(bf_vm, create_buf);
 		if (rv != FICL_VM_STATUS_OUT_OF_TEXT) {
 			printf("error interpreting forth: %d\n", rv);
@@ -1068,6 +1069,7 @@
 {
 	char line[81], *cp, *ep;
 
+	*topic = *subtopic = *desc = NULL;
 	for (;;) {
 		if (fgetstr(line, 80, fd) < 0)
 			return (0);
@@ -1094,10 +1096,8 @@
 			cp = ep;
 		}
 		if (*topic == NULL) {
-			if (*subtopic != NULL)
-				free(*subtopic);
-			if (*desc != NULL)
-				free(*desc);
+			free(*subtopic);
+			free(*desc);
 			continue;
 		}
 		return (1);
@@ -1134,7 +1134,7 @@
 	char *topic, *subtopic, *t, *s, *d;
 
 	/* page the help text from our load path */
-	sprintf(buf, "/boot/loader.help");
+	snprintf(buf, sizeof (buf), "/boot/loader.help");
 	if ((hfd = open(buf, O_RDONLY)) < 0) {
 		printf("Verbose help not available, "
 		    "use '?' to list commands\n");
@@ -1198,25 +1198,27 @@
 		free(t);
 		free(s);
 		free(d);
+		t = s = d = NULL;
 	}
+	free(t);
+	free(s);
+	free(d);
 	pager_close();
 	close(hfd);
 	if (!matched) {
 		snprintf(command_errbuf, sizeof (command_errbuf),
 		    "no help available for '%s'", topic);
 		free(topic);
-		if (subtopic)
-			free(subtopic);
+		free(subtopic);
 		return (CMD_ERROR);
 	}
 	free(topic);
-	if (subtopic)
-		free(subtopic);
+	free(subtopic);
 	return (CMD_OK);
 }
 
 static int
-command_commandlist(int argc, char *argv[])
+command_commandlist(int argc __unused, char *argv[] __unused)
 {
 	struct bootblk_command *cmdp;
 	int res;
@@ -1230,7 +1232,8 @@
 		if (res)
 			break;
 		if ((cmdp->c_name != NULL) && (cmdp->c_desc != NULL)) {
-			sprintf(name, "  %-15s  ", cmdp->c_name);
+			snprintf(name, sizeof (name), "  %-15s  ",
+			    cmdp->c_name);
 			pager_output(name);
 			pager_output(cmdp->c_desc);
 			res = pager_output("\n");
@@ -1451,14 +1454,16 @@
 	res = 0;
 	pager_open();
 	for (i = 1; (i < argc) && (res == 0); i++) {
-		sprintf(line, "*** FILE %s BEGIN ***\n", argv[i]);
+		snprintf(line, sizeof (line), "*** FILE %s BEGIN ***\n",
+		    argv[i]);
 		if (pager_output(line))
 			break;
 		name = get_dev(argv[i]);
 		res = page_file(name);
 		free(name);
 		if (!res) {
-			sprintf(line, "*** FILE %s END ***\n", argv[i]);
+			snprintf(line, sizeof (line), "*** FILE %s END ***\n",
+			    argv[i]);
 			res = pager_output(line);
 		}
 	}
@@ -1537,20 +1542,22 @@
 			sb.st_size = 0;
 			sb.st_mode = 0;
 			buf = malloc(strlen(path) + strlen(d->d_name) + 2);
-			if (path[0] == '\0')
-				sprintf(buf, "%s", d->d_name);
-			else
-				sprintf(buf, "%s/%s", path, d->d_name);
+			if (path[0] == '\0') {
+				snprintf(buf, sizeof (buf), "%s", d->d_name);
+			} else {
+				snprintf(buf, sizeof (buf), "%s/%s", path,
+				    d->d_name);
+			}
 			/* ignore return, could be symlink, etc. */
 			if (stat(buf, &sb))
 				sb.st_size = 0;
 			free(buf);
 			if (verbose) {
-				sprintf(lbuf, " %c %8d %s\n",
+				snprintf(lbuf, sizeof (lbuf), " %c %8d %s\n",
 				    typestr[sb.st_mode >> 12],
 				    (int)sb.st_size, d->d_name);
 			} else {
-				sprintf(lbuf, " %c  %s\n",
+				snprintf(lbuf, sizeof (lbuf), " %c  %s\n",
 				    typestr[sb.st_mode >> 12], d->d_name);
 			}
 			if (pager_output(lbuf))