From 3745c813f924b12232c4f37610aecd23fe5654b8 Mon Sep 17 00:00:00 2001 From: William Wilgus Date: Wed, 23 Nov 2022 21:46:13 -0500 Subject: misc.c open_pathfmt caller supplied buffer Amachronic raised concern about open() blocking causing a static buf to get overwritten in multiple calls its prudent to just have the caller supply the buffer to minimize stack issues later Change-Id: Iae27c7d063adb1a65688f920f6aa5c395fa5694a --- apps/bookmark.c | 12 ++++-------- apps/debug_menu.c | 3 ++- apps/filetypes.c | 9 +++++---- apps/gui/icon.c | 4 +++- apps/logfdisp.c | 3 ++- apps/misc.c | 5 ++--- apps/misc.h | 2 +- apps/tagcache.c | 19 ++++++++++++------- apps/talk.c | 4 +++- 9 files changed, 34 insertions(+), 27 deletions(-) diff --git a/apps/bookmark.c b/apps/bookmark.c index 20841b4940..8256f76ef8 100644 --- a/apps/bookmark.c +++ b/apps/bookmark.c @@ -290,10 +290,8 @@ static bool add_bookmark(const char* bookmark_file_name, const char* bookmark, bool equal; /* Opening up a temp bookmark file */ - snprintf(global_temp_buffer, sizeof(global_temp_buffer), - "%s.tmp", bookmark_file_name); - temp_bookmark_file = open(global_temp_buffer, - O_WRONLY | O_CREAT | O_TRUNC, 0666); + temp_bookmark_file = open_pathfmt(global_temp_buffer, sizeof(global_temp_buffer), + O_WRONLY | O_CREAT | O_TRUNC, "%s.tmp", bookmark_file_name); if (temp_bookmark_file < 0) return false; /* can't open the temp file */ @@ -893,10 +891,8 @@ static bool delete_bookmark(const char* bookmark_file_name, int bookmark_id) int bookmark_count = 0; /* Opening up a temp bookmark file */ - snprintf(global_temp_buffer, sizeof(global_temp_buffer), - "%s.tmp", bookmark_file_name); - temp_bookmark_file = open(global_temp_buffer, - O_WRONLY | O_CREAT | O_TRUNC, 0666); + temp_bookmark_file = open_pathfmt(global_temp_buffer, sizeof(global_temp_buffer), + O_WRONLY | O_CREAT | O_TRUNC, "%s.tmp", bookmark_file_name); if (temp_bookmark_file < 0) return false; /* can't open the temp file */ diff --git a/apps/debug_menu.c b/apps/debug_menu.c index a4bfe65b1c..71c0395e6e 100644 --- a/apps/debug_menu.c +++ b/apps/debug_menu.c @@ -2307,8 +2307,9 @@ static bool cpu_boost_log_dump(void) return false; #if CONFIG_RTC + char fname[MAX_PATH]; struct tm *nowtm = get_time(); - fd = open_pathfmt(O_CREAT|O_WRONLY|O_TRUNC, + fd = open_pathfmt(fname, sizeof(fname), O_CREAT|O_WRONLY|O_TRUNC, "%s/boostlog_%04d%02d%02d%02d%02d%02d.txt", ROCKBOX_DIR, nowtm->tm_year + 1900, nowtm->tm_mon + 1, nowtm->tm_mday, nowtm->tm_hour, nowtm->tm_min, nowtm->tm_sec); diff --git a/apps/filetypes.c b/apps/filetypes.c index d690b554fd..ec9bd1a7ae 100644 --- a/apps/filetypes.c +++ b/apps/filetypes.c @@ -325,8 +325,8 @@ void read_color_theme_file(void) { if (!global_settings.colors_file[0] || global_settings.colors_file[0] == '-') return; - fd = open_pathfmt(O_RDONLY, THEME_DIR "/%s.colours", - global_settings.colors_file); + fd = open_pathfmt(buffer, sizeof(buffer), O_RDONLY, + THEME_DIR "/%s.colours", global_settings.colors_file); if (fd < 0) return; while (read_line(fd, buffer, MAX_PATH) > 0) @@ -365,10 +365,11 @@ void read_viewer_theme_file(void) custom_filetype_icons[i] = filetypes[i].icon; } - fd = open_pathfmt(O_RDONLY, "%s/%s.icons", ICON_DIR, - global_settings.viewers_icon_file); + fd = open_pathfmt(buffer, sizeof(buffer), O_RDONLY, + ICON_DIR "/%s.icons", global_settings.viewers_icon_file); if (fd < 0) return; + while (read_line(fd, buffer, MAX_PATH) > 0) { if (!settings_parseline(buffer, &ext, &icon)) diff --git a/apps/gui/icon.c b/apps/gui/icon.c index 2c09f88852..9deb1a0c65 100644 --- a/apps/gui/icon.c +++ b/apps/gui/icon.c @@ -182,7 +182,9 @@ static void load_icons(const char* filename, enum Iconset iconset, ic->handle = 0; if (filename[0] && filename[0] != '-') { - fd = open_pathfmt(O_RDONLY, ICON_DIR "/%s.bmp", filename); + char fname[MAX_PATH]; + fd = open_pathfmt(fname, sizeof(fname), O_RDONLY, + ICON_DIR "/%s.bmp", filename); if (fd < 0) return; buf_size = read_bmp_fd(fd, &ic->bmp, 0, diff --git a/apps/logfdisp.c b/apps/logfdisp.c index aebc9ffb33..efbfa192f5 100644 --- a/apps/logfdisp.c +++ b/apps/logfdisp.c @@ -225,8 +225,9 @@ bool logfdump(void) logfenabled = false; #if CONFIG_RTC + char fname[MAX_PATH]; struct tm *nowtm = get_time(); - fd = open_pathfmt(O_CREAT|O_WRONLY|O_TRUNC, + fd = open_pathfmt(fname, sizeof(fname), O_CREAT|O_WRONLY|O_TRUNC, "%s/logf_%04d%02d%02d%02d%02d%02d.txt", ROCKBOX_DIR, nowtm->tm_year + 1900, nowtm->tm_mon + 1, nowtm->tm_mday, nowtm->tm_hour, nowtm->tm_min, nowtm->tm_sec); diff --git a/apps/misc.c b/apps/misc.c index e2913d53b3..eb821548fe 100644 --- a/apps/misc.c +++ b/apps/misc.c @@ -1419,12 +1419,11 @@ int string_option(const char *option, const char *const oplist[], bool ignore_ca } /* open but with a builtin printf for assembling the path */ -int open_pathfmt(int oflag, const char *pathfmt, ...) +int open_pathfmt(char *buf, size_t size, int oflag, const char *pathfmt, ...) { - static char buf[MAX_PATH]; va_list ap; va_start(ap, pathfmt); - vsnprintf(buf, sizeof(buf), pathfmt, ap); + vsnprintf(buf, size, pathfmt, ap); va_end(ap); return open(buf, oflag, 0666); } diff --git a/apps/misc.h b/apps/misc.h index 50191f0e95..f31d4d363c 100644 --- a/apps/misc.h +++ b/apps/misc.h @@ -122,7 +122,7 @@ extern int show_logo(void); #define BOM_UTF_16_SIZE 2 int split_string(char *str, const char needle, char *vector[], int vector_length); -int open_pathfmt(int oflag, const char *pathfmt, ...); +int open_pathfmt(char *buf, size_t size, int oflag, const char *pathfmt, ...); int open_utf8(const char* pathname, int flags); int string_option(const char *option, const char *const oplist[], bool ignore_case); diff --git a/apps/tagcache.c b/apps/tagcache.c index a297bc7353..a66f9658ae 100644 --- a/apps/tagcache.c +++ b/apps/tagcache.c @@ -423,11 +423,13 @@ static int open_tag_fd(struct tagcache_header *hdr, int tag, bool write) { int fd; int rc; + char fname[MAX_PATH]; if (TAGCACHE_IS_NUMERIC(tag) || tag < 0 || tag >= TAG_COUNT) return -1; - fd = open_pathfmt(write ? O_RDWR : O_RDONLY, TAGCACHE_FILE_INDEX, tag); + fd = open_pathfmt(fname, sizeof(fname), + write ? O_RDWR : O_RDONLY, TAGCACHE_FILE_INDEX, tag); if (fd < 0) { @@ -803,7 +805,9 @@ static bool open_files(struct tagcache_search *tcs, int tag) { if (tcs->idxfd[tag] < 0) { - tcs->idxfd[tag] = open_pathfmt(O_RDONLY, TAGCACHE_FILE_INDEX, tag); + char fname[MAX_PATH]; + tcs->idxfd[tag] = open_pathfmt(fname, sizeof(fname), + O_RDONLY, TAGCACHE_FILE_INDEX, tag); } if (tcs->idxfd[tag] < 0) @@ -1583,9 +1587,10 @@ bool tagcache_search_add_clause(struct tagcache_search *tcs, } if (!TAGCACHE_IS_NUMERIC(clause->tag) && tcs->idxfd[clause->tag] < 0) - { - tcs->idxfd[clause->tag] = open_pathfmt(O_RDONLY, - TAGCACHE_FILE_INDEX, clause->tag); + { + char fname[MAX_PATH]; + tcs->idxfd[clause->tag] = open_pathfmt(fname, sizeof(fname), O_RDONLY, + TAGCACHE_FILE_INDEX, clause->tag); } } @@ -2743,7 +2748,7 @@ static int build_index(int index_type, struct tagcache_header *h, int tmpfd) * Creating new index file to store the tags. No need to preload * anything whether the index type is sorted or not. */ - fd = open_pathfmt(O_WRONLY | O_CREAT | O_TRUNC, + fd = open_pathfmt(buf, bufsz, O_WRONLY | O_CREAT | O_TRUNC, TAGCACHE_FILE_INDEX, index_type); if (fd < 0) { @@ -4401,7 +4406,7 @@ static bool check_deleted_files(void) logf("reverse scan..."); - fd = open_pathfmt(O_RDONLY, TAGCACHE_FILE_INDEX, tag_filename); + fd = open_pathfmt(buf, bufsz, O_RDONLY, TAGCACHE_FILE_INDEX, tag_filename); if (fd < 0) { logf(TAGCACHE_FILE_INDEX " open fail", tag_filename); diff --git a/apps/talk.c b/apps/talk.c index 89319ae9a2..c3a1148df4 100644 --- a/apps/talk.c +++ b/apps/talk.c @@ -247,6 +247,7 @@ static struct buflib_callbacks talk_ops = { static int open_voicefile(void) { + char fname[MAX_PATH]; char* p_lang = DEFAULT_VOICE_LANG; /* default */ if ( global_settings.lang_file[0] && @@ -255,7 +256,8 @@ static int open_voicefile(void) p_lang = (char *)global_settings.lang_file; } - return open_pathfmt(O_RDONLY, LANG_DIR "/%s.voice", p_lang); + return open_pathfmt(fname, sizeof(fname), + O_RDONLY, LANG_DIR "/%s.voice", p_lang); } -- cgit v1.2.3