svn commit: r304532 - in head/sys/boot: common efi/loader/arch/amd64 fdt

Toomas Soome tsoome at FreeBSD.org
Sat Aug 20 16:23:21 UTC 2016


Author: tsoome
Date: Sat Aug 20 16:23:19 2016
New Revision: 304532
URL: https://svnweb.freebsd.org/changeset/base/304532

Log:
  loader is filling fixed length command_errbuf with sprintf() and is trusting
  strings provided by user/config files. This update is replacing sprintf with
  snprintf for cases the command_errbuf is built from dynamic content.
  
  PR:		211958
  Reported by:	ecturt at gmail.com
  Reviewed by:	imp, allanjude
  Approved by:	imp (mentor), allanjude (mentor)
  Differential Revision:	https://reviews.freebsd.org/D7563

Modified:
  head/sys/boot/common/boot.c
  head/sys/boot/common/bootstrap.h
  head/sys/boot/common/commands.c
  head/sys/boot/common/interp.c
  head/sys/boot/common/ls.c
  head/sys/boot/common/module.c
  head/sys/boot/efi/loader/arch/amd64/framebuffer.c
  head/sys/boot/fdt/fdt_loader_cmd.c

Modified: head/sys/boot/common/boot.c
==============================================================================
--- head/sys/boot/common/boot.c	Sat Aug 20 15:20:01 2016	(r304531)
+++ head/sys/boot/common/boot.c	Sat Aug 20 16:23:19 2016	(r304532)
@@ -61,7 +61,8 @@ command_boot(int argc, char *argv[])
 
 	/* XXX maybe we should discard everything and start again? */
 	if (file_findfile(NULL, NULL) != NULL) {
-	    sprintf(command_errbuf, "can't boot '%s', kernel module already loaded", argv[1]);
+	    snprintf(command_errbuf, sizeof(command_errbuf),
+		"can't boot '%s', kernel module already loaded", argv[1]);
 	    return(CMD_ERROR);
 	}
 
@@ -129,7 +130,8 @@ command_autoboot(int argc, char *argv[])
     case 2:
 	howlong = strtol(argv[1], &cp, 0);
 	if (*cp != 0) {
-	    sprintf(command_errbuf, "bad delay '%s'", argv[1]);
+	    snprintf(command_errbuf, sizeof(command_errbuf),
+		"bad delay '%s'", argv[1]);
 	    return(CMD_ERROR);
 	}
 	/* FALLTHROUGH */

Modified: head/sys/boot/common/bootstrap.h
==============================================================================
--- head/sys/boot/common/bootstrap.h	Sat Aug 20 15:20:01 2016	(r304531)
+++ head/sys/boot/common/bootstrap.h	Sat Aug 20 16:23:19 2016	(r304532)
@@ -35,8 +35,9 @@
 
 /* Commands and return values; nonzero return sets command_errmsg != NULL */
 typedef int	(bootblk_cmd_t)(int argc, char *argv[]);
+#define	COMMAND_ERRBUFSZ	(256)
 extern char	*command_errmsg;	
-extern char	command_errbuf[];	/* XXX blah, length */
+extern char	command_errbuf[COMMAND_ERRBUFSZ];
 #define CMD_OK		0
 #define CMD_WARN	1
 #define CMD_ERROR	2

Modified: head/sys/boot/common/commands.c
==============================================================================
--- head/sys/boot/common/commands.c	Sat Aug 20 15:20:01 2016	(r304531)
+++ head/sys/boot/common/commands.c	Sat Aug 20 16:23:19 2016	(r304532)
@@ -33,7 +33,8 @@ __FBSDID("$FreeBSD$");
 #include "bootstrap.h"
 
 char		*command_errmsg;
-char		command_errbuf[256];	/* XXX should have procedural interface for setting, size limit? */
+/* XXX should have procedural interface for setting, size limit? */
+char		command_errbuf[COMMAND_ERRBUFSZ];
 
 static int page_file(char *filename);
 
