From: chaoskagami Date: Sun, 28 Aug 2016 03:38:53 +0000 (-0400) Subject: No more writable strings. It's a serious problem for memory debugging, better to... X-Git-Tag: v0.3.0~37 X-Git-Url: https://chaos.moe/g/?a=commitdiff_plain;h=502938350070fa9d160acc6eee1b18a9ad855f8f;p=corbenik%2Fcorbenik.git No more writable strings. It's a serious problem for memory debugging, better to have GCC complain Turns out there's also some kind of memory corruption happening here and in master (thus the pad[300]) --- diff --git a/common.mk b/common.mk index 51ab1d1..912ed05 100644 --- a/common.mk +++ b/common.mk @@ -9,7 +9,7 @@ REVISION := $(shell git rev-parse HEAD | head -c10)+$(shell git rev-list --count AM_CFLAGS= -std=gnu11 -Os -g -ffast-math \ -Wpedantic -Wall -Wextra -Wcast-align -Wcast-qual \ -Wdisabled-optimization -Wformat=2 -Winit-self -Wlogical-op \ - -Wmissing-include-dirs -Wredundant-decls \ + -Wmissing-include-dirs -Wredundant-decls -Wunreachable-code -Wmissing-noreturn -Wwrite-strings \ -Wshadow -Wsign-conversion -Wstrict-overflow=5 -Wswitch-default \ -Wundef -Wno-unused -Werror -Wno-error=cast-align -Wno-error=strict-overflow -Wno-error=pedantic \ $(THUMBFLAGS) $(SIZE_OPTIMIZATION) $(INCPATHS) $(C9FLAGS) \ diff --git a/include/interp.h b/include/interp.h index eb6ebf3..88ade2d 100644 --- a/include/interp.h +++ b/include/interp.h @@ -8,7 +8,7 @@ * \return Zero on success. */ -int execb(char *filename, int build_cache); +int execb(const char *filename, int build_cache); /* Low level function to actually execute the bytecode. Do not call directly. * diff --git a/include/menu-backend.h b/include/menu-backend.h index 7375c83..05f0544 100644 --- a/include/menu-backend.h +++ b/include/menu-backend.h @@ -17,14 +17,15 @@ typedef char* (*get_value_t)(void* data); struct options_s { - char *name; ///< Name of menu option - char *desc; ///< Description of option, shown when pressing select + const char *name; ///< Name of menu option + const char *desc; ///< Description of option, shown when pressing select enum type handle; ///< Type of menu entry. See enum type. void *param; ///< Parameter to pass to func_call_t func_call_t func; ///< Function to call on selection of option get_value_t value; ///< Function to get the value of the menu entry uint8_t indent; ///< Indentation/ownership level of menu. -} __attribute__((packed)); + char pad[300]; +}; /* Set the accent foreground color for a screen. * diff --git a/include/std/allocator.h b/include/std/allocator.h index 605c7b1..ae96dc6 100644 --- a/include/std/allocator.h +++ b/include/std/allocator.h @@ -23,14 +23,14 @@ void print_alloc_stats(); * \param size Size in bytes to allocate. * \param info Info to store about malloc */ -void* malloc_chkd(size_t size, char* info); +void* malloc_chkd(size_t size, const char* info); /* Free in-use memory allocated by malloc (debugging only, don't call) * * \param ptr Pointer to free. * \param info Info to store about free */ -void free_chkd(void* ptr, char* info); +void free_chkd(void* ptr, const char* info); #define STRINGIFY(x) #x #define TOSTRING(x) STRINGIFY(x) diff --git a/include/std/draw.h b/include/std/draw.h index 0f24e5e..7ac1c7a 100644 --- a/include/std/draw.h +++ b/include/std/draw.h @@ -72,13 +72,13 @@ void clear_bg(); * * \param fname_top filename to load from. */ -void load_bg_top(char* fname_top); +void load_bg_top(const char* fname_top); /* Loads bottom background image from a path. * * \param fname_bottom filename to load from. */ -void load_bg_bottom(char* fname_bottom); +void load_bg_bottom(const char* fname_bottom); /* Clears the displays either to black or the background image. */ @@ -159,11 +159,11 @@ void clear_disp(uint8_t *screen); * \param Format string. * \param ... Format arguments */ -void fprintf(void *channel, char *format, ...) __attribute__ ((format (printf, 2, 3))); +void fprintf(void *channel, const char *format, ...) __attribute__ ((format (printf, 2, 3))); /* See fprintf. Takes a va_list instead of variable arguments. */ -void vfprintf(void *channel, char *format, va_list ap); +void vfprintf(void *channel, const char *format, va_list ap); #define BLACK 0 #define BLUE 1 diff --git a/include/std/fs.h b/include/std/fs.h index e1b5de4..d2a933a 100644 --- a/include/std/fs.h +++ b/include/std/fs.h @@ -22,9 +22,9 @@ int fmount(void); int fumount(void); -void recurse_call(char *name, void (*call_fun)(char*)); +void recurse_call(const char *name, void (*call_fun)(char*)); -int rrmdir(char *dir_path); +int rrmdir(const char *dir_path); FILE *fopen(const char *filename, const char *mode); @@ -42,8 +42,8 @@ size_t fwrite(const void *buffer, size_t elementSize, size_t elementCnt, FILE *f size_t fread(void *buffer, size_t elementSize, size_t elementCnt, FILE *fp); -size_t write_file(void *data, char *path, size_t size); +size_t write_file(void *data, const char *path, size_t size); -size_t read_file(void *data, char *path, size_t size); +size_t read_file(void *data, const char *path, size_t size); #endif diff --git a/include/std/memory.h b/include/std/memory.h index b228e57..0a14425 100644 --- a/include/std/memory.h +++ b/include/std/memory.h @@ -17,6 +17,6 @@ uint8_t *memfind(uint8_t *startPos, uint32_t size, const void *pattern, uint32_t * \param str String to duplicate * \return Duplicated string. */ -char* strdup_self(char* str); +char* strdup_self(const char* str); #endif diff --git a/source/config-file.c b/source/config-file.c index cb0bc9c..de394d3 100644 --- a/source/config-file.c +++ b/source/config-file.c @@ -8,7 +8,7 @@ char *config_file_path = NULL; int changed_consoles = 0; uint32_t cid[4]; -void list_patches_build(char *name, int desc_is_fname); +void list_patches_build(const char *name, int desc_is_fname); void regenerate_config() diff --git a/source/firm/firm.c b/source/firm/firm.c index df5f046..74c6a06 100644 --- a/source/firm/firm.c +++ b/source/firm/firm.c @@ -295,7 +295,7 @@ decrypt_arm9bin(arm9bin_h *header, uint64_t firm_title, uint8_t version) } int -decrypt_firm(firm_h *dest, char *path_firmkey, char *path_cetk, uint32_t *size) +decrypt_firm(firm_h *dest, const char *path_firmkey, const char *path_cetk, uint32_t *size) { uint8_t firm_key[AES_BLOCK_SIZE]; @@ -326,7 +326,7 @@ decrypt_firm(firm_h *dest, char *path_firmkey, char *path_cetk, uint32_t *size) extern int patch_services(); int -load_firm(firm_h *dest, char *path, char *path_firmkey, char *path_cetk, uint32_t *size, uint64_t firm_title) +load_firm(firm_h *dest, const char *path, const char *path_firmkey, const char *path_cetk, uint32_t *size, uint64_t firm_title) { int status = 0; int firmware_changed = 0; @@ -442,11 +442,14 @@ boot_firm() wait(); #endif + uint32_t entry11 = firm_loc->a11Entry; + uint32_t entry9 = firm_loc->a9Entry; + // Beyond this point, using malloc() memory is unsafe, since we're trashing memory possibly. // free() is also irrelevant from here on. for (firm_section_h *section = firm_loc->section; section < firm_loc->section + 4 && section->address != 0; section++) { - memcpy((void *)section->address, (void *)((uint8_t*)firm_loc + section->offset), section->size); + memmove((void *)section->address, (void *)((uint8_t*)firm_loc + section->offset), section->size); } fprintf(stderr, "Copied FIRM.\n"); @@ -463,9 +466,9 @@ boot_firm() deinitScreens(); - *a11_entry = (uint32_t)firm_loc->a11Entry; + *a11_entry = (uint32_t)entry11; - ((void (*)())firm_loc->a9Entry)(); + ((void (*)())entry9)(); while(1); } diff --git a/source/interpreter.c b/source/interpreter.c index 58c55d8..35e3e3f 100644 --- a/source/interpreter.c +++ b/source/interpreter.c @@ -592,7 +592,7 @@ execb(uint64_t tid, uint16_t ver, uint8_t *text_mem, uint32_t text_len, uint8_t { #else int -execb(char *filename, int build_cache) +execb(const char *filename, int build_cache) { uint16_t ver = 0; // FIXME - Provide native_firm version #endif diff --git a/source/menu-backend.c b/source/menu-backend.c index 35c74a6..2e02cd5 100644 --- a/source/menu-backend.c +++ b/source/menu-backend.c @@ -5,7 +5,7 @@ extern unsigned int font_w; extern unsigned int font_h; void -header(char *append) +header(const char *append) { set_cursor(TOP_SCREEN, 0, 0); fill_line(stdout, 0, config->options[OPTION_ACCENT_COLOR]); @@ -13,7 +13,7 @@ header(char *append) fprintf(stdout, "\x1b[30m ." FW_NAME " // %s\x1b[0m\n\n", append); } -void show_help(char* help) { +void show_help(const char* help) { clear_disp(TOP_SCREEN); set_cursor(TOP_SCREEN, 0, 0); header("Any:Back"); diff --git a/source/menu.c b/source/menu.c index 65d2e16..2198e82 100644 --- a/source/menu.c +++ b/source/menu.c @@ -44,97 +44,102 @@ char* get_opt(void* val) { case OPTION_BRIGHTNESS: case OPTION_ACCENT_COLOR: str[0] += raw; - return str; + break; default: - return (raw) ? "*" : " "; + if (raw) + str[0] = '*'; + else + str[0] = ' '; + break; } + return str; } static struct options_s options[] = { // Patches. - { "General Options", "", unselectable, 0, NULL, NULL, 0 }, + { "General Options", "", unselectable, 0, NULL, NULL, 0, "" }, { "System Module Inject", "Replaces system modules in FIRM like loader, fs, pxi, etc.", - option, (void*)OPTION_LOADER, change_opt, get_opt, 0 }, + option, (void*)OPTION_LOADER, change_opt, get_opt, 0, "" }, { "svcBackdoor Fixup", "Reinserts svcBackdoor on 11.0 NATIVE_FIRM. svcBackdoor allows executing arbitrary functions with ARM11 kernel permissions, and is required by some (poorly coded) applications.", - option, (void*)OPTION_SVCS, change_opt, get_opt, 0 }, + option, (void*)OPTION_SVCS, change_opt, get_opt, 0, "" }, { "Firmlaunch Hook", "Hooks firmlaunch to allow largemem games on o3DS. Also allows patching TWL/AGB on all consoles. Previously called 'Reboot hook' but renamed for accuracy.", - option, (void*)OPTION_REBOOT, change_opt, get_opt, 0 }, + option, (void*)OPTION_REBOOT, change_opt, get_opt, 0, "" }, { "Use EmuNAND", "Redirects NAND write/read to the SD. This supports both Gateway and redirected layouts.", - option, (void*)OPTION_EMUNAND, change_opt, get_opt, 0 }, + option, (void*)OPTION_EMUNAND, change_opt, get_opt, 0, "" }, { "Index", "Which EmuNAND to use. If you only have one, you want 0. Currently the maximum supported is 10 (0-9), but this is arbitrary.", - option, (void*)OPTION_EMUNAND_INDEX, change_opt, get_opt, 1 }, + option, (void*)OPTION_EMUNAND_INDEX, change_opt, get_opt, 1, "" }, { "Autoboot", "Boot the system automatically, unless the R key is held while booting.", - option, (void*)OPTION_AUTOBOOT, change_opt, get_opt, 0 }, + option, (void*)OPTION_AUTOBOOT, change_opt, get_opt, 0, "" }, { "Silent mode", "Suppress all debug output during autoboot. You'll see the screen turn on and then off once.", - option, (void*)OPTION_SILENCE, change_opt, get_opt, 1 }, + option, (void*)OPTION_SILENCE, change_opt, get_opt, 1, "" }, { "Dim Background", "Experimental! Dims colors on lighter backgrounds to improve readability with text. You won't notice the change until scrolling or exiting the current menu due to the way rendering works.", - option, (void*)OPTION_DIM_MODE, change_opt, get_opt, 0 }, + option, (void*)OPTION_DIM_MODE, change_opt, get_opt, 0, "" }, { "Accent color", "Changes the accent color in menus.", - option, (void*)OPTION_ACCENT_COLOR, change_opt, get_opt, 0}, + option, (void*)OPTION_ACCENT_COLOR, change_opt, get_opt, 0, "" }, { "Brightness", "Changes the screeninit brightness in menu. WIP, only takes effect on reboot (this will change.)", - option, (void*)OPTION_BRIGHTNESS, change_opt, get_opt, 0}, + option, (void*)OPTION_BRIGHTNESS, change_opt, get_opt, 0, "" }, // space - { "", "", unselectable, 0, NULL, NULL, 0 }, + { "", "", unselectable, 0, NULL, NULL, 0, "" }, // Patches. - { "Loader Options", "", unselectable, 0, NULL, NULL, 0 }, + { "Loader Options", "", unselectable, 0, NULL, NULL, 0, "" }, { "CPU - L2 cache", "Forces the system to use the L2 cache on all applications. If you have issues with crashes, try turning this off.", - option_n3ds, (void*)OPTION_LOADER_CPU_L2, change_opt, get_opt, 0 }, + option_n3ds, (void*)OPTION_LOADER_CPU_L2, change_opt, get_opt, 0, "" }, { "CPU - 804Mhz", "Forces the system to run in 804Mhz mode on all applications.", - option_n3ds, (void*)OPTION_LOADER_CPU_800MHZ, change_opt, get_opt, 0 }, + option_n3ds, (void*)OPTION_LOADER_CPU_800MHZ, change_opt, get_opt, 0, "" }, { "Language Emulation", "Reads language emulation configuration from `" PATH_LOCEMU "` and imitates the region/language.", - option, (void*)OPTION_LOADER_LANGEMU, change_opt, get_opt, 0 }, + option, (void*)OPTION_LOADER_LANGEMU, change_opt, get_opt, 0, "" }, { "Load Code Sections", "Loads code sections (text/ro/data) from SD card and patches afterwards.", - option, (void*)OPTION_LOADER_LOADCODE, change_opt, get_opt, 0 }, + option, (void*)OPTION_LOADER_LOADCODE, change_opt, get_opt, 0, "" }, { "Dump Code Sections", "Dumps code sections for titles to SD card the first time they're loaded. Slows things down on first launch.", - option, (void*)OPTION_LOADER_DUMPCODE, change_opt, get_opt, 0 }, + option, (void*)OPTION_LOADER_DUMPCODE, change_opt, get_opt, 0, "" }, { "+ System Titles", "Dumps code sections for system titles, too. Expect to sit at a blank screen for >3mins on the first time you do this, because it dumps everything.", - option, (void*)OPTION_LOADER_DUMPCODE_ALL, change_opt, get_opt, 1 }, + option, (void*)OPTION_LOADER_DUMPCODE_ALL, change_opt, get_opt, 1, "" }, // space - { "", "", unselectable, 0, NULL, NULL, 0 }, + { "", "", unselectable, 0, NULL, NULL, 0, "" }, // Patches. - { "Developer Options", "", unselectable, 0, NULL, NULL, 0 }, + { "Developer Options", "", unselectable, 0, NULL, NULL, 0, "" }, { "Step Through", "After each important step, [WAIT] will be shown and you'll need to press a key. Debug feature.", - option, (void*)OPTION_TRACE, change_opt, get_opt, 0 }, + option, (void*)OPTION_TRACE, change_opt, get_opt, 0, "" }, { "Verbose", "Output more debug information than the average user needs.", - option, (void*)OPTION_OVERLY_VERBOSE, change_opt, get_opt, 0 }, + option, (void*)OPTION_OVERLY_VERBOSE, change_opt, get_opt, 0, "" }, { "Logging", "Save logs to `" LOCALSTATEDIR "` as `boot.log` and `loader.log`. Slows operation a bit.", - option, (void*)OPTION_SAVE_LOGS, change_opt, get_opt, 0 }, + option, (void*)OPTION_SAVE_LOGS, change_opt, get_opt, 0, "" }, // Sentinel. - { NULL, NULL, 0, 0, NULL, NULL, 0 }, // cursor_min and cursor_max are stored in the last two. + { NULL, NULL, 0, 0, NULL, NULL, 0, "" }, // cursor_min and cursor_max are stored in the last two. }; static int current_menu_index_patches = 0; @@ -144,7 +149,11 @@ static int desc_is_fname_sto = 0; char* patch_get_enable(void* val) { uint32_t opt = (uint32_t)val; uint32_t raw = enable_list[opt]; - return (raw) ? "*" : " "; + static char ret[2] = " "; + ret[0] = ' '; + if (raw == 1) + ret[0] = '*'; + return ret; } void patch_set_enable(void* val) { @@ -192,7 +201,7 @@ void patch_func(char* fpath) { // will be set to 1. void -list_patches_build(char *name, int desc_is_fname) +list_patches_build(const char *name, int desc_is_fname) { desc_is_fname_sto = desc_is_fname; @@ -231,7 +240,7 @@ menu_options() #define REL "master" #endif -#define ln(s) { s, "", unselectable, 0, NULL, NULL, 0 } +#define ln(s) { s, "", unselectable, 0, NULL, NULL, 0, "" } static struct options_s help_d[] = { ln("About"), @@ -260,7 +269,7 @@ static struct options_s help_d[] = { ln(" forgotten (yell at me, please!)"), ln(""), ln(" "), - { NULL, NULL, unselectable, 0, NULL, NULL, 0 }, // cursor_min and cursor_max are stored in the last two. + { NULL, NULL, unselectable, 0, NULL, NULL, 0, "" }, // cursor_min and cursor_max are stored in the last two. }; void @@ -302,13 +311,13 @@ void chainload_menu(); static struct options_s config_opts[] = { { "Options", "Internal options for the CFW.\nThese are part of " FW_NAME " itself.", - option, 0, menu_options, NULL, 0 }, + option, 0, menu_options, NULL, 0, "" }, { "Patches", "External bytecode patches found in `" PATH_PATCHES "`.\nYou can choose which to enable.", - option, 0, menu_patches, NULL, 0 }, + option, 0, menu_patches, NULL, 0, "" }, // Sentinel. - { NULL, NULL, 0, 0, NULL, NULL, 0 }, // cursor_min and cursor_max are stored in the last two. + { NULL, NULL, 0, 0, NULL, NULL, 0, "" }, // cursor_min and cursor_max are stored in the last two. }; void config_main_menu() { @@ -322,27 +331,27 @@ void config_main_menu() { static struct options_s main_s[] = { { "Configuration", "Configuration options for the CFW.", - option, 0, config_main_menu, NULL, 0 }, + option, 0, config_main_menu, NULL, 0, "" }, { "Readme", "Mini-readme.\nWhy are you opening help on this, though?\nThat's kind of silly.", - option, 0, menu_help, NULL, 0 }, + option, 0, menu_help, NULL, 0, "" }, { "Reboot", "Reboots the console.", - option, 0, reset, NULL, 0 }, + option, 0, reset, NULL, 0, "" }, { "Power off", "Powers off the console.", - option, 0, poweroff, NULL, 0 }, + option, 0, poweroff, NULL, 0, "" }, #if defined(CHAINLOADER) && CHAINLOADER == 1 { "Chainload", "Boot another ARM9 payload file.", - option, 0, chainload_menu, NULL, 0}, + option, 0, chainload_menu, NULL, 0, "" }, #endif { "Boot Firmware", "Generates caches, patches the firmware, and boots it.\nMake sure to 'Save Configuration' first if any options changed.", - break_menu, 0, NULL, NULL, 0 }, + break_menu, 0, NULL, NULL, 0, "" }, // Sentinel. - { NULL, NULL, 0, 0, NULL, NULL, 0 }, // cursor_min and cursor_max are stored in the last two. + { NULL, NULL, 0, 0, NULL, NULL, 0, "" }, // cursor_min and cursor_max are stored in the last two. }; void diff --git a/source/std/allocator.c b/source/std/allocator.c index dd9df41..1b61df7 100644 --- a/source/std/allocator.c +++ b/source/std/allocator.c @@ -28,7 +28,7 @@ typedef struct free_block { size_t size; size_t real_size; #ifdef MALLOC_DEBUG - char* info; + const char* info; #endif struct free_block* next; } free_block; @@ -51,7 +51,7 @@ static size_t allocated_memory = 0; #endif #ifdef MALLOC_DEBUG -void* malloc_chkd(size_t size, char* info) { +void* malloc_chkd(size_t size, const char* info) { #else void* malloc(size_t size) { #endif @@ -102,7 +102,7 @@ void* malloc_zero(size_t size) { } #ifdef MALLOC_DEBUG -void free_chkd(void* ptr, char* info) { +void free_chkd(void* ptr, const char* info) { #else void free(void* ptr) { #endif diff --git a/source/std/draw.c b/source/std/draw.c index b57532e..83622c3 100644 --- a/source/std/draw.c +++ b/source/std/draw.c @@ -156,7 +156,7 @@ void clear_bg() { memset(bottom_bg, 0, BOTTOM_SIZE); } -void load_bg_top(char* fname_top) { +void load_bg_top(const char* fname_top) { FILE* f = fopen(fname_top, "r"); if (!f) return; @@ -165,7 +165,7 @@ void load_bg_top(char* fname_top) { fclose(f); } -void load_bg_bottom(char* fname_bottom) { +void load_bg_bottom(const char* fname_bottom) { FILE* f = fopen(fname_bottom, "r"); if (!f) return; @@ -637,12 +637,13 @@ fflush(void *channel) int disable_format = 0; void -vfprintf(void *channel, char *format, va_list ap) +vfprintf(void *channel, const char *format, va_list ap) { if ((channel == stdout || channel == stderr) && kill_output) return; - char *ref = (char *)format; + char *copy = strdup_self(format); + char *ref = copy; unsigned char *color = NULL; if (channel == TOP_SCREEN) @@ -708,10 +709,12 @@ vfprintf(void *channel, char *format, va_list ap) } ++ref; } + + free(copy); } void -fprintf(void *channel, char *format, ...) +fprintf(void *channel, const char *format, ...) { // The output suppression is in all the functions to minimize overhead. // Function calls and format conversions take time and we don't just want @@ -728,7 +731,7 @@ fprintf(void *channel, char *format, ...) } void -printf(char *format, ...) +printf(const char *format, ...) { va_list ap; va_start(ap, format); diff --git a/source/std/fs.c b/source/std/fs.c index 610263c..1ab7f54 100644 --- a/source/std/fs.c +++ b/source/std/fs.c @@ -50,14 +50,14 @@ recurse_call_back(char *fpath, void (*call_fun_param)(char*)) return 0; } -void recurse_call(char *name, void (*call_fun_param)(char*)) { - char fpath[256]; +void recurse_call(const char *name, void (*call_fun_param)(char*)) { + static char fpath[256]; strncpy(fpath, name, 256); recurse_call_back(fpath, call_fun_param); } int -rrmdir(char *name) +rrmdir(const char *name) { recurse_call(name, (void (*)(char*))f_unlink); return 0; @@ -205,7 +205,7 @@ fread(void *buffer, size_t elementSize, size_t elementCnt, FILE *fp) } size_t -write_file(void *data, char *path, size_t size) +write_file(void *data, const char *path, size_t size) { FILE *temp = fopen(path, "w"); @@ -225,7 +225,7 @@ write_file(void *data, char *path, size_t size) } size_t -read_file(void *data, char *path, size_t size) +read_file(void *data, const char *path, size_t size) { FILE *temp = fopen(path, "r"); diff --git a/source/std/memory.c b/source/std/memory.c index f1c9465..c5d1ddc 100644 --- a/source/std/memory.c +++ b/source/std/memory.c @@ -4,7 +4,7 @@ #include char* -strdup_self(char* str) +strdup_self(const char* str) { size_t l = strlen(str); char* new_st = malloc(l+1);