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: ect...@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); _______________________________________________ svn-src-head@freebsd.org mailing list https://lists.freebsd.org/mailman/listinfo/svn-src-head To unsubscribe, send any mail to "svn-src-head-unsubscr...@freebsd.org"