@@ -196,7 +197,8 @@ command_help(int argc, char *argv[]) 
     pager_close();
     close(hfd);
     if (!matched) {
-	sprintf(command_errbuf, "no help available for '%s'", topic);
+	snprintf(command_errbuf, sizeof(command_errbuf),
+	    "no help available for '%s'", topic);
 	free(topic);
 	if (subtopic)
 	    free(subtopic);
@@ -276,7 +278,8 @@ command_show(int argc, char *argv[])
 	if ((cp = getenv(argv[1])) != NULL) {
 	    printf("%s\n", cp);
 	} else {
-	    sprintf(command_errbuf, "variable '%s' not found", argv[1]);
+	    snprintf(command_errbuf, sizeof(command_errbuf),
+		"variable '%s' not found", argv[1]);
 	    return(CMD_ERROR);
 	}
     }
@@ -386,7 +389,8 @@ command_read(int argc, char *argv[])
 	case 't':
 	    timeout = strtol(optarg, &cp, 0);
 	    if (cp == optarg) {
-		sprintf(command_errbuf, "bad timeout '%s'", optarg);
+		snprintf(command_errbuf, sizeof(command_errbuf),
+		    "bad timeout '%s'", optarg);
 		return(CMD_ERROR);
 	    }
 	    break;
@@ -454,8 +458,10 @@ page_file(char *filename)
 
     result = pager_file(filename);
 
-    if (result == -1)
-	sprintf(command_errbuf, "error showing %s", filename);
+    if (result == -1) {
+	snprintf(command_errbuf, sizeof(command_errbuf),
+	    "error showing %s", filename);
+    }
 
     return result;
 }   

Modified: head/sys/boot/common/interp.c
==============================================================================
--- head/sys/boot/common/interp.c	Sat Aug 20 15:20:01 2016	(r304531)
+++ head/sys/boot/common/interp.c	Sat Aug 20 16:23:19 2016	(r304532)
@@ -214,7 +214,8 @@ include(const char *filename)
 #endif
 
     if (((fd = open(filename, O_RDONLY)) == -1)) {
-	sprintf(command_errbuf,"can't open '%s': %s", filename, strerror(errno));
+	snprintf(command_errbuf, sizeof(command_errbuf),
+	    "can't open '%s': %s", filename, strerror(errno));
 	return(CMD_ERROR);
     }
 
@@ -256,8 +257,9 @@ include(const char *filename)
 			script = script->next;
 			free(se);
 		}
-		sprintf(command_errbuf, "file '%s' line %d: memory allocation "
-		    "failure - aborting", filename, line);
+		snprintf(command_errbuf, sizeof(command_errbuf),
+		    "file '%s' line %d: memory allocation failure - aborting",
+		    filename, line);
 		return (CMD_ERROR);
 	}
 	strcpy(sp->text, cp);
@@ -291,7 +293,9 @@ include(const char *filename)
 #ifdef BOOT_FORTH
 	res = bf_run(sp->text);
 	if (res != VM_OUTOFTEXT) {
-		sprintf(command_errbuf, "Error while including %s, in the line:\n%s", filename, sp->text);
+		snprintf(command_errbuf, sizeof(command_errbuf),
+		    "Error while including %s, in the line:\n%s",
+		    filename, sp->text);
 		res = CMD_ERROR;
 		break;
 	} else

Modified: head/sys/boot/common/ls.c
==============================================================================
--- head/sys/boot/common/ls.c	Sat Aug 20 15:20:01 2016	(r304531)
+++ head/sys/boot/common/ls.c	Sat Aug 20 16:23:19 2016	(r304532)
@@ -150,7 +150,8 @@ ls_getdir(char **pathp)
 
     /* Make sure the path is respectable to begin with */
     if (archsw.arch_getdev(NULL, path, &cp)) {
-	sprintf(command_errbuf, "bad path '%s'", path);
+	snprintf(command_errbuf, sizeof(command_errbuf),
+	    "bad path '%s'", path);
 	goto out;
     }
     
@@ -160,15 +161,18 @@ ls_getdir(char **pathp)
 
     fd = open(path, O_RDONLY);
     if (fd < 0) {
-	sprintf(command_errbuf, "open '%s' failed: %s", path, strerror(errno));
+	snprintf(command_errbuf, sizeof(command_errbuf),
+	    "open '%s' failed: %s", path, strerror(errno));
 	goto out;
     }
     if (fstat(fd, &sb) < 0) {
-	sprintf(command_errbuf, "stat failed: %s", strerror(errno));
+	snprintf(command_errbuf, sizeof(command_errbuf),
+	    "stat failed: %s", strerror(errno));
 	goto out;
     }
     if (!S_ISDIR(sb.st_mode)) {
-	sprintf(command_errbuf, "%s: %s", path, strerror(ENOTDIR));
+	snprintf(command_errbuf, sizeof(command_errbuf),
+	    "%s: %s", path, strerror(ENOTDIR));
 	goto out;
     }
 

