From 74e8328d3f6901d6ba38a313965fe910c8411324 Mon Sep 17 00:00:00 2001 From: Amar Tumballi Date: Thu, 1 Nov 2018 07:25:25 +0530 Subject: all: fix the format string exceptions Currently, there are possibilities in few places, where a user-controlled (like filename, program parameter etc) string can be passed as 'fmt' for printf(), which can lead to segfault, if the user's string contains '%s', '%d' in it. While fixing it, makes sense to make the explicit check for such issues across the codebase, by making the format call properly. Fixes: CVE-2018-14661 Fixes: bz#1644763 Change-Id: Ib547293f2d9eb618594cbff0df3b9c800e88bde4 Signed-off-by: Amar Tumballi --- libglusterfs/src/client_t.c | 7 ++++--- libglusterfs/src/fd.c | 9 +++++---- libglusterfs/src/inode.c | 4 ++-- libglusterfs/src/iobuf.c | 19 ++++++++++--------- libglusterfs/src/latency.c | 2 +- libglusterfs/src/logging.h | 3 ++- libglusterfs/src/mem-pool.h | 3 ++- libglusterfs/src/run.h | 4 ++-- libglusterfs/src/stack.c | 8 ++++---- libglusterfs/src/statedump.c | 13 +++++++------ libglusterfs/src/statedump.h | 6 ++++-- 11 files changed, 43 insertions(+), 35 deletions(-) (limited to 'libglusterfs') diff --git a/libglusterfs/src/client_t.c b/libglusterfs/src/client_t.c index 35e0beda8d6..586cbd84e5c 100644 --- a/libglusterfs/src/client_t.c +++ b/libglusterfs/src/client_t.c @@ -585,7 +585,8 @@ client_dump(client_t *client, char *prefix) if (!client) return; - gf_proc_dump_write("refcount", GF_PRI_ATOMIC, GF_ATOMIC_GET(client->count)); + gf_proc_dump_write("refcount", "%" GF_PRI_ATOMIC, + GF_ATOMIC_GET(client->count)); } void @@ -626,7 +627,7 @@ clienttable_dump(clienttable_t *clienttable, char *prefix) if (GF_CLIENTENTRY_ALLOCATED == clienttable->cliententries[i].next_free) { gf_proc_dump_build_key(key, prefix, "cliententry[%d]", i); - gf_proc_dump_add_section(key); + gf_proc_dump_add_section("%s", key); cliententry_dump(&clienttable->cliententries[i], key); } } @@ -773,7 +774,7 @@ gf_client_dump_fdtables(xlator_t *this) gf_proc_dump_write(key, "%s", client->subdir_mount); } gf_proc_dump_build_key(key, "conn", "%d.ref", count); - gf_proc_dump_write(key, GF_PRI_ATOMIC, + gf_proc_dump_write(key, "%" GF_PRI_ATOMIC, GF_ATOMIC_GET(client->count)); if (client->bound_xl) { gf_proc_dump_build_key(key, "conn", "%d.bound_xl", count); diff --git a/libglusterfs/src/fd.c b/libglusterfs/src/fd.c index d26b7097fd4..6c521317110 100644 --- a/libglusterfs/src/fd.c +++ b/libglusterfs/src/fd.c @@ -992,13 +992,14 @@ fd_dump(fd_t *fd, char *prefix) if (!fd) return; - gf_proc_dump_write("pid", "%llu", fd->pid); - gf_proc_dump_write("refcount", "%d", fd->refcount); + gf_proc_dump_write("pid", "%" PRIu64, fd->pid); + gf_proc_dump_write("refcount", "%" GF_PRI_ATOMIC, + GF_ATOMIC_GET(fd->refcount)); gf_proc_dump_write("flags", "%d", fd->flags); if (fd->inode) { gf_proc_dump_build_key(key, "inode", NULL); - gf_proc_dump_add_section(key); + gf_proc_dump_add_section("%s", key); inode_dump(fd->inode, key); } } @@ -1040,7 +1041,7 @@ fdtable_dump(fdtable_t *fdtable, char *prefix) for (i = 0; i < fdtable->max_fds; i++) { if (GF_FDENTRY_ALLOCATED == fdtable->fdentries[i].next_free) { gf_proc_dump_build_key(key, prefix, "fdentry[%d]", i); - gf_proc_dump_add_section(key); + gf_proc_dump_add_section("%s", key); fdentry_dump(&fdtable->fdentries[i], key); } } diff --git a/libglusterfs/src/inode.c b/libglusterfs/src/inode.c index 538c7797c61..8bfd5c70db8 100644 --- a/libglusterfs/src/inode.c +++ b/libglusterfs/src/inode.c @@ -31,7 +31,7 @@ { \ gf_proc_dump_build_key(key_buf, key_prefix, "%s.%d", list_type, \ i++); \ - gf_proc_dump_add_section(key_buf); \ + gf_proc_dump_add_section("%s", key_buf); \ inode_dump(inode, key); \ } \ } @@ -2341,7 +2341,7 @@ inode_table_dump(inode_table_t *itable, char *prefix) } gf_proc_dump_build_key(key, prefix, "hashsize"); - gf_proc_dump_write(key, "%d", itable->hashsize); + gf_proc_dump_write(key, "%" GF_PRI_SIZET, itable->hashsize); gf_proc_dump_build_key(key, prefix, "name"); gf_proc_dump_write(key, "%s", itable->name); diff --git a/libglusterfs/src/iobuf.c b/libglusterfs/src/iobuf.c index 8682420d8f8..c9e0ff35198 100644 --- a/libglusterfs/src/iobuf.c +++ b/libglusterfs/src/iobuf.c @@ -1068,7 +1068,7 @@ iobuf_info_dump(struct iobuf *iobuf, const char *key_prefix) UNLOCK(&iobuf->lock); gf_proc_dump_build_key(key, key_prefix, "ref"); - gf_proc_dump_write(key, "%d", my_iobuf.ref); + gf_proc_dump_write(key, "%" GF_PRI_ATOMIC, GF_ATOMIC_GET(my_iobuf.ref)); gf_proc_dump_build_key(key, key_prefix, "ptr"); gf_proc_dump_write(key, "%p", my_iobuf.ptr); @@ -1094,13 +1094,13 @@ iobuf_arena_info_dump(struct iobuf_arena *iobuf_arena, const char *key_prefix) gf_proc_dump_build_key(key, key_prefix, "alloc_cnt"); gf_proc_dump_write(key, "%" PRIu64, iobuf_arena->alloc_cnt); gf_proc_dump_build_key(key, key_prefix, "max_active"); - gf_proc_dump_write(key, "%" PRIu64, iobuf_arena->max_active); + gf_proc_dump_write(key, "%d", iobuf_arena->max_active); gf_proc_dump_build_key(key, key_prefix, "page_size"); - gf_proc_dump_write(key, "%" PRIu64, iobuf_arena->page_size); + gf_proc_dump_write(key, "%" GF_PRI_SIZET, iobuf_arena->page_size); list_for_each_entry(trav, &iobuf_arena->active.list, list) { gf_proc_dump_build_key(key, key_prefix, "active_iobuf.%d", i++); - gf_proc_dump_add_section(key); + gf_proc_dump_add_section("%s", key); iobuf_info_dump(trav, key); } @@ -1126,9 +1126,10 @@ iobuf_stats_dump(struct iobuf_pool *iobuf_pool) } gf_proc_dump_add_section("iobuf.global"); gf_proc_dump_write("iobuf_pool", "%p", iobuf_pool); - gf_proc_dump_write("iobuf_pool.default_page_size", "%d", + gf_proc_dump_write("iobuf_pool.default_page_size", "%" GF_PRI_SIZET, iobuf_pool->default_page_size); - gf_proc_dump_write("iobuf_pool.arena_size", "%d", iobuf_pool->arena_size); + gf_proc_dump_write("iobuf_pool.arena_size", "%" GF_PRI_SIZET, + iobuf_pool->arena_size); gf_proc_dump_write("iobuf_pool.arena_cnt", "%d", iobuf_pool->arena_cnt); gf_proc_dump_write("iobuf_pool.request_misses", "%" PRId64, iobuf_pool->request_misses); @@ -1137,21 +1138,21 @@ iobuf_stats_dump(struct iobuf_pool *iobuf_pool) list_for_each_entry(trav, &iobuf_pool->arenas[j], list) { snprintf(msg, sizeof(msg), "arena.%d", i); - gf_proc_dump_add_section(msg); + gf_proc_dump_add_section("%s", msg); iobuf_arena_info_dump(trav, msg); i++; } list_for_each_entry(trav, &iobuf_pool->purge[j], list) { snprintf(msg, sizeof(msg), "purge.%d", i); - gf_proc_dump_add_section(msg); + gf_proc_dump_add_section("%s", msg); iobuf_arena_info_dump(trav, msg); i++; } list_for_each_entry(trav, &iobuf_pool->filled[j], list) { snprintf(msg, sizeof(msg), "filled.%d", i); - gf_proc_dump_add_section(msg); + gf_proc_dump_add_section("%s", msg); iobuf_arena_info_dump(trav, msg); i++; } diff --git a/libglusterfs/src/latency.c b/libglusterfs/src/latency.c index 2dc2a318216..afbb6dcad80 100644 --- a/libglusterfs/src/latency.c +++ b/libglusterfs/src/latency.c @@ -69,7 +69,7 @@ gf_proc_dump_latency_info(xlator_t *xl) int i; snprintf(key_prefix, GF_DUMP_MAX_BUF_LEN, "%s.latency", xl->name); - gf_proc_dump_add_section(key_prefix); + gf_proc_dump_add_section("%s", key_prefix); for (i = 0; i < GF_FOP_MAXVALUE; i++) { gf_proc_dump_build_key(key, key_prefix, "%s", (char *)gf_fop_list[i]); diff --git a/libglusterfs/src/logging.h b/libglusterfs/src/logging.h index f2294488fae..859050d568b 100644 --- a/libglusterfs/src/logging.h +++ b/libglusterfs/src/logging.h @@ -183,7 +183,8 @@ _gf_log_callingfn(const char *domain, const char *file, const char *function, __attribute__((__format__(__printf__, 6, 7))); int -_gf_log_eh(const char *function, const char *fmt, ...); +_gf_log_eh(const char *function, const char *fmt, ...) + __attribute__((__format__(__printf__, 2, 3))); /* treat GF_LOG_TRACE and GF_LOG_NONE as LOG_DEBUG and * other level as is */ diff --git a/libglusterfs/src/mem-pool.h b/libglusterfs/src/mem-pool.h index af6b4decd2c..118170fd17f 100644 --- a/libglusterfs/src/mem-pool.h +++ b/libglusterfs/src/mem-pool.h @@ -93,7 +93,8 @@ int gf_vasprintf(char **string_ptr, const char *format, va_list arg); int -gf_asprintf(char **string_ptr, const char *format, ...); +gf_asprintf(char **string_ptr, const char *format, ...) + __attribute__((__format__(__printf__, 2, 3))); void __gf_free(void *ptr); diff --git a/libglusterfs/src/run.h b/libglusterfs/src/run.h index dd19972d07e..76af95fd27f 100644 --- a/libglusterfs/src/run.h +++ b/libglusterfs/src/run.h @@ -81,8 +81,8 @@ runner_add_args(runner_t *runner, ...); * @param format printf style format specifier */ void -runner_argprintf(runner_t *runner, const char *format, ...); - +runner_argprintf(runner_t *runner, const char *format, ...) + __attribute__((__format__(__printf__, 2, 3))); /** * log a message about the command to be run. * diff --git a/libglusterfs/src/stack.c b/libglusterfs/src/stack.c index c8a3b792979..4b5d13c3fc8 100644 --- a/libglusterfs/src/stack.c +++ b/libglusterfs/src/stack.c @@ -148,7 +148,7 @@ gf_proc_dump_call_frame(call_frame_t *call_frame, const char *key_buf, ...) out: if (ret) { gf_proc_dump_write("Unable to dump the frame information", - "(Lock acquisition failed) %p", my_frame); + "(Lock acquisition failed)"); return; } } @@ -185,9 +185,9 @@ gf_proc_dump_call_stack(call_stack_t *call_stack, const char *key_buf, ...) gf_proc_dump_write("uid", "%d", call_stack->uid); gf_proc_dump_write("gid", "%d", call_stack->gid); gf_proc_dump_write("pid", "%d", call_stack->pid); - gf_proc_dump_write("unique", "%Ld", call_stack->unique); + gf_proc_dump_write("unique", "%" PRIu64, call_stack->unique); gf_proc_dump_write("lk-owner", "%s", lkowner_utoa(&call_stack->lk_owner)); - gf_proc_dump_write("ctime", "%lld.%" GF_PRI_SNSECONDS, + gf_proc_dump_write("ctime", "%" GF_PRI_SECOND ".%" GF_PRI_SNSECONDS, call_stack->tv.tv_sec, call_stack->tv.tv_nsec); if (call_stack->type == GF_OP_TYPE_FOP) @@ -224,7 +224,7 @@ gf_proc_dump_pending_frames(call_pool_t *call_pool) gf_proc_dump_add_section("global.callpool"); section_added = _gf_true; gf_proc_dump_write("callpool_address", "%p", call_pool); - gf_proc_dump_write("callpool.cnt", "%d", call_pool->cnt); + gf_proc_dump_write("callpool.cnt", "%" PRId64, call_pool->cnt); list_for_each_entry(trav, &call_pool->all_frames, all_frames) { diff --git a/libglusterfs/src/statedump.c b/libglusterfs/src/statedump.c index d0701e53a84..0f4a7102697 100644 --- a/libglusterfs/src/statedump.c +++ b/libglusterfs/src/statedump.c @@ -255,12 +255,13 @@ gf_proc_dump_xlator_mem_info_only_in_use(xlator_t *xl) gf_proc_dump_add_section("%s.%s - usage-type %d", xl->type, xl->name, i); - gf_proc_dump_write("size", "%u", xl->mem_acct->rec[i].size); - gf_proc_dump_write("max_size", "%u", xl->mem_acct->rec[i].max_size); + gf_proc_dump_write("size", "%" PRIu64, xl->mem_acct->rec[i].size); + gf_proc_dump_write("max_size", "%" PRIu64, + xl->mem_acct->rec[i].max_size); gf_proc_dump_write("num_allocs", "%u", xl->mem_acct->rec[i].num_allocs); gf_proc_dump_write("max_num_allocs", "%u", xl->mem_acct->rec[i].max_num_allocs); - gf_proc_dump_write("total_allocs", "%u", + gf_proc_dump_write("total_allocs", "%" PRIu64, xl->mem_acct->rec[i].total_allocs); } @@ -379,8 +380,8 @@ gf_proc_dump_mempool_info(glusterfs_ctx_t *ctx) gf_proc_dump_write("-----", "-----"); gf_proc_dump_write("pool-name", "%s", pool->name); gf_proc_dump_write("active-count", "%" GF_PRI_ATOMIC, active); - gf_proc_dump_write("sizeof-type", "%d", pool->sizeof_type); - gf_proc_dump_write("padded-sizeof", "%lu", + gf_proc_dump_write("sizeof-type", "%lu", pool->sizeof_type); + gf_proc_dump_write("padded-sizeof", "%d", 1 << pool->pool->power_of_two); gf_proc_dump_write("size", "%lu", (1 << pool->pool->power_of_two) * active); @@ -466,7 +467,7 @@ gf_proc_dump_dict_info(glusterfs_ctx_t *ctx) total_dicts = GF_ATOMIC_GET(ctx->stats.total_dicts_used); total_pairs = GF_ATOMIC_GET(ctx->stats.total_pairs_used); - gf_proc_dump_write("max-pairs-per-dict", "%u", + gf_proc_dump_write("max-pairs-per-dict", "%" GF_PRI_ATOMIC, GF_ATOMIC_GET(ctx->stats.max_dict_pairs)); gf_proc_dump_write("total-pairs-used", "%lu", total_pairs); gf_proc_dump_write("total-dicts-used", "%lu", total_dicts); diff --git a/libglusterfs/src/statedump.h b/libglusterfs/src/statedump.h index 6c32c161ad3..af653041493 100644 --- a/libglusterfs/src/statedump.h +++ b/libglusterfs/src/statedump.h @@ -81,10 +81,12 @@ void gf_proc_dump_info(int signum, glusterfs_ctx_t *ctx); int -gf_proc_dump_add_section(char *key, ...); +gf_proc_dump_add_section(char *key, ...) + __attribute__((__format__(__printf__, 1, 2))); int -gf_proc_dump_write(char *key, char *value, ...); +gf_proc_dump_write(char *key, char *value, ...) + __attribute__((__format__(__printf__, 2, 3))); void inode_table_dump(inode_table_t *itable, char *prefix); -- cgit