Modified: head/sys/boot/common/module.c
==============================================================================
--- head/sys/boot/common/module.c	Sat Aug 20 15:20:01 2016	(r304531)
+++ head/sys/boot/common/module.c	Sat Aug 20 16:23:19 2016	(r304532)
@@ -143,7 +143,8 @@ command_load(int argc, char *argv[])
 
 	fp = file_findfile(argv[1], typestr);
 	if (fp) {
-		sprintf(command_errbuf, "warning: file '%s' already loaded", argv[1]);
+		snprintf(command_errbuf, sizeof(command_errbuf),
+		    "warning: file '%s' already loaded", argv[1]);
 		return (CMD_WARN);
 	}
 
@@ -162,7 +163,8 @@ command_load(int argc, char *argv[])
     if (dokld || file_havepath(argv[1])) {
 	error = mod_loadkld(argv[1], argc - 2, argv + 2);
 	if (error == EEXIST) {
-	    sprintf(command_errbuf, "warning: KLD '%s' already loaded", argv[1]);
+	    snprintf(command_errbuf, sizeof(command_errbuf),
+		"warning: KLD '%s' already loaded", argv[1]);
 	    return (CMD_WARN);
 	}
 	
@@ -173,7 +175,8 @@ command_load(int argc, char *argv[])
      */
     error = mod_load(argv[1], NULL, argc - 2, argv + 2);
     if (error == EEXIST) {
-	sprintf(command_errbuf, "warning: module '%s' already loaded", argv[1]);
+	snprintf(command_errbuf, sizeof(command_errbuf),
+	    "warning: module '%s' already loaded", argv[1]);
 	return (CMD_WARN);
     }
 
@@ -202,7 +205,8 @@ command_load_geli(int argc, char *argv[]
 	case 'n':
 	    num = strtol(optarg, &cp, 0);
 	    if (cp == optarg) {
-		    sprintf(command_errbuf, "bad key index '%s'", optarg);
+		    snprintf(command_errbuf, sizeof(command_errbuf),
+			"bad key index '%s'", optarg);
 		    return(CMD_ERROR);
 	    }
 	    break;
@@ -334,8 +338,8 @@ file_load(char *filename, vm_offset_t de
 	if (error == EFTYPE)
 	    continue;		/* Unknown to this handler? */
 	if (error) {
-	    sprintf(command_errbuf, "can't load file '%s': %s",
-		filename, strerror(error));
+	    snprintf(command_errbuf, sizeof(command_errbuf),
+		"can't load file '%s': %s", filename, strerror(error));
 	    break;
 	}
     }
@@ -371,8 +375,8 @@ file_load_dependencies(struct preloaded_
 	     */
 	    mp = file_findmodule(NULL, dmodname, verinfo);
 	    if (mp == NULL) {
-		sprintf(command_errbuf, "module '%s' exists but with wrong version",
-		    dmodname);
+		snprintf(command_errbuf, sizeof(command_errbuf),
+		    "module '%s' exists but with wrong version", dmodname);
 		error = ENOENT;
 		break;
 	    }
@@ -411,12 +415,14 @@ file_loadraw(const char *fname, char *ty
     /* locate the file on the load path */
     name = file_search(fname, NULL);
     if (name == NULL) {
-	sprintf(command_errbuf, "can't find '%s'", fname);
+	snprintf(command_errbuf, sizeof(command_errbuf),
+	    "can't find '%s'", fname);
 	return(NULL);
     }
 
     if ((fd = open(name, O_RDONLY)) < 0) {
-	sprintf(command_errbuf, "can't open '%s': %s", name, strerror(errno));
+	snprintf(command_errbuf, sizeof(command_errbuf),
+	    "can't open '%s': %s", name, strerror(errno));
 	free(name);
 	return(NULL);
     }
@@ -433,7 +439,8 @@ file_loadraw(const char *fname, char *ty
 	if (got == 0)				/* end of file */
 	    break;
 	if (got < 0) {				/* error */
-	    sprintf(command_errbuf, "error reading '%s': %s", name, strerror(errno));
+	    snprintf(command_errbuf, sizeof(command_errbuf),
+		"error reading '%s': %s", name, strerror(errno));
 	    free(name);
 	    close(fd);
 	    return(NULL);
@@ -487,13 +494,15 @@ mod_load(char *modname, struct mod_depen
 	    free(mp->m_args);
 	mp->m_args = unargv(argc, argv);
 #endif
-	sprintf(command_errbuf, "warning: module '%s' already loaded", mp->m_name);
+	snprintf(command_errbuf, sizeof(command_errbuf),
+	    "warning: module '%s' already loaded", mp->m_name);
 	return (0);
     }
     /* locate file with the module on the search path */
     filename = mod_searchmodule(modname, verinfo);
     if (filename == NULL) {
-	sprintf(command_errbuf, "can't find '%s'", modname);
+	snprintf(command_errbuf, sizeof(command_errbuf),
+	    "can't find '%s'", modname);
 	return (ENOENT);
     }
     err = mod_loadkld(filename, argc, argv);
@@ -516,7 +525,8 @@ mod_loadkld(const char *kldname, int arg
      */
     filename = file_search(kldname, kld_ext_list);
     if (filename == NULL) {
-	sprintf(command_errbuf, "can't find '%s'", kldname);
+	snprintf(command_errbuf, sizeof(command_errbuf),
+	    "can't find '%s'", kldname);
 	return (ENOENT);
     }
     /*
@@ -524,7 +534,8 @@ mod_loadkld(const char *kldname, int arg
      */
     fp = file_findfile(filename, NULL);
     if (fp) {
-	sprintf(command_errbuf, "warning: KLD '%s' already loaded", filename);
+	snprintf(command_errbuf, sizeof(command_errbuf),
+	    "warning: KLD '%s' already loaded", filename);
 	free(filename);
 	return (0);
     }
@@ -548,8 +559,10 @@ mod_loadkld(const char *kldname, int arg
 	    break;
 	}
     } while(0);
-    if (err == EFTYPE)
-	sprintf(command_errbuf, "don't know how to load module '%s'", filename);
+    if (err == EFTYPE) {
+	snprintf(command_errbuf, sizeof(command_errbuf),
+	    "don't know how to load module '%s'", filename);
+    }
     if (err && fp)
 	file_discard(fp);
     free(filename);

Modified: head/sys/boot/efi/loader/arch/amd64/framebuffer.c
==============================================================================
--- head/sys/boot/efi/loader/arch/amd64/framebuffer.c	Sat Aug 20 15:20:01 2016	(r304531)
+++ head/sys/boot/efi/loader/arch/amd64/framebuffer.c	Sat Aug 20 16:23:19 2016	(r304532)
@@ -474,8 +474,9 @@ command_gop(int argc, char *argv[])
 
 	status = BS->LocateProtocol(&gop_guid, NULL, (VOID **)&gop);
 	if (EFI_ERROR(status)) {
-		sprintf(command_errbuf, "%s: Graphics Output Protocol not "
-		    "present (error=%lu)", argv[0], EFI_ERROR_CODE(status));
+		snprintf(command_errbuf, sizeof(command_errbuf),
+		    "%s: Graphics Output Protocol not present (error=%lu)",
+		    argv[0], EFI_ERROR_CODE(status));
 		return (CMD_ERROR);
 	}
 
@@ -494,9 +495,9 @@ command_gop(int argc, char *argv[])
 		}
 		status = gop->SetMode(gop, mode);
 		if (EFI_ERROR(status)) {
-			sprintf(command_errbuf, "%s: Unable to set mode to "
-			    "%u (error=%lu)", argv[0], mode,
-			    EFI_ERROR_CODE(status));
+			snprintf(command_errbuf, sizeof(command_errbuf),
+			    "%s: Unable to set mode to %u (error=%lu)",
+			    argv[0], mode, EFI_ERROR_CODE(status));
 			return (CMD_ERROR);
 		}
 	} else if (!strcmp(argv[1], "get")) {
@@ -526,8 +527,8 @@ command_gop(int argc, char *argv[])
 	return (CMD_OK);
 
  usage:
-	sprintf(command_errbuf, "usage: %s [list | get | set <mode>]",
-	    argv[0]);
+	snprintf(command_errbuf, sizeof(command_errbuf),
+	    "usage: %s [list | get | set <mode>]", argv[0]);
 	return (CMD_ERROR);
 }
 
@@ -542,8 +543,9 @@ command_uga(int argc, char *argv[])
 
 	status = BS->LocateProtocol(&uga_guid, NULL, (VOID **)&uga);
 	if (EFI_ERROR(status)) {
-		sprintf(command_errbuf, "%s: UGA Protocol not present "
-		    "(error=%lu)", argv[0], EFI_ERROR_CODE(status));
+		snprintf(command_errbuf, sizeof(command_errbuf),
+		    "%s: UGA Protocol not present (error=%lu)",
+		    argv[0], EFI_ERROR_CODE(status));
 		return (CMD_ERROR);
 	}
 
@@ -551,8 +553,8 @@ command_uga(int argc, char *argv[])
 		goto usage;
 
 	if (efifb_from_uga(&efifb, uga) != CMD_OK) {
-		sprintf(command_errbuf, "%s: Unable to get UGA information",
-		    argv[0]);
+		snprintf(command_errbuf, sizeof(command_errbuf),
+		    "%s: Unable to get UGA information", argv[0]);
 		return (CMD_ERROR);
 	}
 
@@ -561,6 +563,6 @@ command_uga(int argc, char *argv[])
 	return (CMD_OK);
 
  usage:
-	sprintf(command_errbuf, "usage: %s", argv[0]);
+	snprintf(command_errbuf, sizeof(command_errbuf), "usage: %s", argv[0]);
 	return (CMD_ERROR);
 }

Modified: head/sys/boot/fdt/fdt_loader_cmd.c
==============================================================================
--- head/sys/boot/fdt/fdt_loader_cmd.c	Sat Aug 20 15:20:01 2016	(r304531)
+++ head/sys/boot/fdt/fdt_loader_cmd.c	Sat Aug 20 16:23:19 2016	(r304532)
@@ -194,14 +194,14 @@ fdt_load_dtb(vm_offset_t va)
 	COPYOUT(va, &header, sizeof(header));
 	err = fdt_check_header(&header);
 	if (err < 0) {
-		if (err == -FDT_ERR_BADVERSION)
-			sprintf(command_errbuf,
+		if (err == -FDT_ERR_BADVERSION) {
+			snprintf(command_errbuf, sizeof(command_errbuf),
 			    "incompatible blob version: %d, should be: %d",
 			    fdt_version(fdtp), FDT_LAST_SUPPORTED_VERSION);
-
-		else
-			sprintf(command_errbuf, "error validating blob: %s",
-			    fdt_strerror(err));
+		} else {
+			snprintf(command_errbuf, sizeof(command_errbuf),
+			    "error validating blob: %s", fdt_strerror(err));
+		}
 		return (1);
 	}
 
@@ -236,8 +236,8 @@ fdt_load_dtb_addr(struct fdt_header *hea
 	fdtp_size = fdt_totalsize(header);
 	err = fdt_check_header(header);
 	if (err < 0) {
-		sprintf(command_errbuf, "error validating blob: %s",
-		    fdt_strerror(err));
+		snprintf(command_errbuf, sizeof(command_errbuf),
+		    "error validating blob: %s", fdt_strerror(err));
 		return (err);
 	}
 	free(fdtp);
@@ -263,7 +263,8 @@ fdt_load_dtb_file(const char * filename)
 
 	/* Attempt to load and validate a new dtb from a file. */
 	if ((bfp = file_loadraw(filename, "dtb", 1)) == NULL) {
-		sprintf(command_errbuf, "failed to load file '%s'", filename);
+		snprintf(command_errbuf, sizeof(command_errbuf),
+		    "failed to load file '%s'", filename);
 		return (1);
 	}
 	if ((err = fdt_load_dtb(bfp->f_addr)) != 0) {
@@ -609,7 +610,8 @@ fdt_fixup_memory(struct fdt_mem_region *
 		/* Create proper '/memory' node. */
 		memory = fdt_add_subnode(fdtp, root, "memory");
 		if (memory <= 0) {
-			sprintf(command_errbuf, "Could not fixup '/memory' "
+			snprintf(command_errbuf, sizeof(command_errbuf),
+			    "Could not fixup '/memory' "
 			    "node, error code : %d!\n", memory);
 			return;
 		}
@@ -626,7 +628,8 @@ fdt_fixup_memory(struct fdt_mem_region *
 	size_cellsp = (uint32_t *)fdt_getprop(fdtp, root, "#size-cells", NULL);
 
 	if (addr_cellsp == NULL || size_cellsp == NULL) {
-		sprintf(command_errbuf, "Could not fixup '/memory' node : "
+		snprintf(command_errbuf, sizeof(command_errbuf),
+		    "Could not fixup '/memory' node : "
 		    "%s %s property not found in root node!\n",
 		    (!addr_cellsp) ? "#address-cells" : "",
 		    (!size_cellsp) ? "#size-cells" : "");
@@ -906,7 +909,8 @@ fdt_cmd_addr(int argc, char *argv[])
 
 	hdr = (struct fdt_header *)strtoul(addr, &cp, 16);
 	if (cp == addr) {
-		sprintf(command_errbuf, "Invalid address: %s", addr);
+		snprintf(command_errbuf, sizeof(command_errbuf),
+		    "Invalid address: %s", addr);
 		return (CMD_ERROR);
 	}
 
@@ -945,7 +949,8 @@ fdt_cmd_cd(int argc, char *argv[])
 
 	o = fdt_path_offset(fdtp, path);
 	if (o < 0) {
-		sprintf(command_errbuf, "could not find node: '%s'", path);
+		snprintf(command_errbuf, sizeof(command_errbuf),
+		    "could not find node: '%s'", path);
 		return (CMD_ERROR);
 	}
 
@@ -953,8 +958,8 @@ fdt_cmd_cd(int argc, char *argv[])
 	return (CMD_OK);
 
 fail:
-	sprintf(command_errbuf, "path too long: %d, max allowed: %d",
-	    len, FDT_CWD_LEN - 1);
+	snprintf(command_errbuf, sizeof(command_errbuf),
+	    "path too long: %d, max allowed: %d", len, FDT_CWD_LEN - 1);
 	return (CMD_ERROR);
 }
 
@@ -1037,7 +1042,8 @@ fdt_cmd_ls(int argc, char *argv[])
 
 	o = fdt_path_offset(fdtp, path);
 	if (o < 0) {
-		sprintf(command_errbuf, "could not find node: '%s'", path);
+		snprintf(command_errbuf, sizeof(command_errbuf),
+		    "could not find node: '%s'", path);
 		return (CMD_ERROR);
 	}
 
@@ -1483,7 +1489,8 @@ fdt_extract_nameloc(char **pathp, char *
 		return (1);
 	}
 	if (o < 0) {
-		sprintf(command_errbuf, "could not find node: '%s'", path);
+		snprintf(command_errbuf, sizeof(command_errbuf),
+		    "could not find node: '%s'", path);
 		return (1);
 	}
 	*namep = name;
@@ -1530,7 +1537,8 @@ fdt_cmd_prop(int argc, char *argv[])
 	o = fdt_path_offset(fdtp, path);
 
 	if (o < 0) {
-		sprintf(command_errbuf, "could not find node: '%s'", path);
+		snprintf(command_errbuf, sizeof(command_errbuf),
+		    "could not find node: '%s'", path);
 		rv = CMD_ERROR;
 		goto out;
 	}
@@ -1623,8 +1631,9 @@ fdt_cmd_rm(int argc, char *argv[])
 			return (CMD_ERROR);
 
 		if ((rv = fdt_delprop(fdtp, o, propname)) != 0) {
-			sprintf(command_errbuf, "could not delete"
-			    "%s\n", (rv == -FDT_ERR_NOTFOUND) ?
+			snprintf(command_errbuf, sizeof(command_errbuf),
+			    "could not delete %s\n",
+			    (rv == -FDT_ERR_NOTFOUND) ?
 			    "(property/node does not exist)" : "");
 			return (CMD_ERROR);
 


More information about the svn-src-head mailing list